Messages in this thread | | | Date | Fri, 20 Feb 2015 13:59:37 -0500 | From | Konrad Rzeszutek Wilk <> | Subject | Re: [PATCH 04/10] xen/blkfront: separate ring information to an new struct |
| |
> >>>> > >>>> 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?
| |