Re: [PATCH 00/13] Re: Scalability requirements for sysv ipc

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Mon, Apr 14, 2008 at 10:33:23AM +0200, Nadia Derbey wrote:
> Peter Zijlstra wrote:
> >On Mon, 2008-04-14 at 07:18 +0200, Nadia Derbey wrote:
> >
> >>Peter Zijlstra wrote:
> >>
> >>>On Fri, 2008-04-11 at 18:17 +0200, Nadia.Derbey@bull.net wrote:
> >>>
> >>>
> >>>>Here is finally the ipc ridr-based implementation I was talking about 
> >>>>last
> >>>>week (see http://lkml.org/lkml/2008/4/4/208).
> >>>>I couldn't avoid much of the code duplication, but at least made things
> >>>>incremental.
> >>>>
> >>>>Does somebody now a test suite that exists for the idr API, that I could
> >>>>run on this new api?
> >>>>
> >>>>Mike, can you try to run it on your victim: I had such a hard time 
> >>>>building
> >>>>this patch, that I couldn't re-run the test on my 8-core with this new
> >>>>version. So the last results I have are for 2.6.25-rc3-mm1.
> >>>>
> >>>>Also, I think a careful review should be done to avoid introducing yet 
> >>>>other
> >>>>problems :-(
> >>>
> >>>
> >>>Why duplicate the whole thing, when we converted the Radix tree to be
> >>>RCU safe we did it in-place. Is there a reason this is not done for idr?
> >>>
> >>>
> >>>
> >>
> >>I did that because I wanted to go fast and try to fix the performance 
> >>problem we have with sysV ipc's. I didn't want to introduce (yet other) 
> >>regressions in the code that uses idr's today and that works well ;-)
> >>May be in the future if this rcu based api appears to be ok, we can 
> >>replace one with the other?
> >
> >
> >>From what I can see the API doesn't change at all,
> 
> Well, 1 interface changes, 1 is added and another one went away:
> 
> 1) for the preload part (it becomes like the radix-tree preload part):
> 
> int idr_pre_get(struct idr *, gfp_t);
> would become
> int idr_pre_get(gfp_t);
> 
> 2) idr_pre_get_end() is added (same as radix_tree_preload_end()).
> 
> 3) The idr_init() disappears.
> 
> You might see that other interfaces are not provided by ridr, but this 
> is only because I've taken those that are useful for the ipc part (so 
> should not be a problem to make the whole thing rcu safe).

Part of this is because you need to allow the caller to choose the
locking for updates.  Mightn't it be better to have both styles of
API, and share the bit-twiddling and tree-walking code?

> >so I don't see why
> >you need to duplicate - either the new code works as expected or its
> >broken.
> 
> That's why I asked for an "IDR test suite": I wanted to test potential 
> regressions.

That would be good -- certainly the #ifdef TEST leads one to believe
that such a test suite existed at some time.  ;-)

							Thanx, Paul

> >If it works its good enough for all IDR users, if its broken we
> >should fix it. Seems simple enough.. am I missing something obvious?
> >
> 
> Regards,
> Nadia
> 
> 
> 
>