diff mbox series

[v5,2/5] nfsd: reorganize filecache.c

Message ID 20221101144647.136696-3-jlayton@kernel.org (mailing list archive)
State New, archived
Headers show
Series nfsd: clean up refcounting in the filecache | expand

Commit Message

Jeffrey Layton Nov. 1, 2022, 2:46 p.m. UTC 
In a coming patch, we're going to rework how the filecache refcounting
works. Move some code around in the function to reduce the churn in the
later patches, and rename some of the functions with (hopefully) clearer
names. This should introduce no functional changes.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/filecache.c | 135 ++++++++++++++++++++++----------------------
 fs/nfsd/trace.h     |   4 +-
 2 files changed, 70 insertions(+), 69 deletions(-)

Comments

NeilBrown Nov. 1, 2022, 8:59 p.m. UTC | #1 
On Wed, 02 Nov 2022, Jeff Layton wrote:
> In a coming patch, we're going to rework how the filecache refcounting
> works. Move some code around in the function to reduce the churn in the
> later patches, and rename some of the functions with (hopefully) clearer
> names. This should introduce no functional changes.

Just for future reference, it can help to list here which functions
changed names and what the new names are.  It makes review that little
bit easier.
I think:
   nfsd_file_flush -> nfsd_file_fsync
   nfsd_file_unhash_and_dispose -> nfsd_file_unhash_and_queue

That patch make it look like
   nfsd_file_unhash -> nfsd_file_get
   nfsd_file_close_inode_sync -> nfsd_file_close_inode
   nfsd_file_close_inode -> nfsd_file_close_inode_sync
as well, but there the content of the functions also change
so one concludes that you just moved functions, and diff sees the '{'
and '}' and blank lines are common and aligns on them...

Why did you just swap the order of those last two ??

You also moved a tracepoint from nfsd_file_put_final to nfsd_file_free,
but didn't mention it in the commit comment (is that being too picky?).

But allowing for all that - the patch looks ok.

Reviewed-by: NeilBrown <neilb@suse.de>

Thanks,
NeilBrown


> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/filecache.c | 135 ++++++++++++++++++++++----------------------
>  fs/nfsd/trace.h     |   4 +-
>  2 files changed, 70 insertions(+), 69 deletions(-)
> 
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index 90e62042d6d6..0bf3727455e2 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -311,16 +311,59 @@ nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may)
>  	return nf;
>  }
>  
> +static void
> +nfsd_file_fsync(struct nfsd_file *nf)
> +{
> +	struct file *file = nf->nf_file;
> +
> +	if (!file || !(file->f_mode & FMODE_WRITE))
> +		return;
> +	if (vfs_fsync(file, 1) != 0)
> +		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> +}
> +
> +static int
> +nfsd_file_check_write_error(struct nfsd_file *nf)
> +{
> +	struct file *file = nf->nf_file;
> +
> +	if (!file || !(file->f_mode & FMODE_WRITE))
> +		return 0;
> +	return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err));
> +}
> +
> +static void
> +nfsd_file_hash_remove(struct nfsd_file *nf)
> +{
> +	trace_nfsd_file_unhash(nf);
> +
> +	if (nfsd_file_check_write_error(nf))
> +		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> +	rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash,
> +			       nfsd_file_rhash_params);
> +}
> +
> +static bool
> +nfsd_file_unhash(struct nfsd_file *nf)
> +{
> +	if (test_and_clear_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> +		nfsd_file_hash_remove(nf);
> +		return true;
> +	}
> +	return false;
> +}
> +
>  static bool
>  nfsd_file_free(struct nfsd_file *nf)
>  {
>  	s64 age = ktime_to_ms(ktime_sub(ktime_get(), nf->nf_birthtime));
>  	bool flush = false;
>  
> +	trace_nfsd_file_free(nf);
> +
>  	this_cpu_inc(nfsd_file_releases);
>  	this_cpu_add(nfsd_file_total_age, age);
>  
> -	trace_nfsd_file_put_final(nf);
>  	if (nf->nf_mark)
>  		nfsd_file_mark_put(nf->nf_mark);
>  	if (nf->nf_file) {
> @@ -354,27 +397,6 @@ nfsd_file_check_writeback(struct nfsd_file *nf)
>  		mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
>  }
>  
> -static int
> -nfsd_file_check_write_error(struct nfsd_file *nf)
> -{
> -	struct file *file = nf->nf_file;
> -
> -	if (!file || !(file->f_mode & FMODE_WRITE))
> -		return 0;
> -	return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err));
> -}
> -
> -static void
> -nfsd_file_flush(struct nfsd_file *nf)
> -{
> -	struct file *file = nf->nf_file;
> -
> -	if (!file || !(file->f_mode & FMODE_WRITE))
> -		return;
> -	if (vfs_fsync(file, 1) != 0)
> -		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> -}
> -
>  static void nfsd_file_lru_add(struct nfsd_file *nf)
>  {
>  	set_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
> @@ -388,31 +410,18 @@ static void nfsd_file_lru_remove(struct nfsd_file *nf)
>  		trace_nfsd_file_lru_del(nf);
>  }
>  
> -static void
> -nfsd_file_hash_remove(struct nfsd_file *nf)
> -{
> -	trace_nfsd_file_unhash(nf);
> -
> -	if (nfsd_file_check_write_error(nf))
> -		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> -	rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash,
> -			       nfsd_file_rhash_params);
> -}
> -
> -static bool
> -nfsd_file_unhash(struct nfsd_file *nf)
> +struct nfsd_file *
> +nfsd_file_get(struct nfsd_file *nf)
>  {
> -	if (test_and_clear_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> -		nfsd_file_hash_remove(nf);
> -		return true;
> -	}
> -	return false;
> +	if (likely(refcount_inc_not_zero(&nf->nf_ref)))
> +		return nf;
> +	return NULL;
>  }
>  
>  static void
> -nfsd_file_unhash_and_dispose(struct nfsd_file *nf, struct list_head *dispose)
> +nfsd_file_unhash_and_queue(struct nfsd_file *nf, struct list_head *dispose)
>  {
> -	trace_nfsd_file_unhash_and_dispose(nf);
> +	trace_nfsd_file_unhash_and_queue(nf);
>  	if (nfsd_file_unhash(nf)) {
>  		/* caller must call nfsd_file_dispose_list() later */
>  		nfsd_file_lru_remove(nf);
> @@ -450,7 +459,7 @@ nfsd_file_put(struct nfsd_file *nf)
>  		nfsd_file_unhash_and_put(nf);
>  
>  	if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> -		nfsd_file_flush(nf);
> +		nfsd_file_fsync(nf);
>  		nfsd_file_put_noref(nf);
>  	} else if (nf->nf_file && test_bit(NFSD_FILE_GC, &nf->nf_flags)) {
>  		nfsd_file_put_noref(nf);
> @@ -459,14 +468,6 @@ nfsd_file_put(struct nfsd_file *nf)
>  		nfsd_file_put_noref(nf);
>  }
>  
> -struct nfsd_file *
> -nfsd_file_get(struct nfsd_file *nf)
> -{
> -	if (likely(refcount_inc_not_zero(&nf->nf_ref)))
> -		return nf;
> -	return NULL;
> -}
> -
>  static void
>  nfsd_file_dispose_list(struct list_head *dispose)
>  {
> @@ -475,7 +476,7 @@ nfsd_file_dispose_list(struct list_head *dispose)
>  	while(!list_empty(dispose)) {
>  		nf = list_first_entry(dispose, struct nfsd_file, nf_lru);
>  		list_del_init(&nf->nf_lru);
> -		nfsd_file_flush(nf);
> +		nfsd_file_fsync(nf);
>  		nfsd_file_put_noref(nf);
>  	}
>  }
> @@ -489,7 +490,7 @@ nfsd_file_dispose_list_sync(struct list_head *dispose)
>  	while(!list_empty(dispose)) {
>  		nf = list_first_entry(dispose, struct nfsd_file, nf_lru);
>  		list_del_init(&nf->nf_lru);
> -		nfsd_file_flush(nf);
> +		nfsd_file_fsync(nf);
>  		if (!refcount_dec_and_test(&nf->nf_ref))
>  			continue;
>  		if (nfsd_file_free(nf))
> @@ -689,7 +690,7 @@ __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
>  				       nfsd_file_rhash_params);
>  		if (!nf)
>  			break;
> -		nfsd_file_unhash_and_dispose(nf, dispose);
> +		nfsd_file_unhash_and_queue(nf, dispose);
>  		count++;
>  	} while (1);
>  	rcu_read_unlock();
> @@ -697,37 +698,37 @@ __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
>  }
>  
>  /**
> - * nfsd_file_close_inode_sync - attempt to forcibly close a nfsd_file
> + * nfsd_file_close_inode - attempt a delayed close of a nfsd_file
>   * @inode: inode of the file to attempt to remove
>   *
> - * Unhash and put, then flush and fput all cache items associated with @inode.
> + * Unhash and put all cache item associated with @inode.
>   */
> -void
> -nfsd_file_close_inode_sync(struct inode *inode)
> +static void
> +nfsd_file_close_inode(struct inode *inode)
>  {
>  	LIST_HEAD(dispose);
>  	unsigned int count;
>  
>  	count = __nfsd_file_close_inode(inode, &dispose);
> -	trace_nfsd_file_close_inode_sync(inode, count);
> -	nfsd_file_dispose_list_sync(&dispose);
> +	trace_nfsd_file_close_inode(inode, count);
> +	nfsd_file_dispose_list_delayed(&dispose);
>  }
>  
>  /**
> - * nfsd_file_close_inode - attempt a delayed close of a nfsd_file
> + * nfsd_file_close_inode_sync - attempt to forcibly close a nfsd_file
>   * @inode: inode of the file to attempt to remove
>   *
> - * Unhash and put all cache item associated with @inode.
> + * Unhash and put, then flush and fput all cache items associated with @inode.
>   */
> -static void
> -nfsd_file_close_inode(struct inode *inode)
> +void
> +nfsd_file_close_inode_sync(struct inode *inode)
>  {
>  	LIST_HEAD(dispose);
>  	unsigned int count;
>  
>  	count = __nfsd_file_close_inode(inode, &dispose);
> -	trace_nfsd_file_close_inode(inode, count);
> -	nfsd_file_dispose_list_delayed(&dispose);
> +	trace_nfsd_file_close_inode_sync(inode, count);
> +	nfsd_file_dispose_list_sync(&dispose);
>  }
>  
>  /**
> @@ -891,7 +892,7 @@ __nfsd_file_cache_purge(struct net *net)
>  		nf = rhashtable_walk_next(&iter);
>  		while (!IS_ERR_OR_NULL(nf)) {
>  			if (!net || nf->nf_net == net)
> -				nfsd_file_unhash_and_dispose(nf, &dispose);
> +				nfsd_file_unhash_and_queue(nf, &dispose);
>  			nf = rhashtable_walk_next(&iter);
>  		}
>  
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index b09ab4f92d43..940252482fd4 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -903,10 +903,10 @@ DEFINE_EVENT(nfsd_file_class, name, \
>  	TP_PROTO(struct nfsd_file *nf), \
>  	TP_ARGS(nf))
>  
> -DEFINE_NFSD_FILE_EVENT(nfsd_file_put_final);
> +DEFINE_NFSD_FILE_EVENT(nfsd_file_free);
>  DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash);
>  DEFINE_NFSD_FILE_EVENT(nfsd_file_put);
> -DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash_and_dispose);
> +DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash_and_queue);
>  
>  TRACE_EVENT(nfsd_file_alloc,
>  	TP_PROTO(
> -- 
> 2.38.1
> 
>
Jeffrey Layton Nov. 1, 2022, 9:04 p.m. UTC | #2 
On Wed, 2022-11-02 at 07:59 +1100, NeilBrown wrote:
> On Wed, 02 Nov 2022, Jeff Layton wrote:
> > In a coming patch, we're going to rework how the filecache refcounting
> > works. Move some code around in the function to reduce the churn in the
> > later patches, and rename some of the functions with (hopefully) clearer
> > names. This should introduce no functional changes.
> 
> Just for future reference, it can help to list here which functions
> changed names and what the new names are.  It makes review that little
> bit easier.
> I think:
>    nfsd_file_flush -> nfsd_file_fsync
>    nfsd_file_unhash_and_dispose -> nfsd_file_unhash_and_queue
> 

Yep. I think those names are more descriptive. I'll add some more info
to the changelog for the next posting.

> That patch make it look like
>    nfsd_file_unhash -> nfsd_file_get
>    nfsd_file_close_inode_sync -> nfsd_file_close_inode
>    nfsd_file_close_inode -> nfsd_file_close_inode_sync
> as well, but there the content of the functions also change
> so one concludes that you just moved functions, and diff sees the '{'
> and '}' and blank lines are common and aligns on them...
> 
> Why did you just swap the order of those last two ??

Yes. In a later patch, nfsd_file_close_inode_sync calls
nfsd_file_close_inode, so it made sense to swap them.
> 
> You also moved a tracepoint from nfsd_file_put_final to nfsd_file_free,
> but didn't mention it in the commit comment (is that being too picky?).
> 

There is no nfsd_file_put_final. The nfsd_file_put_final tracepoint is
in nfsd_file_free. This patch just renames it to something more suitable
(and moves it up a little in the function).

> But allowing for all that - the patch looks ok.
> 
> Reviewed-by: NeilBrown <neilb@suse.de>
> 

Thanks!

> Thanks,
> NeilBrown
> 
> 
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/nfsd/filecache.c | 135 ++++++++++++++++++++++----------------------
> >  fs/nfsd/trace.h     |   4 +-
> >  2 files changed, 70 insertions(+), 69 deletions(-)
> > 
> > diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> > index 90e62042d6d6..0bf3727455e2 100644
> > --- a/fs/nfsd/filecache.c
> > +++ b/fs/nfsd/filecache.c
> > @@ -311,16 +311,59 @@ nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may)
> >  	return nf;
> >  }
> >  
> > +static void
> > +nfsd_file_fsync(struct nfsd_file *nf)
> > +{
> > +	struct file *file = nf->nf_file;
> > +
> > +	if (!file || !(file->f_mode & FMODE_WRITE))
> > +		return;
> > +	if (vfs_fsync(file, 1) != 0)
> > +		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> > +}
> > +
> > +static int
> > +nfsd_file_check_write_error(struct nfsd_file *nf)
> > +{
> > +	struct file *file = nf->nf_file;
> > +
> > +	if (!file || !(file->f_mode & FMODE_WRITE))
> > +		return 0;
> > +	return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err));
> > +}
> > +
> > +static void
> > +nfsd_file_hash_remove(struct nfsd_file *nf)
> > +{
> > +	trace_nfsd_file_unhash(nf);
> > +
> > +	if (nfsd_file_check_write_error(nf))
> > +		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> > +	rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash,
> > +			       nfsd_file_rhash_params);
> > +}
> > +
> > +static bool
> > +nfsd_file_unhash(struct nfsd_file *nf)
> > +{
> > +	if (test_and_clear_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> > +		nfsd_file_hash_remove(nf);
> > +		return true;
> > +	}
> > +	return false;
> > +}
> > +
> >  static bool
> >  nfsd_file_free(struct nfsd_file *nf)
> >  {
> >  	s64 age = ktime_to_ms(ktime_sub(ktime_get(), nf->nf_birthtime));
> >  	bool flush = false;
> >  
> > +	trace_nfsd_file_free(nf);
> > +
> >  	this_cpu_inc(nfsd_file_releases);
> >  	this_cpu_add(nfsd_file_total_age, age);
> >  
> > -	trace_nfsd_file_put_final(nf);
> >  	if (nf->nf_mark)
> >  		nfsd_file_mark_put(nf->nf_mark);
> >  	if (nf->nf_file) {
> > @@ -354,27 +397,6 @@ nfsd_file_check_writeback(struct nfsd_file *nf)
> >  		mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
> >  }
> >  
> > -static int
> > -nfsd_file_check_write_error(struct nfsd_file *nf)
> > -{
> > -	struct file *file = nf->nf_file;
> > -
> > -	if (!file || !(file->f_mode & FMODE_WRITE))
> > -		return 0;
> > -	return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err));
> > -}
> > -
> > -static void
> > -nfsd_file_flush(struct nfsd_file *nf)
> > -{
> > -	struct file *file = nf->nf_file;
> > -
> > -	if (!file || !(file->f_mode & FMODE_WRITE))
> > -		return;
> > -	if (vfs_fsync(file, 1) != 0)
> > -		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> > -}
> > -
> >  static void nfsd_file_lru_add(struct nfsd_file *nf)
> >  {
> >  	set_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
> > @@ -388,31 +410,18 @@ static void nfsd_file_lru_remove(struct nfsd_file *nf)
> >  		trace_nfsd_file_lru_del(nf);
> >  }
> >  
> > -static void
> > -nfsd_file_hash_remove(struct nfsd_file *nf)
> > -{
> > -	trace_nfsd_file_unhash(nf);
> > -
> > -	if (nfsd_file_check_write_error(nf))
> > -		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
> > -	rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash,
> > -			       nfsd_file_rhash_params);
> > -}
> > -
> > -static bool
> > -nfsd_file_unhash(struct nfsd_file *nf)
> > +struct nfsd_file *
> > +nfsd_file_get(struct nfsd_file *nf)
> >  {
> > -	if (test_and_clear_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> > -		nfsd_file_hash_remove(nf);
> > -		return true;
> > -	}
> > -	return false;
> > +	if (likely(refcount_inc_not_zero(&nf->nf_ref)))
> > +		return nf;
> > +	return NULL;
> >  }
> >  
> >  static void
> > -nfsd_file_unhash_and_dispose(struct nfsd_file *nf, struct list_head *dispose)
> > +nfsd_file_unhash_and_queue(struct nfsd_file *nf, struct list_head *dispose)
> >  {
> > -	trace_nfsd_file_unhash_and_dispose(nf);
> > +	trace_nfsd_file_unhash_and_queue(nf);
> >  	if (nfsd_file_unhash(nf)) {
> >  		/* caller must call nfsd_file_dispose_list() later */
> >  		nfsd_file_lru_remove(nf);
> > @@ -450,7 +459,7 @@ nfsd_file_put(struct nfsd_file *nf)
> >  		nfsd_file_unhash_and_put(nf);
> >  
> >  	if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> > -		nfsd_file_flush(nf);
> > +		nfsd_file_fsync(nf);
> >  		nfsd_file_put_noref(nf);
> >  	} else if (nf->nf_file && test_bit(NFSD_FILE_GC, &nf->nf_flags)) {
> >  		nfsd_file_put_noref(nf);
> > @@ -459,14 +468,6 @@ nfsd_file_put(struct nfsd_file *nf)
> >  		nfsd_file_put_noref(nf);
> >  }
> >  
> > -struct nfsd_file *
> > -nfsd_file_get(struct nfsd_file *nf)
> > -{
> > -	if (likely(refcount_inc_not_zero(&nf->nf_ref)))
> > -		return nf;
> > -	return NULL;
> > -}
> > -
> >  static void
> >  nfsd_file_dispose_list(struct list_head *dispose)
> >  {
> > @@ -475,7 +476,7 @@ nfsd_file_dispose_list(struct list_head *dispose)
> >  	while(!list_empty(dispose)) {
> >  		nf = list_first_entry(dispose, struct nfsd_file, nf_lru);
> >  		list_del_init(&nf->nf_lru);
> > -		nfsd_file_flush(nf);
> > +		nfsd_file_fsync(nf);
> >  		nfsd_file_put_noref(nf);
> >  	}
> >  }
> > @@ -489,7 +490,7 @@ nfsd_file_dispose_list_sync(struct list_head *dispose)
> >  	while(!list_empty(dispose)) {
> >  		nf = list_first_entry(dispose, struct nfsd_file, nf_lru);
> >  		list_del_init(&nf->nf_lru);
> > -		nfsd_file_flush(nf);
> > +		nfsd_file_fsync(nf);
> >  		if (!refcount_dec_and_test(&nf->nf_ref))
> >  			continue;
> >  		if (nfsd_file_free(nf))
> > @@ -689,7 +690,7 @@ __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
> >  				       nfsd_file_rhash_params);
> >  		if (!nf)
> >  			break;
> > -		nfsd_file_unhash_and_dispose(nf, dispose);
> > +		nfsd_file_unhash_and_queue(nf, dispose);
> >  		count++;
> >  	} while (1);
> >  	rcu_read_unlock();
> > @@ -697,37 +698,37 @@ __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
> >  }
> >  
> >  /**
> > - * nfsd_file_close_inode_sync - attempt to forcibly close a nfsd_file
> > + * nfsd_file_close_inode - attempt a delayed close of a nfsd_file
> >   * @inode: inode of the file to attempt to remove
> >   *
> > - * Unhash and put, then flush and fput all cache items associated with @inode.
> > + * Unhash and put all cache item associated with @inode.
> >   */
> > -void
> > -nfsd_file_close_inode_sync(struct inode *inode)
> > +static void
> > +nfsd_file_close_inode(struct inode *inode)
> >  {
> >  	LIST_HEAD(dispose);
> >  	unsigned int count;
> >  
> >  	count = __nfsd_file_close_inode(inode, &dispose);
> > -	trace_nfsd_file_close_inode_sync(inode, count);
> > -	nfsd_file_dispose_list_sync(&dispose);
> > +	trace_nfsd_file_close_inode(inode, count);
> > +	nfsd_file_dispose_list_delayed(&dispose);
> >  }
> >  
> >  /**
> > - * nfsd_file_close_inode - attempt a delayed close of a nfsd_file
> > + * nfsd_file_close_inode_sync - attempt to forcibly close a nfsd_file
> >   * @inode: inode of the file to attempt to remove
> >   *
> > - * Unhash and put all cache item associated with @inode.
> > + * Unhash and put, then flush and fput all cache items associated with @inode.
> >   */
> > -static void
> > -nfsd_file_close_inode(struct inode *inode)
> > +void
> > +nfsd_file_close_inode_sync(struct inode *inode)
> >  {
> >  	LIST_HEAD(dispose);
> >  	unsigned int count;
> >  
> >  	count = __nfsd_file_close_inode(inode, &dispose);
> > -	trace_nfsd_file_close_inode(inode, count);
> > -	nfsd_file_dispose_list_delayed(&dispose);
> > +	trace_nfsd_file_close_inode_sync(inode, count);
> > +	nfsd_file_dispose_list_sync(&dispose);
> >  }
> >  
> >  /**
> > @@ -891,7 +892,7 @@ __nfsd_file_cache_purge(struct net *net)
> >  		nf = rhashtable_walk_next(&iter);
> >  		while (!IS_ERR_OR_NULL(nf)) {
> >  			if (!net || nf->nf_net == net)
> > -				nfsd_file_unhash_and_dispose(nf, &dispose);
> > +				nfsd_file_unhash_and_queue(nf, &dispose);
> >  			nf = rhashtable_walk_next(&iter);
> >  		}
> >  
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index b09ab4f92d43..940252482fd4 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -903,10 +903,10 @@ DEFINE_EVENT(nfsd_file_class, name, \
> >  	TP_PROTO(struct nfsd_file *nf), \
> >  	TP_ARGS(nf))
> >  
> > -DEFINE_NFSD_FILE_EVENT(nfsd_file_put_final);
> > +DEFINE_NFSD_FILE_EVENT(nfsd_file_free);
> >  DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash);
> >  DEFINE_NFSD_FILE_EVENT(nfsd_file_put);
> > -DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash_and_dispose);
> > +DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash_and_queue);
> >  
> >  TRACE_EVENT(nfsd_file_alloc,
> >  	TP_PROTO(
> > -- 
> > 2.38.1
> > 
> >
diff mbox series

Patch

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 90e62042d6d6..0bf3727455e2 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -311,16 +311,59 @@  nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may)
 	return nf;
 }
 
+static void
+nfsd_file_fsync(struct nfsd_file *nf)
+{
+	struct file *file = nf->nf_file;
+
+	if (!file || !(file->f_mode & FMODE_WRITE))
+		return;
+	if (vfs_fsync(file, 1) != 0)
+		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
+}
+
+static int
+nfsd_file_check_write_error(struct nfsd_file *nf)
+{
+	struct file *file = nf->nf_file;
+
+	if (!file || !(file->f_mode & FMODE_WRITE))
+		return 0;
+	return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err));
+}
+
+static void
+nfsd_file_hash_remove(struct nfsd_file *nf)
+{
+	trace_nfsd_file_unhash(nf);
+
+	if (nfsd_file_check_write_error(nf))
+		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
+	rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash,
+			       nfsd_file_rhash_params);
+}
+
+static bool
+nfsd_file_unhash(struct nfsd_file *nf)
+{
+	if (test_and_clear_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
+		nfsd_file_hash_remove(nf);
+		return true;
+	}
+	return false;
+}
+
 static bool
 nfsd_file_free(struct nfsd_file *nf)
 {
 	s64 age = ktime_to_ms(ktime_sub(ktime_get(), nf->nf_birthtime));
 	bool flush = false;
 
+	trace_nfsd_file_free(nf);
+
 	this_cpu_inc(nfsd_file_releases);
 	this_cpu_add(nfsd_file_total_age, age);
 
-	trace_nfsd_file_put_final(nf);
 	if (nf->nf_mark)
 		nfsd_file_mark_put(nf->nf_mark);
 	if (nf->nf_file) {
@@ -354,27 +397,6 @@  nfsd_file_check_writeback(struct nfsd_file *nf)
 		mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
 }
 
-static int
-nfsd_file_check_write_error(struct nfsd_file *nf)
-{
-	struct file *file = nf->nf_file;
-
-	if (!file || !(file->f_mode & FMODE_WRITE))
-		return 0;
-	return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err));
-}
-
-static void
-nfsd_file_flush(struct nfsd_file *nf)
-{
-	struct file *file = nf->nf_file;
-
-	if (!file || !(file->f_mode & FMODE_WRITE))
-		return;
-	if (vfs_fsync(file, 1) != 0)
-		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
-}
-
 static void nfsd_file_lru_add(struct nfsd_file *nf)
 {
 	set_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
@@ -388,31 +410,18 @@  static void nfsd_file_lru_remove(struct nfsd_file *nf)
 		trace_nfsd_file_lru_del(nf);
 }
 
-static void
-nfsd_file_hash_remove(struct nfsd_file *nf)
-{
-	trace_nfsd_file_unhash(nf);
-
-	if (nfsd_file_check_write_error(nf))
-		nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id));
-	rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash,
-			       nfsd_file_rhash_params);
-}
-
-static bool
-nfsd_file_unhash(struct nfsd_file *nf)
+struct nfsd_file *
+nfsd_file_get(struct nfsd_file *nf)
 {
-	if (test_and_clear_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
-		nfsd_file_hash_remove(nf);
-		return true;
-	}
-	return false;
+	if (likely(refcount_inc_not_zero(&nf->nf_ref)))
+		return nf;
+	return NULL;
 }
 
 static void
-nfsd_file_unhash_and_dispose(struct nfsd_file *nf, struct list_head *dispose)
+nfsd_file_unhash_and_queue(struct nfsd_file *nf, struct list_head *dispose)
 {
-	trace_nfsd_file_unhash_and_dispose(nf);
+	trace_nfsd_file_unhash_and_queue(nf);
 	if (nfsd_file_unhash(nf)) {
 		/* caller must call nfsd_file_dispose_list() later */
 		nfsd_file_lru_remove(nf);
@@ -450,7 +459,7 @@  nfsd_file_put(struct nfsd_file *nf)
 		nfsd_file_unhash_and_put(nf);
 
 	if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
-		nfsd_file_flush(nf);
+		nfsd_file_fsync(nf);
 		nfsd_file_put_noref(nf);
 	} else if (nf->nf_file && test_bit(NFSD_FILE_GC, &nf->nf_flags)) {
 		nfsd_file_put_noref(nf);
@@ -459,14 +468,6 @@  nfsd_file_put(struct nfsd_file *nf)
 		nfsd_file_put_noref(nf);
 }
 
-struct nfsd_file *
-nfsd_file_get(struct nfsd_file *nf)
-{
-	if (likely(refcount_inc_not_zero(&nf->nf_ref)))
-		return nf;
-	return NULL;
-}
-
 static void
 nfsd_file_dispose_list(struct list_head *dispose)
 {
@@ -475,7 +476,7 @@  nfsd_file_dispose_list(struct list_head *dispose)
 	while(!list_empty(dispose)) {
 		nf = list_first_entry(dispose, struct nfsd_file, nf_lru);
 		list_del_init(&nf->nf_lru);
-		nfsd_file_flush(nf);
+		nfsd_file_fsync(nf);
 		nfsd_file_put_noref(nf);
 	}
 }
