Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Feb 2022 10:54:32 -0500
From:   "Theodore Ts'o" <tytso@mit.edu>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Cc:     Jan Kara <jack@suse.cz>, Qian Cai <quic_qiancai@quicinc.com>,
        Jan Kara <jack@suse.com>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Boqun Feng <boqun.feng@gmail.com>, linux-ext4@vger.kernel.org,
        rcu@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] jbd2: avoid __GFP_ZERO with SLAB_TYPESAFE_BY_RCU
Message-ID: <YgU1ONDsn9xgs/CG@mit.edu>
References: <20220209165742.5659-1-quic_qiancai@quicinc.com>
 <20220209181010.gfn66rvip56i54df@quack3.lan>
 <20220209201137.GY4285@paulmck-ThinkPad-P17-Gen-1>
 <YgSdlew4B5FWY6Qm@mit.edu>
 <20220210054357.GH4285@paulmck-ThinkPad-P17-Gen-1>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220210054357.GH4285@paulmck-ThinkPad-P17-Gen-1>
Precedence: bulk

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