[PATCH] xfs: compute buffer address correctly in xmbuf_map_backing_mem

[PATCH] xfs: compute buffer address correctly in xmbuf_map_backing_mem

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Prior to commit e614a00117bc2d, xmbuf_map_backing_mem relied on
folio_file_page to return the base page for the xmbuf's loff_t in the
xfile, and set b_addr to the page_address of that base page.

Now that folio_file_page has been removed from xmbuf_map_backing_mem, we
always set b_addr to the folio_address of the folio.  This is correct
for the situation where the folio size matches the buffer size, but it's
totally wrong if tmpfs uses large folios.  We need to use
offset_in_folio here.

Found via xfs/801, which demonstrated evidence of corruption of an
in-memory rmap btree block right after initializing an adjacent block.

Fixes: e614a00117bc2d ("xfs: cleanup mapping tmpfs folios into the buffer cache")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_buf_mem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_buf_mem.c b/fs/xfs/xfs_buf_mem.c
index b4ffd80b7cb632..dcbfa274e06dc6 100644
--- a/fs/xfs/xfs_buf_mem.c
+++ b/fs/xfs/xfs_buf_mem.c
@@ -165,7 +165,7 @@ xmbuf_map_backing_mem(
 	folio_set_dirty(folio);
 	folio_unlock(folio);
 
-	bp->b_addr = folio_address(folio);
+	bp->b_addr = folio_address(folio) + offset_in_folio(folio, pos);
 	return 0;
 }
 

Re: [PATCH] xfs: compute buffer address correctly in xmbuf_map_backing_mem

[Christoph Hellwig]

On Mon, Apr 07, 2025 at 05:30:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Prior to commit e614a00117bc2d, xmbuf_map_backing_mem relied on
> folio_file_page to return the base page for the xmbuf's loff_t in the
> xfile, and set b_addr to the page_address of that base page.
> 
> Now that folio_file_page has been removed from xmbuf_map_backing_mem, we
> always set b_addr to the folio_address of the folio.  This is correct
> for the situation where the folio size matches the buffer size, but it's
> totally wrong if tmpfs uses large folios.  We need to use
> offset_in_folio here.
> 
> Found via xfs/801, which demonstrated evidence of corruption of an
> in-memory rmap btree block right after initializing an adjacent block.

Hmm, I thought we'd never get large folios for our non-standard tmpfs
use.  I guess I was wrong on that..

The fix looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But a little note below:

> +	bp->b_addr = folio_address(folio) + offset_in_folio(folio, pos);

Given that this is or at least will become a common pattern, do we
want a mm layer helper for it?

Re: [PATCH] xfs: compute buffer address correctly in xmbuf_map_backing_mem

[Darrick J. Wong]

On Mon, Apr 07, 2025 at 10:11:08PM -0700, Christoph Hellwig wrote:
> On Mon, Apr 07, 2025 at 05:30:30PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Prior to commit e614a00117bc2d, xmbuf_map_backing_mem relied on
> > folio_file_page to return the base page for the xmbuf's loff_t in the
> > xfile, and set b_addr to the page_address of that base page.
> > 
> > Now that folio_file_page has been removed from xmbuf_map_backing_mem, we
> > always set b_addr to the folio_address of the folio.  This is correct
> > for the situation where the folio size matches the buffer size, but it's
> > totally wrong if tmpfs uses large folios.  We need to use
> > offset_in_folio here.
> > 
> > Found via xfs/801, which demonstrated evidence of corruption of an
> > in-memory rmap btree block right after initializing an adjacent block.
> 
> Hmm, I thought we'd never get large folios for our non-standard tmpfs
> use.  I guess I was wrong on that..

Yeah, you can force THPs for tmpfs.  I don't know why you would, the
memory usage is gawful on most files that end up in there.

> The fix looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But a little note below:
> 
> > +	bp->b_addr = folio_address(folio) + offset_in_folio(folio, pos);
> 
> Given that this is or at least will become a common pattern, do we
> want a mm layer helper for it?

Yeah, we should; this is the third one in XFS.  What to name it, though?

void *folio_addr(const struct folio *folio, loff_t pos) ?

I'm surprised there wasn't an equivalent for struct page.

--D

Re: [PATCH] xfs: compute buffer address correctly in xmbuf_map_backing_mem

[Carlos Maiolino]

On Mon, 07 Apr 2025 17:30:30 -0700, Darrick J. Wong wrote:
> Prior to commit e614a00117bc2d, xmbuf_map_backing_mem relied on
> folio_file_page to return the base page for the xmbuf's loff_t in the
> xfile, and set b_addr to the page_address of that base page.
> 
> Now that folio_file_page has been removed from xmbuf_map_backing_mem, we
> always set b_addr to the folio_address of the folio.  This is correct
> for the situation where the folio size matches the buffer size, but it's
> totally wrong if tmpfs uses large folios.  We need to use
> offset_in_folio here.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: compute buffer address correctly in xmbuf_map_backing_mem
      commit: a37b3b9c3cc595521c7f9d9b2b0b2ad367bf9c98

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>