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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

> >>>>
> >>>> 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?

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