public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org, hch@lst.de
Subject: Re: [PATCH 05/10] xfs: move page freeing into _xfs_buf_free_pages()
Date: Thu, 27 May 2021 16:03:45 -0700	[thread overview]
Message-ID: <20210527230345.GF2402049@locust> (raw)
In-Reply-To: <20210526224722.1111377-6-david@fromorbit.com>

On Thu, May 27, 2021 at 08:47:17AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Rather than open coding it just before we call
> _xfs_buf_free_pages(). Also, rename the function to
> xfs_buf_free_pages() as the leading underscore has no useful
> meaning.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks pretty straightforward.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_buf.c | 61 ++++++++++++++++++------------------------------
>  1 file changed, 23 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index aa978111c01f..d15999c41885 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -272,25 +272,30 @@ _xfs_buf_alloc(
>  	return 0;
>  }
>  
> -/*
> - *	Frees b_pages if it was allocated.
> - */
> -STATIC void
> -_xfs_buf_free_pages(
> +static void
> +xfs_buf_free_pages(
>  	struct xfs_buf	*bp)
>  {
> +	uint		i;
> +
> +	ASSERT(bp->b_flags & _XBF_PAGES);
> +
> +	if (xfs_buf_is_vmapped(bp))
> +		vm_unmap_ram(bp->b_addr - bp->b_offset, bp->b_page_count);
> +
> +	for (i = 0; i < bp->b_page_count; i++) {
> +		if (bp->b_pages[i])
> +			__free_page(bp->b_pages[i]);
> +	}
> +	if (current->reclaim_state)
> +		current->reclaim_state->reclaimed_slab += bp->b_page_count;
> +
>  	if (bp->b_pages != bp->b_page_array)
>  		kmem_free(bp->b_pages);
>  	bp->b_pages = NULL;
> +	bp->b_flags &= ~_XBF_PAGES;
>  }
>  
> -/*
> - *	Releases the specified buffer.
> - *
> - * 	The modification state of any associated pages is left unchanged.
> - * 	The buffer must not be on any hash - use xfs_buf_rele instead for
> - * 	hashed and refcounted buffers
> - */
>  static void
>  xfs_buf_free(
>  	struct xfs_buf		*bp)
> @@ -299,24 +304,11 @@ xfs_buf_free(
>  
>  	ASSERT(list_empty(&bp->b_lru));
>  
> -	if (bp->b_flags & _XBF_PAGES) {
> -		uint		i;
> -
> -		if (xfs_buf_is_vmapped(bp))
> -			vm_unmap_ram(bp->b_addr - bp->b_offset,
> -					bp->b_page_count);
> -
> -		for (i = 0; i < bp->b_page_count; i++) {
> -			struct page	*page = bp->b_pages[i];
> -
> -			__free_page(page);
> -		}
> -		if (current->reclaim_state)
> -			current->reclaim_state->reclaimed_slab +=
> -							bp->b_page_count;
> -	} else if (bp->b_flags & _XBF_KMEM)
> +	if (bp->b_flags & _XBF_PAGES)
> +		xfs_buf_free_pages(bp);
> +	else if (bp->b_flags & _XBF_KMEM)
>  		kmem_free(bp->b_addr);
> -	_xfs_buf_free_pages(bp);
> +
>  	xfs_buf_free_maps(bp);
>  	kmem_cache_free(xfs_buf_zone, bp);
>  }
> @@ -361,7 +353,6 @@ xfs_buf_alloc_pages(
>  {
>  	gfp_t		gfp_mask = xb_to_gfp(flags);
>  	long		filled = 0;
> -	int		error;
>  
>  	/* Make sure that we have a page list */
>  	bp->b_page_count = page_count;
> @@ -398,20 +389,14 @@ xfs_buf_alloc_pages(
>  			continue;
>  
>  		if (flags & XBF_READ_AHEAD) {
> -			error = -ENOMEM;
> -			goto out_free_pages;
> +			xfs_buf_free_pages(bp);
> +			return -ENOMEM;
>  		}
>  
>  		XFS_STATS_INC(bp->b_mount, xb_page_retries);
>  		congestion_wait(BLK_RW_ASYNC, HZ/50);
>  	}
>  	return 0;
> -
> -out_free_pages:
> -	while (--filled >= 0)
> -		__free_page(bp->b_pages[filled]);
> -	bp->b_flags &= ~_XBF_PAGES;
> -	return error;
>  }
>  
>  
> -- 
> 2.31.1
> 

  reply	other threads:[~2021-05-27 23:03 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-26 22:47 [PATCH 00/10] xfs: buffer bulk page allocation and cleanups Dave Chinner
2021-05-26 22:47 ` [PATCH 01/10] xfs: split up xfs_buf_allocate_memory Dave Chinner
2021-05-27 22:48   ` Darrick J. Wong
2021-05-27 23:10     ` Darrick J. Wong
2021-05-26 22:47 ` [PATCH 02/10] xfs: use xfs_buf_alloc_pages for uncached buffers Dave Chinner
2021-05-27 22:50   ` Darrick J. Wong
2021-05-26 22:47 ` [PATCH 03/10] xfs: use alloc_pages_bulk_array() for buffers Dave Chinner
2021-05-27 22:59   ` Darrick J. Wong
2021-05-27 23:01     ` Darrick J. Wong
2021-05-26 22:47 ` [PATCH 04/10] xfs: merge _xfs_buf_get_pages() Dave Chinner
2021-05-27 23:02   ` Darrick J. Wong
2021-05-26 22:47 ` [PATCH 05/10] xfs: move page freeing into _xfs_buf_free_pages() Dave Chinner
2021-05-27 23:03   ` Darrick J. Wong [this message]
2021-05-26 22:47 ` [PATCH 06/10] xfs: remove ->b_offset handling for page backed buffers Dave Chinner
2021-05-27 23:09   ` Darrick J. Wong
2021-06-01  1:46     ` Dave Chinner
2021-05-26 22:47 ` [PATCH 07/10] xfs: simplify the b_page_count calculation Dave Chinner
2021-05-27 23:15   ` Darrick J. Wong
2021-05-27 23:29     ` Dave Chinner
2021-05-26 22:47 ` [PATCH 08/10] xfs: get rid of xb_to_gfp() Dave Chinner
2021-05-27 23:12   ` Darrick J. Wong
2021-05-26 22:47 ` [PATCH 09/10] xfs: cleanup error handling in xfs_buf_get_map Dave Chinner
2021-05-27 23:16   ` Darrick J. Wong
2021-05-26 22:47 ` [PATCH 10/10] xfs: merge xfs_buf_allocate_memory Dave Chinner
2021-05-27 23:17   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210527230345.GF2402049@locust \
    --to=djwong@kernel.org \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox