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;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Li zeming" <zeming@nfschina.com>
Cc:     bvanassche@acm.org, jlayton@kernel.org,
        linux-kernel@vger.kernel.org, reiserfs-devel@vger.kernel.org,
        song@kernel.org, willy@infradead.org
Subject: Re: [PATCH] reiserfs: journal: Increase jl pointer check
In-reply-to: <20221026014727.3020-1-zeming@nfschina.com>
References: <166673341340.7585.173987927705263434@noble.neil.brown.name>,
 <20221026014727.3020-1-zeming@nfschina.com>
Date:   Wed, 26 Oct 2022 14:05:11 +1100
Message-id: <166675351102.12462.1842756348742191725@noble.neil.brown.name>
Precedence: bulk

On Wed, 26 Oct 2022, Li zeming wrote:
> I'm sorry, I think __GFP_NOFAIL should be like this:
> 
> The __GFP_NOFAIL flag will cause memory to be allocated an infinite
>  number of times until the allocation is successful, but it is best to
>  use it only for very necessary code, and try not to use it.

I agree with that.  But you didn't try not to use it - you left it there
in the code.
Had you removed the flag as well, then your patch would have made a bit
more sense.  However you would then need to explain why it is safe to
return NULL from alloc_journal_list().  Are you sure callers don't try
to dereference the pointer?

> 
> I'm not sure alloc_journal_list function is a very important function
>  here. If it is, this patch ignores it and does not consider it anymore
>  __GFP_NOFAIL allocatiIon problem.
> 
> 

You are correct that the function isn't very important, but that is
because reiserfs itself isn't very important.
You might not have noticed, but in the reiserfs Kconfig file it says:

	  Reiserfs is deprecated and scheduled to be removed from the kernel
	  in 2025. If you are still using it, please migrate to another
	  filesystem or tell us your usecase for reiserfs.

So it will still be around for a few years, but there isn't much point
with small clean-ups like this as hardly anyone uses the code, and in a
few years it won't be in mainline Linux at all.

Thanks,
NeilBrown