@@ -489,7 +490,7 @@  nfsd_file_dispose_list_sync(struct list_head *dispose)
 	while(!list_empty(dispose)) {
 		nf = list_first_entry(dispose, struct nfsd_file, nf_lru);
 		list_del_init(&nf->nf_lru);
-		nfsd_file_flush(nf);
+		nfsd_file_fsync(nf);
 		if (!refcount_dec_and_test(&nf->nf_ref))
 			continue;
 		if (nfsd_file_free(nf))
@@ -689,7 +690,7 @@  __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
 				       nfsd_file_rhash_params);
 		if (!nf)
 			break;
-		nfsd_file_unhash_and_dispose(nf, dispose);
+		nfsd_file_unhash_and_queue(nf, dispose);
 		count++;
 	} while (1);
 	rcu_read_unlock();
@@ -697,37 +698,37 @@  __nfsd_file_close_inode(struct inode *inode, struct list_head *dispose)
 }
 
 /**
- * nfsd_file_close_inode_sync - attempt to forcibly close a nfsd_file
+ * nfsd_file_close_inode - attempt a delayed close of a nfsd_file
  * @inode: inode of the file to attempt to remove
  *
- * Unhash and put, then flush and fput all cache items associated with @inode.
+ * Unhash and put all cache item associated with @inode.
  */
