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

[Paul E. McKenney]

On Wed, Feb 09, 2022 at 07:10:10PM +0100, Jan Kara wrote:
> On Wed 09-02-22 11:57:42, Qian Cai wrote:
> > Since the linux-next commit 120aa5e57479 (mm: Check for
> > SLAB_TYPESAFE_BY_RCU and __GFP_ZERO slab allocation), we will get a
> > boot warning. Avoid it by calling synchronize_rcu() before the zeroing.
> > 
> > Signed-off-by: Qian Cai <quic_qiancai@xxxxxxxxxxx>
> 
> No, the performance impact of this would be just horrible. Can you
> ellaborate a bit why SLAB_TYPESAFE_BY_RCU + __GFP_ZERO is a problem and why
> synchronize_rcu() would be needed here before the memset() please? I mean
> how is zeroing here any different from the memory just being used?

Suppose a reader picks up a pointer to a memory block, then that memory
is freed.  No problem, given that this is a SLAB_TYPESAFE_BY_RCU slab,
so the memory won't be freed while the reader is accessing it.  But while
the reader is in the process of validating the block, it is zeroed.

How does the validation step handle this in all cases?

If you have a way of handling this, I will of course drop the patch.
And learn something new, which is always a good thing.  ;-)

							Thanx, Paul

> > ---
> >  fs/jbd2/journal.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > index c2cf74b01ddb..323112de5921 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -2861,15 +2861,18 @@ static struct journal_head *journal_alloc_journal_head(void)
> >  #ifdef CONFIG_JBD2_DEBUG
> >  	atomic_inc(&nr_journal_heads);
> >  #endif
> > -	ret = kmem_cache_zalloc(jbd2_journal_head_cache, GFP_NOFS);
> > +	ret = kmem_cache_alloc(jbd2_journal_head_cache, GFP_NOFS);
> >  	if (!ret) {
> >  		jbd_debug(1, "out of memory for journal_head\n");
> >  		pr_notice_ratelimited("ENOMEM in %s, retrying.\n", __func__);
> > -		ret = kmem_cache_zalloc(jbd2_journal_head_cache,
> > +		ret = kmem_cache_alloc(jbd2_journal_head_cache,
> >  				GFP_NOFS | __GFP_NOFAIL);
> >  	}
> > -	if (ret)
> > +	if (ret) {
> > +		synchronize_rcu();
> > +		memset(ret, 0, sizeof(*ret));
> >  		spin_lock_init(&ret->b_state_lock);
> > +	}
> >  	return ret;
> >  }
> >  
> > -- 
> > 2.30.2
> > 
> -- 
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR