public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fs/isofs: Replace kmap() with kmap_local_page()
@ 2022-07-31 19:01 Fabio M. De Francesco
  2022-07-31 22:09 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Fabio M. De Francesco @ 2022-07-31 19:01 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle), Jan Kara, Roman Gushchin,
	Pali Rohár, Andrew Morton, Muchun Song, Ira Weiny,
	linux-kernel
  Cc: Fabio M. De Francesco

The use of kmap() is being deprecated in favor of kmap_local_page().

There are two main problems with kmap(): (1) It comes with an overhead as
mapping space is restricted and protected by a global lock for
synchronization and (2) it also requires global TLB invalidation when the
kmap’s pool wraps and it might block when the mapping space is fully
utilized until a slot becomes available.

With kmap_local_page() the mappings are per thread, CPU local, can take
page faults, and can be called from any context (including interrupts).
Tasks can be preempted and, when scheduled to run again, the kernel
virtual addresses are restored and still valid. It is faster than kmap()
in kernels with HIGHMEM enabled.

Since kmap_local_page() can be safely used in compress.c, it should be
called everywhere instead of kmap().

Therefore, replace kmap() with kmap_local_page() in compress.c. Where it
is needed, use memzero_page() instead of open coding kmap_local_page()
plus memset() to fill the pages with zeros. Delete the redundant
flush_dcache_page() in the two call sites of memzero_page().

Tested with mkisofs on a QEMU/KVM x86_32 VM, 6GB RAM, booting a kernel
with HIGHMEM64GB enabled.

Cc: Jan Kara <jack@suse.cz>
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---

Many thanks to Jan Kara for the comments and suggestions provided as
reply to my previous RFC.[1] Furthermore, I want to thank Ira Weiny for
the advice he provided privately, especially about how to use mkisofs to
test that this patch works properly.

[1] https://lore.kernel.org/lkml/20220726145024.rryvw7ot7j2c6tqv@quack3/ 

 fs/isofs/compress.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/isofs/compress.c b/fs/isofs/compress.c
index 95a19f25d61c..f17754484a75 100644
--- a/fs/isofs/compress.c
+++ b/fs/isofs/compress.c
@@ -67,8 +67,7 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 		for ( i = 0 ; i < pcount ; i++ ) {
 			if (!pages[i])
 				continue;
-			memset(page_address(pages[i]), 0, PAGE_SIZE);
-			flush_dcache_page(pages[i]);
+			memzero_page(pages[i], 0, PAGE_SIZE);
 			SetPageUptodate(pages[i]);
 		}
 		return ((loff_t)pcount) << PAGE_SHIFT;
@@ -120,7 +119,7 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 	       zerr != Z_STREAM_END) {
 		if (!stream.avail_out) {
 			if (pages[curpage]) {
-				stream.next_out = page_address(pages[curpage])
+				stream.next_out = kmap_local_page(pages[curpage])
 						+ poffset;
 				stream.avail_out = PAGE_SIZE - poffset;
 				poffset = 0;
@@ -176,6 +175,10 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 				flush_dcache_page(pages[curpage]);
 				SetPageUptodate(pages[curpage]);
 			}
+			if (stream.next_out != (char *)zisofs_sink_page) {
+				kunmap_local(stream.next_out);
+				stream.next_out = NULL;
+			}
 			curpage++;
 		}
 		if (!stream.avail_in)
@@ -183,6 +186,8 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 	}
 inflate_out:
 	zlib_inflateEnd(&stream);
+	if (stream.next_out && stream.next_out != (char *)zisofs_sink_page)
+		kunmap_local(stream.next_out);
 
 z_eio:
 	mutex_unlock(&zisofs_zlib_lock);
@@ -283,9 +288,7 @@ static int zisofs_fill_pages(struct inode *inode, int full_page, int pcount,
 	}
 
 	if (poffset && *pages) {
-		memset(page_address(*pages) + poffset, 0,
-		       PAGE_SIZE - poffset);
-		flush_dcache_page(*pages);
+		memzero_page(*pages, poffset, PAGE_SIZE - poffset);
 		SetPageUptodate(*pages);
 	}
 	return 0;
@@ -343,10 +346,8 @@ static int zisofs_read_folio(struct file *file, struct folio *folio)
 	for (i = 0; i < pcount; i++, index++) {
 		if (i != full_page)
 			pages[i] = grab_cache_page_nowait(mapping, index);
-		if (pages[i]) {
+		if (pages[i])
 			ClearPageError(pages[i]);
-			kmap(pages[i]);
-		}
 	}
 
 	err = zisofs_fill_pages(inode, full_page, pcount, pages);
@@ -357,7 +358,6 @@ static int zisofs_read_folio(struct file *file, struct folio *folio)
 			flush_dcache_page(pages[i]);
 			if (i == full_page && err)
 				SetPageError(pages[i]);
-			kunmap(pages[i]);
 			unlock_page(pages[i]);
 			if (i != full_page)
 				put_page(pages[i]);
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] fs/isofs: Replace kmap() with kmap_local_page()
  2022-07-31 19:01 [PATCH] fs/isofs: Replace kmap() with kmap_local_page() Fabio M. De Francesco
@ 2022-07-31 22:09 ` kernel test robot
  2022-07-31 22:19 ` kernel test robot
  2022-08-01  1:01 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-07-31 22:09 UTC (permalink / raw)
  To: Fabio M. De Francesco, Matthew Wilcox (Oracle), Jan Kara,
	Roman Gushchin, Pali Rohár, Andrew Morton, Muchun Song,
	Ira Weiny, linux-kernel
  Cc: kbuild-all, Linux Memory Management List, Fabio M. De Francesco

Hi "Fabio,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jack-fs/for_next]
[also build test WARNING on akpm-mm/mm-everything linus/master v5.19-rc8 next-20220728]
[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/Fabio-M-De-Francesco/fs-isofs-Replace-kmap-with-kmap_local_page/20220801-030233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
config: i386-defconfig (https://download.01.org/0day-ci/archive/20220801/202208010627.8fQXYgTu-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/7c25888be273e928336283deae5b57f8ffa78a50
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Fabio-M-De-Francesco/fs-isofs-Replace-kmap-with-kmap_local_page/20220801-030233
        git checkout 7c25888be273e928336283deae5b57f8ffa78a50
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/isofs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/isofs/compress.c: In function 'zisofs_uncompress_block':
>> fs/isofs/compress.c:178:45: warning: comparison of distinct pointer types lacks a cast
     178 |                         if (stream.next_out != (char *)zisofs_sink_page) {
         |                                             ^~
   fs/isofs/compress.c:189:48: warning: comparison of distinct pointer types lacks a cast
     189 |         if (stream.next_out && stream.next_out != (char *)zisofs_sink_page)
         |                                                ^~


vim +178 fs/isofs/compress.c

    34	
    35	/*
    36	 * Read data of @inode from @block_start to @block_end and uncompress
    37	 * to one zisofs block. Store the data in the @pages array with @pcount
    38	 * entries. Start storing at offset @poffset of the first page.
    39	 */
    40	static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
    41					      loff_t block_end, int pcount,
    42					      struct page **pages, unsigned poffset,
    43					      int *errp)
    44	{
    45		unsigned int zisofs_block_shift = ISOFS_I(inode)->i_format_parm[1];
    46		unsigned int bufsize = ISOFS_BUFFER_SIZE(inode);
    47		unsigned int bufshift = ISOFS_BUFFER_BITS(inode);
    48		unsigned int bufmask = bufsize - 1;
    49		int i, block_size = block_end - block_start;
    50		z_stream stream = { .total_out = 0,
    51				    .avail_in = 0,
    52				    .avail_out = 0, };
    53		int zerr;
    54		int needblocks = (block_size + (block_start & bufmask) + bufmask)
    55					>> bufshift;
    56		int haveblocks;
    57		blkcnt_t blocknum;
    58		struct buffer_head **bhs;
    59		int curbh, curpage;
    60	
    61		if (block_size > deflateBound(1UL << zisofs_block_shift)) {
    62			*errp = -EIO;
    63			return 0;
    64		}
    65		/* Empty block? */
    66		if (block_size == 0) {
    67			for ( i = 0 ; i < pcount ; i++ ) {
    68				if (!pages[i])
    69					continue;
    70				memzero_page(pages[i], 0, PAGE_SIZE);
    71				SetPageUptodate(pages[i]);
    72			}
    73			return ((loff_t)pcount) << PAGE_SHIFT;
    74		}
    75	
    76		/* Because zlib is not thread-safe, do all the I/O at the top. */
    77		blocknum = block_start >> bufshift;
    78		bhs = kcalloc(needblocks + 1, sizeof(*bhs), GFP_KERNEL);
    79		if (!bhs) {
    80			*errp = -ENOMEM;
    81			return 0;
    82		}
    83		haveblocks = isofs_get_blocks(inode, blocknum, bhs, needblocks);
    84		ll_rw_block(REQ_OP_READ, 0, haveblocks, bhs);
    85	
    86		curbh = 0;
    87		curpage = 0;
    88		/*
    89		 * First block is special since it may be fractional.  We also wait for
    90		 * it before grabbing the zlib mutex; odds are that the subsequent
    91		 * blocks are going to come in in short order so we don't hold the zlib
    92		 * mutex longer than necessary.
    93		 */
    94	
    95		if (!bhs[0])
    96			goto b_eio;
    97	
    98		wait_on_buffer(bhs[0]);
    99		if (!buffer_uptodate(bhs[0])) {
   100			*errp = -EIO;
   101			goto b_eio;
   102		}
   103	
   104		stream.workspace = zisofs_zlib_workspace;
   105		mutex_lock(&zisofs_zlib_lock);
   106			
   107		zerr = zlib_inflateInit(&stream);
   108		if (zerr != Z_OK) {
   109			if (zerr == Z_MEM_ERROR)
   110				*errp = -ENOMEM;
   111			else
   112				*errp = -EIO;
   113			printk(KERN_DEBUG "zisofs: zisofs_inflateInit returned %d\n",
   114				       zerr);
   115			goto z_eio;
   116		}
   117	
   118		while (curpage < pcount && curbh < haveblocks &&
   119		       zerr != Z_STREAM_END) {
   120			if (!stream.avail_out) {
   121				if (pages[curpage]) {
   122					stream.next_out = kmap_local_page(pages[curpage])
   123							+ poffset;
   124					stream.avail_out = PAGE_SIZE - poffset;
   125					poffset = 0;
   126				} else {
   127					stream.next_out = (void *)&zisofs_sink_page;
   128					stream.avail_out = PAGE_SIZE;
   129				}
   130			}
   131			if (!stream.avail_in) {
   132				wait_on_buffer(bhs[curbh]);
   133				if (!buffer_uptodate(bhs[curbh])) {
   134					*errp = -EIO;
   135					break;
   136				}
   137				stream.next_in  = bhs[curbh]->b_data +
   138							(block_start & bufmask);
   139				stream.avail_in = min_t(unsigned, bufsize -
   140							(block_start & bufmask),
   141							block_size);
   142				block_size -= stream.avail_in;
   143				block_start = 0;
   144			}
   145	
   146			while (stream.avail_out && stream.avail_in) {
   147				zerr = zlib_inflate(&stream, Z_SYNC_FLUSH);
   148				if (zerr == Z_BUF_ERROR && stream.avail_in == 0)
   149					break;
   150				if (zerr == Z_STREAM_END)
   151					break;
   152				if (zerr != Z_OK) {
   153					/* EOF, error, or trying to read beyond end of input */
   154					if (zerr == Z_MEM_ERROR)
   155						*errp = -ENOMEM;
   156					else {
   157						printk(KERN_DEBUG
   158						       "zisofs: zisofs_inflate returned"
   159						       " %d, inode = %lu,"
   160						       " page idx = %d, bh idx = %d,"
   161						       " avail_in = %ld,"
   162						       " avail_out = %ld\n",
   163						       zerr, inode->i_ino, curpage,
   164						       curbh, stream.avail_in,
   165						       stream.avail_out);
   166						*errp = -EIO;
   167					}
   168					goto inflate_out;
   169				}
   170			}
   171	
   172			if (!stream.avail_out) {
   173				/* This page completed */
   174				if (pages[curpage]) {
   175					flush_dcache_page(pages[curpage]);
   176					SetPageUptodate(pages[curpage]);
   177				}
 > 178				if (stream.next_out != (char *)zisofs_sink_page) {
   179					kunmap_local(stream.next_out);
   180					stream.next_out = NULL;
   181				}
   182				curpage++;
   183			}
   184			if (!stream.avail_in)
   185				curbh++;
   186		}
   187	inflate_out:
   188		zlib_inflateEnd(&stream);
   189		if (stream.next_out && stream.next_out != (char *)zisofs_sink_page)
   190			kunmap_local(stream.next_out);
   191	
   192	z_eio:
   193		mutex_unlock(&zisofs_zlib_lock);
   194	
   195	b_eio:
   196		for (i = 0; i < haveblocks; i++)
   197			brelse(bhs[i]);
   198		kfree(bhs);
   199		return stream.total_out;
   200	}
   201	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fs/isofs: Replace kmap() with kmap_local_page()
  2022-07-31 19:01 [PATCH] fs/isofs: Replace kmap() with kmap_local_page() Fabio M. De Francesco
  2022-07-31 22:09 ` kernel test robot
@ 2022-07-31 22:19 ` kernel test robot
  2022-08-01  1:01 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-07-31 22:19 UTC (permalink / raw)
  To: Fabio M. De Francesco, Matthew Wilcox (Oracle), Jan Kara,
	Roman Gushchin, Pali Rohár, Andrew Morton, Muchun Song,
	Ira Weiny, linux-kernel
  Cc: llvm, kbuild-all, Linux Memory Management List,
	Fabio M. De Francesco

Hi "Fabio,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jack-fs/for_next]
[also build test WARNING on akpm-mm/mm-everything linus/master v5.19 next-20220728]
[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/Fabio-M-De-Francesco/fs-isofs-Replace-kmap-with-kmap_local_page/20220801-030233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
config: hexagon-randconfig-r041-20220801 (https://download.01.org/0day-ci/archive/20220801/202208010650.OP1Bigo2-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
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
        # https://github.com/intel-lab-lkp/linux/commit/7c25888be273e928336283deae5b57f8ffa78a50
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Fabio-M-De-Francesco/fs-isofs-Replace-kmap-with-kmap_local_page/20220801-030233
        git checkout 7c25888be273e928336283deae5b57f8ffa78a50
        # 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=hexagon SHELL=/bin/bash fs/isofs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/isofs/compress.c:178:24: warning: comparison of distinct pointer types ('Byte *' (aka 'unsigned char *') and 'char *') [-Wcompare-distinct-pointer-types]
                           if (stream.next_out != (char *)zisofs_sink_page) {
                               ~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~
   fs/isofs/compress.c:189:41: warning: comparison of distinct pointer types ('Byte *' (aka 'unsigned char *') and 'char *') [-Wcompare-distinct-pointer-types]
           if (stream.next_out && stream.next_out != (char *)zisofs_sink_page)
                                  ~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~
   2 warnings generated.


vim +178 fs/isofs/compress.c

    34	
    35	/*
    36	 * Read data of @inode from @block_start to @block_end and uncompress
    37	 * to one zisofs block. Store the data in the @pages array with @pcount
    38	 * entries. Start storing at offset @poffset of the first page.
    39	 */
    40	static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
    41					      loff_t block_end, int pcount,
    42					      struct page **pages, unsigned poffset,
    43					      int *errp)
    44	{
    45		unsigned int zisofs_block_shift = ISOFS_I(inode)->i_format_parm[1];
    46		unsigned int bufsize = ISOFS_BUFFER_SIZE(inode);
    47		unsigned int bufshift = ISOFS_BUFFER_BITS(inode);
    48		unsigned int bufmask = bufsize - 1;
    49		int i, block_size = block_end - block_start;
    50		z_stream stream = { .total_out = 0,
    51				    .avail_in = 0,
    52				    .avail_out = 0, };
    53		int zerr;
    54		int needblocks = (block_size + (block_start & bufmask) + bufmask)
    55					>> bufshift;
    56		int haveblocks;
    57		blkcnt_t blocknum;
    58		struct buffer_head **bhs;
    59		int curbh, curpage;
    60	
    61		if (block_size > deflateBound(1UL << zisofs_block_shift)) {
    62			*errp = -EIO;
    63			return 0;
    64		}
    65		/* Empty block? */
    66		if (block_size == 0) {
    67			for ( i = 0 ; i < pcount ; i++ ) {
    68				if (!pages[i])
    69					continue;
    70				memzero_page(pages[i], 0, PAGE_SIZE);
    71				SetPageUptodate(pages[i]);
    72			}
    73			return ((loff_t)pcount) << PAGE_SHIFT;
    74		}
    75	
    76		/* Because zlib is not thread-safe, do all the I/O at the top. */
    77		blocknum = block_start >> bufshift;
    78		bhs = kcalloc(needblocks + 1, sizeof(*bhs), GFP_KERNEL);
    79		if (!bhs) {
    80			*errp = -ENOMEM;
    81			return 0;
    82		}
    83		haveblocks = isofs_get_blocks(inode, blocknum, bhs, needblocks);
    84		ll_rw_block(REQ_OP_READ, 0, haveblocks, bhs);
    85	
    86		curbh = 0;
    87		curpage = 0;
    88		/*
    89		 * First block is special since it may be fractional.  We also wait for
    90		 * it before grabbing the zlib mutex; odds are that the subsequent
    91		 * blocks are going to come in in short order so we don't hold the zlib
    92		 * mutex longer than necessary.
    93		 */
    94	
    95		if (!bhs[0])
    96			goto b_eio;
    97	
    98		wait_on_buffer(bhs[0]);
    99		if (!buffer_uptodate(bhs[0])) {
   100			*errp = -EIO;
   101			goto b_eio;
   102		}
   103	
   104		stream.workspace = zisofs_zlib_workspace;
   105		mutex_lock(&zisofs_zlib_lock);
   106			
   107		zerr = zlib_inflateInit(&stream);
   108		if (zerr != Z_OK) {
   109			if (zerr == Z_MEM_ERROR)
   110				*errp = -ENOMEM;
   111			else
   112				*errp = -EIO;
   113			printk(KERN_DEBUG "zisofs: zisofs_inflateInit returned %d\n",
   114				       zerr);
   115			goto z_eio;
   116		}
   117	
   118		while (curpage < pcount && curbh < haveblocks &&
   119		       zerr != Z_STREAM_END) {
   120			if (!stream.avail_out) {
   121				if (pages[curpage]) {
   122					stream.next_out = kmap_local_page(pages[curpage])
   123							+ poffset;
   124					stream.avail_out = PAGE_SIZE - poffset;
   125					poffset = 0;
   126				} else {
   127					stream.next_out = (void *)&zisofs_sink_page;
   128					stream.avail_out = PAGE_SIZE;
   129				}
   130			}
   131			if (!stream.avail_in) {
   132				wait_on_buffer(bhs[curbh]);
   133				if (!buffer_uptodate(bhs[curbh])) {
   134					*errp = -EIO;
   135					break;
   136				}
   137				stream.next_in  = bhs[curbh]->b_data +
   138							(block_start & bufmask);
   139				stream.avail_in = min_t(unsigned, bufsize -
   140							(block_start & bufmask),
   141							block_size);
   142				block_size -= stream.avail_in;
   143				block_start = 0;
   144			}
   145	
   146			while (stream.avail_out && stream.avail_in) {
   147				zerr = zlib_inflate(&stream, Z_SYNC_FLUSH);
   148				if (zerr == Z_BUF_ERROR && stream.avail_in == 0)
   149					break;
   150				if (zerr == Z_STREAM_END)
   151					break;
   152				if (zerr != Z_OK) {
   153					/* EOF, error, or trying to read beyond end of input */
   154					if (zerr == Z_MEM_ERROR)
   155						*errp = -ENOMEM;
   156					else {
   157						printk(KERN_DEBUG
   158						       "zisofs: zisofs_inflate returned"
   159						       " %d, inode = %lu,"
   160						       " page idx = %d, bh idx = %d,"
   161						       " avail_in = %ld,"
   162						       " avail_out = %ld\n",
   163						       zerr, inode->i_ino, curpage,
   164						       curbh, stream.avail_in,
   165						       stream.avail_out);
   166						*errp = -EIO;
   167					}
   168					goto inflate_out;
   169				}
   170			}
   171	
   172			if (!stream.avail_out) {
   173				/* This page completed */
   174				if (pages[curpage]) {
   175					flush_dcache_page(pages[curpage]);
   176					SetPageUptodate(pages[curpage]);
   177				}
 > 178				if (stream.next_out != (char *)zisofs_sink_page) {
   179					kunmap_local(stream.next_out);
   180					stream.next_out = NULL;
   181				}
   182				curpage++;
   183			}
   184			if (!stream.avail_in)
   185				curbh++;
   186		}
   187	inflate_out:
   188		zlib_inflateEnd(&stream);
   189		if (stream.next_out && stream.next_out != (char *)zisofs_sink_page)
   190			kunmap_local(stream.next_out);
   191	
   192	z_eio:
   193		mutex_unlock(&zisofs_zlib_lock);
   194	
   195	b_eio:
   196		for (i = 0; i < haveblocks; i++)
   197			brelse(bhs[i]);
   198		kfree(bhs);
   199		return stream.total_out;
   200	}
   201	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fs/isofs: Replace kmap() with kmap_local_page()
  2022-07-31 19:01 [PATCH] fs/isofs: Replace kmap() with kmap_local_page() Fabio M. De Francesco
  2022-07-31 22:09 ` kernel test robot
  2022-07-31 22:19 ` kernel test robot
@ 2022-08-01  1:01 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-08-01  1:01 UTC (permalink / raw)
  To: Fabio M. De Francesco, Matthew Wilcox (Oracle), Jan Kara,
	Roman Gushchin, Pali Rohár, Andrew Morton, Muchun Song,
	Ira Weiny, linux-kernel
  Cc: kbuild-all, Linux Memory Management List, Fabio M. De Francesco

Hi "Fabio,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jack-fs/for_next]
[also build test WARNING on akpm-mm/mm-everything linus/master v5.19 next-20220728]
[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/Fabio-M-De-Francesco/fs-isofs-Replace-kmap-with-kmap_local_page/20220801-030233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
config: nios2-randconfig-s043-20220731 (https://download.01.org/0day-ci/archive/20220801/202208010804.HdlHVBnW-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/7c25888be273e928336283deae5b57f8ffa78a50
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Fabio-M-De-Francesco/fs-isofs-Replace-kmap-with-kmap_local_page/20220801-030233
        git checkout 7c25888be273e928336283deae5b57f8ffa78a50
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=nios2 SHELL=/bin/bash fs/isofs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> fs/isofs/compress.c:178:45: sparse: sparse: incompatible types in comparison expression (different signedness):
>> fs/isofs/compress.c:178:45: sparse:    unsigned char [usertype] *
>> fs/isofs/compress.c:178:45: sparse:    char *
   fs/isofs/compress.c:189:48: sparse: sparse: incompatible types in comparison expression (different signedness):
   fs/isofs/compress.c:189:48: sparse:    unsigned char [usertype] *
   fs/isofs/compress.c:189:48: sparse:    char *

vim +178 fs/isofs/compress.c

    34	
    35	/*
    36	 * Read data of @inode from @block_start to @block_end and uncompress
    37	 * to one zisofs block. Store the data in the @pages array with @pcount
    38	 * entries. Start storing at offset @poffset of the first page.
    39	 */
    40	static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
    41					      loff_t block_end, int pcount,
    42					      struct page **pages, unsigned poffset,
    43					      int *errp)
    44	{
    45		unsigned int zisofs_block_shift = ISOFS_I(inode)->i_format_parm[1];
    46		unsigned int bufsize = ISOFS_BUFFER_SIZE(inode);
    47		unsigned int bufshift = ISOFS_BUFFER_BITS(inode);
    48		unsigned int bufmask = bufsize - 1;
    49		int i, block_size = block_end - block_start;
    50		z_stream stream = { .total_out = 0,
    51				    .avail_in = 0,
    52				    .avail_out = 0, };
    53		int zerr;
    54		int needblocks = (block_size + (block_start & bufmask) + bufmask)
    55					>> bufshift;
    56		int haveblocks;
    57		blkcnt_t blocknum;
    58		struct buffer_head **bhs;
    59		int curbh, curpage;
    60	
    61		if (block_size > deflateBound(1UL << zisofs_block_shift)) {
    62			*errp = -EIO;
    63			return 0;
    64		}
    65		/* Empty block? */
    66		if (block_size == 0) {
    67			for ( i = 0 ; i < pcount ; i++ ) {
    68				if (!pages[i])
    69					continue;
    70				memzero_page(pages[i], 0, PAGE_SIZE);
    71				SetPageUptodate(pages[i]);
    72			}
    73			return ((loff_t)pcount) << PAGE_SHIFT;
    74		}
    75	
    76		/* Because zlib is not thread-safe, do all the I/O at the top. */
    77		blocknum = block_start >> bufshift;
    78		bhs = kcalloc(needblocks + 1, sizeof(*bhs), GFP_KERNEL);
    79		if (!bhs) {
    80			*errp = -ENOMEM;
    81			return 0;
    82		}
    83		haveblocks = isofs_get_blocks(inode, blocknum, bhs, needblocks);
    84		ll_rw_block(REQ_OP_READ, 0, haveblocks, bhs);
    85	
    86		curbh = 0;
    87		curpage = 0;
    88		/*
    89		 * First block is special since it may be fractional.  We also wait for
    90		 * it before grabbing the zlib mutex; odds are that the subsequent
    91		 * blocks are going to come in in short order so we don't hold the zlib
    92		 * mutex longer than necessary.
    93		 */
    94	
    95		if (!bhs[0])
    96			goto b_eio;
    97	
    98		wait_on_buffer(bhs[0]);
    99		if (!buffer_uptodate(bhs[0])) {
   100			*errp = -EIO;
   101			goto b_eio;
   102		}
   103	
   104		stream.workspace = zisofs_zlib_workspace;
   105		mutex_lock(&zisofs_zlib_lock);
   106			
   107		zerr = zlib_inflateInit(&stream);
   108		if (zerr != Z_OK) {
   109			if (zerr == Z_MEM_ERROR)
   110				*errp = -ENOMEM;
   111			else
   112				*errp = -EIO;
   113			printk(KERN_DEBUG "zisofs: zisofs_inflateInit returned %d\n",
   114				       zerr);
   115			goto z_eio;
   116		}
   117	
   118		while (curpage < pcount && curbh < haveblocks &&
   119		       zerr != Z_STREAM_END) {
   120			if (!stream.avail_out) {
   121				if (pages[curpage]) {
   122					stream.next_out = kmap_local_page(pages[curpage])
   123							+ poffset;
   124					stream.avail_out = PAGE_SIZE - poffset;
   125					poffset = 0;
   126				} else {
   127					stream.next_out = (void *)&zisofs_sink_page;
   128					stream.avail_out = PAGE_SIZE;
   129				}
   130			}
   131			if (!stream.avail_in) {
   132				wait_on_buffer(bhs[curbh]);
   133				if (!buffer_uptodate(bhs[curbh])) {
   134					*errp = -EIO;
   135					break;
   136				}
   137				stream.next_in  = bhs[curbh]->b_data +
   138							(block_start & bufmask);
   139				stream.avail_in = min_t(unsigned, bufsize -
   140							(block_start & bufmask),
   141							block_size);
   142				block_size -= stream.avail_in;
   143				block_start = 0;
   144			}
   145	
   146			while (stream.avail_out && stream.avail_in) {
   147				zerr = zlib_inflate(&stream, Z_SYNC_FLUSH);
   148				if (zerr == Z_BUF_ERROR && stream.avail_in == 0)
   149					break;
   150				if (zerr == Z_STREAM_END)
   151					break;
   152				if (zerr != Z_OK) {
   153					/* EOF, error, or trying to read beyond end of input */
   154					if (zerr == Z_MEM_ERROR)
   155						*errp = -ENOMEM;
   156					else {
   157						printk(KERN_DEBUG
   158						       "zisofs: zisofs_inflate returned"
   159						       " %d, inode = %lu,"
   160						       " page idx = %d, bh idx = %d,"
   161						       " avail_in = %ld,"
   162						       " avail_out = %ld\n",
   163						       zerr, inode->i_ino, curpage,
   164						       curbh, stream.avail_in,
   165						       stream.avail_out);
   166						*errp = -EIO;
   167					}
   168					goto inflate_out;
   169				}
   170			}
   171	
   172			if (!stream.avail_out) {
   173				/* This page completed */
   174				if (pages[curpage]) {
   175					flush_dcache_page(pages[curpage]);
   176					SetPageUptodate(pages[curpage]);
   177				}
 > 178				if (stream.next_out != (char *)zisofs_sink_page) {
   179					kunmap_local(stream.next_out);
   180					stream.next_out = NULL;
   181				}
   182				curpage++;
   183			}
   184			if (!stream.avail_in)
   185				curbh++;
   186		}
   187	inflate_out:
   188		zlib_inflateEnd(&stream);
   189		if (stream.next_out && stream.next_out != (char *)zisofs_sink_page)
   190			kunmap_local(stream.next_out);
   191	
   192	z_eio:
   193		mutex_unlock(&zisofs_zlib_lock);
   194	
   195	b_eio:
   196		for (i = 0; i < haveblocks; i++)
   197			brelse(bhs[i]);
   198		kfree(bhs);
   199		return stream.total_out;
   200	}
   201	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-01  1:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-31 19:01 [PATCH] fs/isofs: Replace kmap() with kmap_local_page() Fabio M. De Francesco
2022-07-31 22:09 ` kernel test robot
2022-07-31 22:19 ` kernel test robot
2022-08-01  1:01 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox