Re: [PATCH 04/10] xen/blkfront: separate ring information to an new struct

[Bob Liu <bob.liu@oracle.com>]

On 02/21/2015 02:59 AM, Konrad Rzeszutek Wilk wrote:
>>>>>>
>>>>>> Agree, Life would be easier if we can remove the persistent feature.
> 
> ..snip..
>>>>>
>>>>> If Konrad/Bob agree I would like to send a patch to remove persistent
>>>>> grants and then have the multiqueue series rebased on top of that.
> 
> ..snip..
>>>>
>>>> I agree with this.
>>>>
>>>> I think we can get better  performance/scalability gains of with improvements
>>>> to grant table locking and TLB flush avoidance.
>>>>
>>>> David
>>>
>>> It doesn't change the fact that persistent grants (as well as the grant copy implementation we did for tapdisk3) were alternatives that allowed aggregate storage performance to increase drastically. Before committing to removing something that allow Xen users to scale their deployments, I think we need to revisit whether the recent improvements to the whole grant mechanisms (grant table locking, TLB flushing, batched calls, etc) are performing as we would (now) expect.
>>
>> The fact that this extension improved performance doesn't mean it's
>> right or desirable. So IMHO we should just remove it and take the
>> performance hit. Then we can figure ways to deal with the limitations
> 
> .. snip..
> 
> Removing code just because without a clear forward plan might lead to
> re-instating said code back again - if no forward plan has been achieved.
> 
> If the matter here is purely code complication I would stress that doing
> cleanups in code can simplify this - as in the code can do with some
> moving of the 'grant' ops (persistent or not) in a different file.
> 
> That ought to short-term remove the problems with the 'if (persistent_grant)'
> problem.
> 
> David assertion that better performance and scalbility can be gained
> with grant table locking and TLB flush avoidance is interesting - as
> 1). The grant locking is going in Xen 4.6 but not earlier - so when running
>     on older hypervisors this gives an performance benefit.
> 
> 2). I have not seen any prototype TLB flush avoidance code so not know
>     when that would be available.
> 
> Perhaps a better choice is to do the removal of the persistence support
> when the changes in Xen hypervisor are known?
> 

With patch: [PATCH v5 0/2] gnttab: Improve scaleability, I can get
nearly the same performance as without persistence support.

But I'm not sure about the benchmark described here:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/block/xen-blkfront.c?id=0a8704a51f386cab7394e38ff1d66eef924d8ab8

-- 
Regards,
-Bob