Re: [Xen-devel] [PATCH] gnttab: lock the left grant table earlier in __gnttab_unmap_common()

[Anthony Liguori <anthony@xxxxxxxxxxxxx>]

On Mon, Nov 11, 2013 at 2:53 AM, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> On 11/11/13 02:07, Matt Wilson wrote:
>> From: Matt Wilson <msw@xxxxxxxxxx>
>>
>> Luckily today maptrack_limit never shrinks. But if at some point in
>> the future this were to change, checking maptrack_limit without
>> holding the grant table spinlock would no longer be safe.
>
> I don't think we should extend region the grant table lock protects if
> it's not needed since we know this lock is heavily contended.

It is heavily contended.  So heavily contended that with a high number
of disks (20+) you lose 50% of the potential IOPS just because of the
grant lock.  Persistent grants paper over this problem but with older
guests there's no way to avoid it.

A little critical section like this isn't going to move the scale very
much and correctness should always trump performance.

Letting a guest confuse the hypervisor is a recipe for an XSA.  I
could appreciate a suggestion to replace the change with a comment
explaining why we're allowing inconsistency but letting it be silently
broken is not reasonable IMHO.

Just keeping a few instructions out of the critical section isn't
going to fix the 50% degradation.  Fortunately, Matt has been working
on this problem and that's why he is poking at this code.  He's got a
patch that splits the grant lock and we get back all of the lost
performance.  Matt, perhaps you can post a WIP version of the patch?

> Also, doesn't the 'l' and 'r' prefixes mean 'local' and 'remote' not
> 'left' and 'right'?

I know since the great bit shortage of '05, people have been using
obscure three variable names to conserve bits.  But these days, we can
resolve conflicts like this by just using variable names like
local_grant_table or left_grant_table ;-)

Regards,

Anthony Liguori

>
> David
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel