Re: [PATCH 1/3] KVM: Support sharing gpc locks

[David Woodhouse]

(Resending as the corporate email system has taken to adding redundant
information in transit as an HTML part — to a message that didn't need
it, which already had it in the Organization: header anyway, and was
previously plain text only.)

On Fri, 2023-01-27 at 13:44 +0900, David Stevens wrote:
> @@ -316,19 +316,19 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
>                  * takes them more than one at a time. Set a subclass on the
>                  * gpc1 lock to make lockdep shut up about it.
>                  */
> -               lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_);
> +               lock_set_subclass(gpc1->lock.dep_map, 1, _THIS_IP_);

The test robot already pointed out this should be gpc1->lock->dep_map,
but if we do what I said below in the first attempt at this email, this
all goes away anyway.


>                 if (atomic) {
> -                       if (!read_trylock(&gpc2->lock)) {
> -                               read_unlock_irqrestore(&gpc1->lock, flags);
> +                       if (!read_trylock(gpc2->lock)) {
> +                               read_unlock_irqrestore(gpc1->lock, flags);
>                                 return;
>                         }

LGTM without having had time to go through it in detail. As Sean said,
we were waiting for someone (perhaps even ourselves) to do this.
Thanks.

For the runstate one above, this is crying out for gpc1 and gpc2 to
*share* a lock, and ditch the relocking and the whole comment about
being able to declare a lock ordering between the two, which is half
shown in the context cited above.

Probably worth doing that in a separate patch on top of this; I'll take
a look at it when my feet next hit the ground on Tuesday if you haven't
already done so. It's easy to test with the xen_shinfo_test self-test.

Or with this if you want more fun:
https://lore.kernel.org/qemu-devel/20230128081113.1615111-1-dwmw2@xxxxxxxxxxxxx/

:)

Attachment: smime.p7s
Description: S/MIME cryptographic signature