Re: [PATCH 05/11] xfs: remove the xb_page_found stat counter in xfs_buf_alloc_pages

From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 05/11] xfs: remove the xb_page_found stat counter in xfs_buf_alloc_pages
Date: Thu, 20 May 2021 08:55:18 +1000	[thread overview]
Message-ID: <20210519225518.GE664593@dread.disaster.area> (raw)
In-Reply-To: <20210519190900.320044-6-hch@lst.de>

On Wed, May 19, 2021 at 09:08:54PM +0200, Christoph Hellwig wrote:
> We did not find any page, we're allocating them all from the page
> allocator.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 9c64c374411081..76240d84d58b61 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -436,8 +436,6 @@ xfs_buf_alloc_pages(
>  			goto retry;
>  		}
>  
> -		XFS_STATS_INC(bp->b_mount, xb_page_found);
> -
>  		nbytes = min_t(size_t, size, PAGE_SIZE);
>  		size -= nbytes;
>  		bp->b_pages[i] = page;

NACK. This is actually telling us that a page was allocated
successfully. I just used this very stat in combination with the
page allocate failure stat (xb_page_retries) to determine that the
bulk alloc code was failing to allocate all the pages asked for at
least 20% of the time.

$ xfs_stats.pl
.....
    xs_ig_dup............             0  Buf Statistics
    xs_ig_reclaims.......      38377759    pb_get................     432887411
    xs_ig_attrchg........             1    pb_create.............       1839653
  Log Operations                           pb_get_locked.........     431047794
    xs_log_writes........         71572    pb_get_locked_waited..           346
    xs_log_blocks........      36644864    pb_busy_locked........         13615
    xs_log_noiclogs......           265    pb_miss_locked........       1839651
    xs_log_force.........           521    pb_page_retries.......        488537
    xs_log_force_sleep...           495    pb_page_found.........       1839431
                                           pb_get_read...........           577

See the pb_miss_locked, pb_page_found and pb_page_retries numbers?
Almost all cache misses required page (rather than heap) allocation,
and 25% of them bulk allocation failed to allocate all pages in a
single call.

So, yeah, the buffer cache stats are useful diagnostic information
that I use a lot...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com