-void
-nfsd_file_close_inode_sync(struct inode *inode)
+static void
+nfsd_file_close_inode(struct inode *inode)
 {
 	LIST_HEAD(dispose);
 	unsigned int count;
 
 	count = __nfsd_file_close_inode(inode, &dispose);
-	trace_nfsd_file_close_inode_sync(inode, count);
-	nfsd_file_dispose_list_sync(&dispose);
+	trace_nfsd_file_close_inode(inode, count);
+	nfsd_file_dispose_list_delayed(&dispose);
 }
 
 /**
- * nfsd_file_close_inode - attempt a delayed close of a nfsd_file
+ * nfsd_file_close_inode_sync - attempt to forcibly close a nfsd_file
  * @inode: inode of the file to attempt to remove
  *
- * Unhash and put all cache item associated with @inode.
+ * Unhash and put, then flush and fput all cache items associated with @inode.
  */
-static void
-nfsd_file_close_inode(struct inode *inode)
+void
+nfsd_file_close_inode_sync(struct inode *inode)
 {
 	LIST_HEAD(dispose);
 	unsigned int count;
 
 	count = __nfsd_file_close_inode(inode, &dispose);
-	trace_nfsd_file_close_inode(inode, count);
-	nfsd_file_dispose_list_delayed(&dispose);
+	trace_nfsd_file_close_inode_sync(inode, count);
+	nfsd_file_dispose_list_sync(&dispose);
 }
 
 /**
@@ -891,7 +892,7 @@  __nfsd_file_cache_purge(struct net *net)
 		nf = rhashtable_walk_next(&iter);
 		while (!IS_ERR_OR_NULL(nf)) {
 			if (!net || nf->nf_net == net)
-				nfsd_file_unhash_and_dispose(nf, &dispose);
+				nfsd_file_unhash_and_queue(nf, &dispose);
 			nf = rhashtable_walk_next(&iter);
 		}
 
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index b09ab4f92d43..940252482fd4 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -903,10 +903,10 @@  DEFINE_EVENT(nfsd_file_class, name, \
 	TP_PROTO(struct nfsd_file *nf), \
 	TP_ARGS(nf))
 
-DEFINE_NFSD_FILE_EVENT(nfsd_file_put_final);
+DEFINE_NFSD_FILE_EVENT(nfsd_file_free);
 DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash);
 DEFINE_NFSD_FILE_EVENT(nfsd_file_put);
-DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash_and_dispose);
+DEFINE_NFSD_FILE_EVENT(nfsd_file_unhash_and_queue);
 
 TRACE_EVENT(nfsd_file_alloc,
 	TP_PROTO(