On Thu, 2023-01-12 at 16:28 +0000, Paul Durrant wrote:
> On 12/01/2023 16:27, David Woodhouse wrote:
> > On Thu, 2023-01-12 at 16:17 +0000, Paul Durrant wrote:
> > > >
> > > > @@ -309,7 +317,14 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
> > > > * gpc1 lock to make lockdep shut up about it.
> > > > */
> > > > lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_);
> > > > - read_lock(&gpc2->lock);
> > > > + if (atomic) {
> > > > + if (!read_trylock(&gpc2->lock)) {
> > >
> > > You could avoid the nesting in this case with:
> > >
> > > if (atomic && !read_trylock(&gpc2->lock))
> > >
> > > > + read_unlock_irqrestore(&gpc1->lock, flags);
> > > > + return;
> > > > + }
> > > > + } else {
> > > > + read_lock(&gpc2->lock);
> > > > + }
> >
> > Hm? Wouldn't it take the lock twice then? It'd still take the 'else' branch.
>
> Actually, yes... So much for hoping to make it look prettier.
I suppose we could avoid the extra indentation by making it
if (atomic && !read_trylock(&gpc2->lock)) {
read_unlock_irqrestore(&gpc1->lock, flags);
return;
} else if (!atomic) {
read_lock(&gpc2->lock);
}
Not sure it's much of an improvement in overall readability?
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
