Re: [PATCH] slub: fix slab_pad_check()

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

On Thu, Sep 03, 2009 at 05:43:12PM -0500, Christoph Lameter wrote:
> On Thu, 3 Sep 2009, Paul E. McKenney wrote:
> 
> > 2.	CPU 0 discovers that the slab cache can now be destroyed.
> >
> > 	It determines that there are no users, and has guaranteed
> > 	that there will be no future users.  So it knows that it
> > 	can safely do kmem_cache_destroy().
> >
> > 3.	In absence of rcu_barrier(), kmem_cache_destroy() would
> > 	immediately tear down the slab data structures.
> 
> Of course. This has been discussed before.
> 
> You need to ensure that no objects are in use before destroying a slab. In
> case of DESTROY_BY_RCU you must ensure that there are no potential
> readers. So use a suitable rcu barrier or something else like a
> synchronize_rcu...

If by "you must ensure" you mean "kmem_cache_destroy must ensure", then
we are in complete agreement.  Otherwise, not a chance.

> > > But going through the RCU period is pointless since no user of the cache
> > > remains.
> >
> > Which is irrelevant.  The outstanding RCU callback was posted by the
> > slab cache itself, -not- by the user of the slab cache.
> 
> There will be no rcu callbacks generated at kmem_cache_destroy with the
> patch I posted.

That is true.  However, there well might be RCU callbacks generated by
kmem_cache_free() that have not yet been invoked.  Since kmem_cache_free()
generated them, it is ridiculous to insist that the user account for them.
That responsibility must instead fall on kmem_cache_destroy().

> > > The dismantling does not need RCU since there are no operations on the
> > > objects in progress. So simply switch DESTROY_BY_RCU off for close.
> >
> > Unless I am missing something, this patch re-introduces the bug that
> > the rcu_barrier() was added to prevent.  So, in absence of a better
> > explanation of what I am missing:
> 
> The "fix" was ill advised. Slab users must ensure that no objects are in
> use before destroying a slab. Only the slab users know how the objects
> are being used. The slab allocator itself cannot know how to ensure that
> there are no pending references. Putting a rcu_barrier in there creates an
> inconsistency in the operation of kmem_cache_destroy() and an expectation
> of functionality that the function cannot provide.

If by "must ensure that no objects are in use", you mean "must have
no further references to them", then we are in agreement.  And in my
scenario above, it is not the -user- who later references the memory,
but rather the slab code itself.

Put the rcu_barrier() in kmem_cache_free().  Please.

							Thanx, Paul