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

[Konrad Rzeszutek Wilk]

> >>>>
> >>>> 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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/