Re: [PATCH v2 02/14] nfsd: replace fh_fill_both_attrs() with fh_fill_post_noop()

On Mon, 2026-07-06 at 08:19 +1000, NeilBrown wrote:
> From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> 
> fh_fill_both_attrs() is only needed for open/create and is used in the
> case when the target already existed so no creating happens.
> 
> As part of refactoring this code it is changed to call
> fh_fill_pre_attrs() once early on (so errors only need to be caught in
> one place) and then to use a new fh_fill_post_noop() when it is
> determined that no creation happened.
> 
> fh_fill_pre_attrs() now stores the attrs (which it had to get all of
> anyway)_ in ->fh_post_attr.  fh_fill_post_noop() simply marks them as
> valid.  fh_fill_post_attrs() replaces them.
> 
> This change involves moving fh_fill_pre_attrs() out of the inode_lock on
> the directory.  This means that we cannot provide "atomic" wcc data so a
> new fh_fill_pre_attrs_unlocked() is provided which marks the attrs as
> non-atomic.
> 
> This is unfortunate but inevitable if we are ever to allow concurrent
> updates in a directory (which can significantly improve performance in
> some cases).  To get atomic pre/post attributes we will need to be able
> to ask the fs to provide them, or to request a lease on the directory
> for the duration of an operation.
> 
> Note that we haven't provided pre/post attrs on WRITE requests for a
> long time for exactly this reason - we cannot lock the file to get them.
> 
> Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> ---
>  fs/nfsd/nfs4proc.c | 20 ++++++--------
>  fs/nfsd/nfsfh.c    | 69 +++++++++++++++++++++++-----------------------
>  fs/nfsd/nfsfh.h    | 14 +++++++++-
>  3 files changed, 56 insertions(+), 47 deletions(-)
> 

The loss of atomic post-op attrs is lamentable, but it is in service of
a higher cause!

Reviewed-by: Jeff Layton <[email protected]>