[PATCH 0/2] folio_copy_tail

[PATCH 0/2] folio_copy_tail

[Matthew Wilcox (Oracle)]

I'm trying to make it easy & efficient for a filesystem to read its file
tails into a folio.  iomap's implementation was pretty good, but had
some limitations (eg tails couldn't cross a page boundary).

This should be an all-singing, all-dancing implementation which copies
the correct part of the buffer into the correct part of the folio and
zeroes the remainder of the folio.  It should work with highmem, but
the calculations are a bit tricky and I may have got something wrong.

For some reason I'm currently running an XFS test against it, even
though I know XFS doesn't support inline data.  If there's good feedback,
I'll take a look at converting udf_adinicb_readpage() and other similar
functions.

Matthew Wilcox (Oracle) (2):
  filemap: Add folio_copy_tail()
  iomap: Use folio_copy_tail()

 fs/iomap/buffered-io.c  | 23 +++++------------
 include/linux/pagemap.h |  1 +
 mm/filemap.c            | 56 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 17 deletions(-)

-- 
2.39.1

[PATCH 1/2] filemap: Add folio_copy_tail()

[Matthew Wilcox (Oracle)]

This is a helper function for filesystems which support file tails.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/pagemap.h |  1 +
 mm/filemap.c            | 57 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index c21b3ad1068c..618fe184c248 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -764,6 +764,7 @@ static inline struct page *grab_cache_page(struct address_space *mapping,
 	return find_or_create_page(mapping, index, mapping_gfp_mask(mapping));
 }
 
+void folio_copy_tail(struct folio *, loff_t pos, void *src, size_t max);
 struct folio *read_cache_folio(struct address_space *, pgoff_t index,
 		filler_t *filler, struct file *file);
 struct folio *mapping_read_folio_gfp(struct address_space *, pgoff_t index,
diff --git a/mm/filemap.c b/mm/filemap.c
index 40be33b5ee46..b02d9c390d3c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -4145,3 +4145,60 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp)
 	return try_to_free_buffers(folio);
 }
 EXPORT_SYMBOL(filemap_release_folio);
+
+/**
+ * folio_copy_tail - Copy an in-memory file tail into a page cache folio.
+ * @folio: The folio to copy into.
+ * @pos: The file position of the first byte of data in the tail.
+ * @src: The address of the tail data.
+ * @max: The size of the buffer used for the tail data.
+ *
+ * Supports file tails starting at @pos that are a maximum of @max
+ * bytes in size.  Zeroes the remainder of the folio.
+ */
+void folio_copy_tail(struct folio *folio, loff_t pos, void *src, size_t max)
+{
+	loff_t isize = i_size_read(folio->mapping->host);
+	size_t offset, len = isize - pos;
+	char *dst;
+
+	if (folio_pos(folio) > isize) {
+		len = 0;
+	} else if (folio_pos(folio) > pos) {
+		len -= folio_pos(folio) - pos;
+		src += folio_pos(folio) - pos;
+		max -= folio_pos(folio) - pos;
+		pos = folio_pos(folio);
+	}
+	/*
+	 * i_size is larger than the number of bytes stored in the tail?
+	 * Assume the remainder is zero-padded.
+	 */
+	if (WARN_ON_ONCE(len > max))
+		len = max;
+	offset = offset_in_folio(folio, pos);
+	dst = kmap_local_folio(folio, offset);
+	if (folio_test_highmem(folio) && folio_test_large(folio)) {
+		size_t poff = offset_in_page(offset);
+		size_t plen = min(poff + len, PAGE_SIZE) - poff;
+
+		for (;;) {
+			memcpy(dst, src, plen);
+			memset(dst + plen, 0, PAGE_SIZE - poff - plen);
+			offset += PAGE_SIZE - poff;
+			if (offset == folio_size(folio))
+				break;
+			kunmap_local(dst);
+			dst = kmap_local_folio(folio, offset);
+			len -= plen;
+			poff = 0;
+			plen = min(len, PAGE_SIZE);
+		}
+	} else {
+		memcpy(dst, src, len);
+		memset(dst + len, 0, folio_size(folio) - len - offset);
+	}
+	kunmap_local(dst);
+	flush_dcache_folio(folio);
+}
+EXPORT_SYMBOL_GPL(folio_copy_tail);
-- 
2.39.1

[PATCH 2/2] iomap: Use folio_copy_tail()

[Matthew Wilcox (Oracle)]

