Re: [PATCH 04/11] nfs: combine NFS_LAYOUT_RETURN and NFS_LAYOUT_RETURN_LOCK

[Mike Snitzer <snitzer@kernel.org>]

On Fri, Dec 06, 2024 at 01:15:30PM +1100, NeilBrown wrote:
> The flags NFS_LAYOUT_RETURN and NFS_LAYOUT_RETURN_LOCK are effectively
> identical.
> The only time either are cleared is in pnfs_clear_layoutreturn_waitbit(),
> and there both are cleared.
> The only time NFS_LAYOUT_RETURN is set is in pnfs_prepare_layoutreturn()
> immediately after NFS_LAYOUT_RETURN_LOCK was set.
> The only other time that NFS_LAYOUT_RETURN_LOCK is set is in
> pnfs_mark_layout_stateid_invalid() if NFS_LAYOUT_RETURN was set but
> NFS_LAYOUT_RETURN_LOCK was not set - but that is an impossible
> combination given that else where the flags are set or cleared together.
> 
> So we only need one of these flags.  This patch discards
> NFS_LAYOUT_RETURN_LOCK and does the test_and_set needed for exclusion with
> NFS_LAYOUT_RETURN.
> 
> Also the wake_up_bit in pnfs_clear_layoutreturn_waitbit() is changed to
> clear_and_wake_up_bit() which includes all needed barriers internally.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

I appreciate that you've done a general audit of the NFS code and
looked to improve / optimize the wake_up_bit() callers, etc.  But how
did you test this specific patch's changes to the pnfs code?

Reason I ask is if you look at the commit that introduced
NFS_LAYOUT_RETURN_LOCK way back when:
6604b203fb63 pNFS: On error, do not send LAYOUTGET until the LAYOUTRETURN has completed

You'll see that, with your patch, you've seem to have now reverted the
code back to before stable@ commit 6604b203fb63 was applied.

Now there may be merit to doing that due to other changes in the pnfs
code that didn't exist back then but... your changes look suspicious
given the evolution of this code.

Mike