diff mbox series

[09/15] xfs: don't allow highmem pages in xfile mappings

Message ID 20240103084126.513354-10-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/15] shmem: move the shmem_mapping assert into shmem_get_folio_gfp | expand

Commit Message

Christoph Hellwig Jan. 3, 2024, 8:41 a.m. UTC 
XFS is generally used on 64-bit, non-highmem platforms and xfile
mappings are accessed all the time.  Reduce our pain by not allowing
any highmem mappings in the xfile page cache and remove all the kmap
calls for it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/xfarray.c |  3 +--
 fs/xfs/scrub/xfile.c   | 21 +++++++++------------
 2 files changed, 10 insertions(+), 14 deletions(-)

Comments

Christoph Hellwig Jan. 4, 2024, 6:24 a.m. UTC | #1 
On Wed, Jan 03, 2024 at 04:03:24PM -0800, Darrick J. Wong wrote:
> > +	/*
> > +	 * We don't want to bother with kmapping data during repair, so don't
> > +	 * allow highmem pages to back this mapping.
> > +	 */
> > +	mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
> 
> Gonna be fun to see what happens on 32-bit. ;)

32-bit with highmem, yes.  I suspect we should just not allow online
repair and scrub on that.  I've in fact been tempted to see who would
scream if we'd disallow XFS on 32-bit entirel, as that would simplify
a lot of things.
Darrick J. Wong Jan. 4, 2024, 7:01 a.m. UTC | #2 
On Thu, Jan 04, 2024 at 07:24:28AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 03, 2024 at 04:03:24PM -0800, Darrick J. Wong wrote:
> > > +	/*
> > > +	 * We don't want to bother with kmapping data during repair, so don't
> > > +	 * allow highmem pages to back this mapping.
> > > +	 */
> > > +	mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
> > 
> > Gonna be fun to see what happens on 32-bit. ;)
> 
> 32-bit with highmem, yes.  I suspect we should just not allow online
> repair and scrub on that.  I've in fact been tempted to see who would
> scream if we'd disallow XFS on 32-bit entirel, as that would simplify
> a lot of things.

You and Dave both. :)

I guess we could propose deprecating it like V4 and see if people come
out of the woodwork.

--D
diff mbox series

Patch

diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index f0f532c10a5acc..3a44700037924b 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -580,7 +580,7 @@  xfarray_sort_get_page(
 	 * xfile pages must never be mapped into userspace, so we skip the
 	 * dcache flush when mapping the page.
 	 */
-	si->page_kaddr = kmap_local_page(si->xfpage.page);
+	si->page_kaddr = page_address(si->xfpage.page);
 	return 0;
 }
 
@@ -592,7 +592,6 @@  xfarray_sort_put_page(
 	if (!si->page_kaddr)
 		return 0;
 
-	kunmap_local(si->page_kaddr);
 	si->page_kaddr = NULL;
 
 	return xfile_put_page(si->array->xfile, &si->xfpage);
diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
index e872f4f0263f59..afbd205289e9b0 100644
--- a/fs/xfs/scrub/xfile.c
+++ b/fs/xfs/scrub/xfile.c
@@ -77,6 +77,12 @@  xfile_create(
 	inode = file_inode(xf->file);
 	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
 
+	/*
+	 * We don't want to bother with kmapping data during repair, so don't
+	 * allow highmem pages to back this mapping.
+	 */
+	mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
+
 	trace_xfile_create(xf);
 
 	*xfilep = xf;
@@ -126,7 +132,6 @@  xfile_obj_load(
 
 	pflags = memalloc_nofs_save();
 	while (count > 0) {
-		void		*p, *kaddr;
 		unsigned int	len;
 
 		len = min_t(ssize_t, count, PAGE_SIZE - offset_in_page(pos));
@@ -153,10 +158,7 @@  xfile_obj_load(
 		 * xfile pages must never be mapped into userspace, so
 		 * we skip the dcache flush.
 		 */
-		kaddr = kmap_local_page(page);
-		p = kaddr + offset_in_page(pos);
-		memcpy(buf, p, len);
-		kunmap_local(kaddr);
+		memcpy(buf, page_address(page) + offset_in_page(pos), len);
 		put_page(page);
 
 advance:
@@ -221,14 +223,13 @@  xfile_obj_store(
 		 * the dcache flush.  If the page is not uptodate, zero it
 		 * before writing data.
 		 */
-		kaddr = kmap_local_page(page);
+		kaddr = page_address(page);
 		if (!PageUptodate(page)) {
 			memset(kaddr, 0, PAGE_SIZE);
 			SetPageUptodate(page);
 		}
 		p = kaddr + offset_in_page(pos);
 		memcpy(p, buf, len);
-		kunmap_local(kaddr);
 
 		ret = aops->write_end(NULL, mapping, pos, len, len, page,
 				fsdata);
@@ -314,11 +315,7 @@  xfile_get_page(
 	 * to the caller and make sure the backing store will hold on to them.
 	 */
 	if (!PageUptodate(page)) {
-		void	*kaddr;
-
-		kaddr = kmap_local_page(page);
-		memset(kaddr, 0, PAGE_SIZE);
-		kunmap_local(kaddr);
+		memset(page_address(page), 0, PAGE_SIZE);
 		SetPageUptodate(page);
 	}