The iomap handling of tails doesn't support multi-page folios, so
convert it to call folio_copy_tail(), which does.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/iomap/buffered-io.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 10a203515583..d08edf2de19c 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -215,31 +215,20 @@ static int iomap_read_inline_data(const struct iomap_iter *iter,
 {
 	struct iomap_page *iop;
 	const struct iomap *iomap = iomap_iter_srcmap(iter);
-	size_t size = i_size_read(iter->inode) - iomap->offset;
-	size_t poff = offset_in_page(iomap->offset);
-	size_t offset = offset_in_folio(folio, iomap->offset);
-	void *addr;
+	loff_t pos = iomap->offset;
 
 	if (folio_test_uptodate(folio))
 		return 0;
 
-	if (WARN_ON_ONCE(size > PAGE_SIZE - poff))
-		return -EIO;
-	if (WARN_ON_ONCE(size > PAGE_SIZE -
-			 offset_in_page(iomap->inline_data)))
-		return -EIO;
-	if (WARN_ON_ONCE(size > iomap->length))
-		return -EIO;
-	if (offset > 0)
+	if (pos > folio_pos(folio))
 		iop = iomap_page_create(iter->inode, folio, iter->flags);
 	else
 		iop = to_iomap_page(folio);
 
-	addr = kmap_local_folio(folio, offset);
-	memcpy(addr, iomap->inline_data, size);
-	memset(addr + size, 0, PAGE_SIZE - poff - size);
-	kunmap_local(addr);
-	iomap_set_range_uptodate(folio, iop, offset, PAGE_SIZE - poff);
+	folio_copy_tail(folio, pos, iomap->inline_data,
+			iomap->length);
+	iomap_set_range_uptodate(folio, iop, pos - folio_pos(folio),
+			folio_size(folio) - offset_in_folio(folio, pos));
 	return 0;
 }
 
-- 
2.39.1

Re: [PATCH 1/2] filemap: Add folio_copy_tail()

[kernel test robot]

Hi Matthew,

I love your patch! Perhaps something to improve:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master next-20230303]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/filemap-Add-folio_copy_tail/20230303-144359
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230303064315.701090-2-willy%40infradead.org
patch subject: [PATCH 1/2] filemap: Add folio_copy_tail()
config: arm-randconfig-r006-20230302 (https://download.01.org/0day-ci/archive/20230303/202303031911.we6DjYXd-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/a4ee841f30215e4f7c5dda2ab82007f590a6a62b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/filemap-Add-folio_copy_tail/20230303-144359
        git checkout a4ee841f30215e4f7c5dda2ab82007f590a6a62b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303031911.we6DjYXd-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> mm/filemap.c:4160:17: warning: comparison of distinct pointer types ('typeof (poff + len) *' (aka 'unsigned int *') and 'typeof (((1UL) << 12)) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
                   size_t plen = min(poff + len, PAGE_SIZE) - poff;
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:67:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> mm/filemap.c:4172:11: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (((1UL) << 12)) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
                           plen = min(len, PAGE_SIZE);
                                  ^~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:67:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   2 warnings generated.


vim +4160 mm/filemap.c

  4125	
  4126	/**
  4127	 * folio_copy_tail - Copy an in-memory file tail into a page cache folio.
  4128	 * @folio: The folio to copy into.
  4129	 * @pos: The file position of the first byte of data in the tail.
  4130	 * @src: The address of the tail data.
  4131	 * @max: The size of the buffer used for the tail data.
  4132	 *
  4133	 * Supports file tails starting at @pos that are a maximum of @max
  4134	 * bytes in size.  Zeroes the remainder of the folio.
  4135	 */
  4136	void folio_copy_tail(struct folio *folio, loff_t pos, void *src, size_t max)
  4137	{
  4138		loff_t isize = i_size_read(folio->mapping->host);
  4139		size_t offset, len = isize - pos;
  4140		char *dst;
  4141	
  4142		if (folio_pos(folio) > isize) {
  4143			len = 0;
  4144		} else if (folio_pos(folio) > pos) {
  4145			len -= folio_pos(folio) - pos;
  4146			src += folio_pos(folio) - pos;
  4147			max -= folio_pos(folio) - pos;
  4148			pos = folio_pos(folio);
  4149		}
  4150		/*
  4151		 * i_size is larger than the number of bytes stored in the tail?
  4152		 * Assume the remainder is zero-padded.
  4153		 */
  4154		if (WARN_ON_ONCE(len > max))
  4155			len = max;
  4156		offset = offset_in_folio(folio, pos);
  4157		dst = kmap_local_folio(folio, offset);
  4158		if (folio_test_highmem(folio) && folio_test_large(folio)) {
  4159			size_t poff = offset_in_page(offset);
> 4160			size_t plen = min(poff + len, PAGE_SIZE) - poff;
  4161	
  4162			for (;;) {
  4163				memcpy(dst, src, plen);
  4164				memset(dst + plen, 0, PAGE_SIZE - poff - plen);
  4165				offset += PAGE_SIZE - poff;
  4166				if (offset == folio_size(folio))
  4167					break;
  4168				kunmap_local(dst);
  4169				dst = kmap_local_folio(folio, offset);
  4170				len -= plen;
  4171				poff = 0;
> 4172				plen = min(len, PAGE_SIZE);

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests