diff mbox series

[11/24] xfs:: account for memory freed from metadata buffers

Message ID 20190801021752.4986-12-david@fromorbit.com (mailing list archive)
State New, archived
Headers show
Series mm, xfs: non-blocking inode reclaim | expand

Commit Message

Dave Chinner Aug. 1, 2019, 2:17 a.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

The buffer cache shrinker frees more than just the xfs_buf slab
objects - it also frees the pages attached to the buffers. Make sure
the memory reclaim code accounts for this memory being freed
correctly, similar to how the inode shrinker accounts for pages
freed from the page cache due to mapping invalidation.

We also need to make sure that the mm subsystem knows these are
reclaimable objects. We provide the memory reclaim subsystem with a
a shrinker to reclaim xfs_bufs, so we should really mark the slab
that way.

We also have a lot of xfs_bufs in a busy system, spread them around
like we do inodes.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Aug. 1, 2019, 8:16 a.m. UTC | #1 
> +
> +		/*
> +		 * Account for the buffer memory freed here so memory reclaim
> +		 * sees this and not just the xfs_buf slab entry being freed.
> +		 */
> +		if (current->reclaim_state)
> +			current->reclaim_state->reclaimed_pages += bp->b_page_count;
> +

I think this wants a mm-layer helper ala:

static inline void shrinker_mark_pages_reclaimed(unsigned long nr_pages)
{
	if (current->reclaim_state)
		current->reclaim_state->reclaimed_pages += nr_pages;
}

plus good documentation on when to use it.
Dave Chinner Aug. 1, 2019, 9:21 a.m. UTC | #2 
On Thu, Aug 01, 2019 at 01:16:03AM -0700, Christoph Hellwig wrote:
> > +
> > +		/*
> > +		 * Account for the buffer memory freed here so memory reclaim
> > +		 * sees this and not just the xfs_buf slab entry being freed.
> > +		 */
> > +		if (current->reclaim_state)
> > +			current->reclaim_state->reclaimed_pages += bp->b_page_count;
> > +
> 
> I think this wants a mm-layer helper ala:
> 
> static inline void shrinker_mark_pages_reclaimed(unsigned long nr_pages)
> {
> 	if (current->reclaim_state)
> 		current->reclaim_state->reclaimed_pages += nr_pages;
> }
> 
> plus good documentation on when to use it.

Sure, but that's something for patch 6, not this one :)

Cheers,

Dave.
Christoph Hellwig Aug. 6, 2019, 5:51 a.m. UTC | #3 
On Thu, Aug 01, 2019 at 07:21:33PM +1000, Dave Chinner wrote:
> > static inline void shrinker_mark_pages_reclaimed(unsigned long nr_pages)
> > {
> > 	if (current->reclaim_state)
> > 		current->reclaim_state->reclaimed_pages += nr_pages;
> > }
> > 
> > plus good documentation on when to use it.
> 
> Sure, but that's something for patch 6, not this one :)

Sounds good, I just skimmend through the XFS patches.  While we are at
it:  there is a double : in the subject line.
diff mbox series

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 6e0f76532535..beb816cd54d6 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1667,6 +1667,14 @@  xfs_buftarg_shrink_scan(
 		struct xfs_buf *bp;
 		bp = list_first_entry(&dispose, struct xfs_buf, b_lru);
 		list_del_init(&bp->b_lru);
+
+		/*
+		 * Account for the buffer memory freed here so memory reclaim
+		 * sees this and not just the xfs_buf slab entry being freed.
+		 */
+		if (current->reclaim_state)
+			current->reclaim_state->reclaimed_pages += bp->b_page_count;
+
 		xfs_buf_rele(bp);
 	}
 
@@ -2057,7 +2065,8 @@  int __init
 xfs_buf_init(void)
 {
 	xfs_buf_zone = kmem_zone_init_flags(sizeof(xfs_buf_t), "xfs_buf",
-						KM_ZONE_HWALIGN, NULL);
+			KM_ZONE_HWALIGN | KM_ZONE_SPREAD | KM_ZONE_RECLAIM,
+			NULL);
 	if (!xfs_buf_zone)
 		goto out;