Re: [RFC PATCH] jbd2: avoid __GFP_ZERO with SLAB_TYPESAFE_BY_RCU

[Theodore Ts'o]

On Wed, Feb 09, 2022 at 09:43:57PM -0800, Paul E. McKenney wrote:
> So the typical RCU approach (not involving SLAB_TYPESAFE_BY_RCU)
> is to take the grace period at the time of the free.  This can be
> done synchronously using synchronize_rcu(), but is often instead done
> asynchronously using call_rcu() or kfree_rcu().  So in this case,
> you don't need synchronize_rcu() on allocation because the required
> grace period already happened at *free() time.
> 
> But there are a few situations where it makes sense to free blocks that
> readers might still be referencing.  Readers must then add validity
> checks to detect this case, and also prevent freeing, for example,
> using a per-block spinlock for synchronization.  For example, a reader
> might acquire a spinlock in the block to prevent changes, recheck the
> lookup key, and if the key does not match, release the lock and pretend
> not to have found the block.  If the key does match, anything attempting
> to delete and free the block will be spinning on that same spinlock.
> 
> And so if you specify SLAB_TYPESAFE_BY_RCU, the slab allocator is
> guaranteeing type safety to RCU readers instead of the usual existence
> guarantee.  A memory block might be freed out from under an RCU reader,
> but its type will remain the same.  This means that the grace period
> happens internally to the slab allocator when a slab is returned to
> the system.
> 
> So either the validation checks are quite novel, the kmem_cache_zalloc()
> calls should be replaced by kmem_cache_alloc() plus validation checks,
> or the SLAB_TYPESAFE_BY_RCU should be removed.
> 
> Just out of curiosity, what is your mental model of SLAB_TYPESAFE_BY_RCU?

Hmm, so the code in question the flag was called SLAB_DESTROY_BY_RCU
in June 205 by commit de92c8caf16c ("jbd2: speedup
jbd2_journal_get_[write|undo]_access()"), and it was written by Jan.
I don't see anything to make sure the jh doesn't get freed until after
the grace period, and so that looks like a problem unless I'm missing
something.   Jan, what do you think?

					- Ted