All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] isofs compress: Remove VLA usage
@ 2018-04-04 23:28 Kyle Spiers
  2018-04-05  7:11 ` kbuild test robot
  0 siblings, 1 reply; 2+ messages in thread
From: Kyle Spiers @ 2018-04-04 23:28 UTC (permalink / raw)
  To: jack; +Cc: arnd, dhowells, viro, gregkh, keescook, linux-kernel, Kyle Spiers

As part of the effort to remove VLAs from the kernel[1], this changes
the allocation of the bhs and pages arrays from being on the stack to being
kcalloc()ed. This also allows for the removal of the explicit zeroing
of bhs.

https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Kyle Spiers <ksspiers@google.com>
---
 fs/isofs/compress.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/isofs/compress.c b/fs/isofs/compress.c
index 9bb2fe35799d..39cc99aecff8 100644
--- a/fs/isofs/compress.c
+++ b/fs/isofs/compress.c
@@ -59,7 +59,7 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 				>> bufshift;
 	int haveblocks;
 	blkcnt_t blocknum;
-	struct buffer_head *bhs[needblocks + 1];
+	struct buffer_head **bhs;
 	int curbh, curpage;
 
 	if (block_size > deflateBound(1UL << zisofs_block_shift)) {
@@ -80,7 +80,9 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 
 	/* Because zlib is not thread-safe, do all the I/O at the top. */
 	blocknum = block_start >> bufshift;
-	memset(bhs, 0, (needblocks + 1) * sizeof(struct buffer_head *));
+	bhs = kcalloc(needblocks + 1, sizeof(*bhs), GFP_KERNEL);
+	if (!bhs)
+		return -ENOMEM;
 	haveblocks = isofs_get_blocks(inode, blocknum, bhs, needblocks);
 	ll_rw_block(REQ_OP_READ, 0, haveblocks, bhs);
 
@@ -190,6 +192,7 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 b_eio:
 	for (i = 0; i < haveblocks; i++)
 		brelse(bhs[i]);
+	kfree(bhs);
 	return stream.total_out;
 }
 
@@ -305,7 +308,7 @@ static int zisofs_readpage(struct file *file, struct page *page)
 	unsigned int zisofs_pages_per_cblock =
 		PAGE_SHIFT <= zisofs_block_shift ?
 		(1 << (zisofs_block_shift - PAGE_SHIFT)) : 0;
-	struct page *pages[max_t(unsigned, zisofs_pages_per_cblock, 1)];
+	struct page **pages;
 	pgoff_t index = page->index, end_index;
 
 	end_index = (inode->i_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
@@ -330,6 +333,10 @@ static int zisofs_readpage(struct file *file, struct page *page)
 		full_page = 0;
 		pcount = 1;
 	}
+	pages = kcalloc(max_t(unsigned int, zisofs_pages_per_cblock, 1),
+					sizeof(*pages), GFP_KERNEL);
+	if (!pages)
+		return -ENOMEM;
 	pages[full_page] = page;
 
 	for (i = 0; i < pcount; i++, index++) {
@@ -357,6 +364,7 @@ static int zisofs_readpage(struct file *file, struct page *page)
 	}			
 
 	/* At this point, err contains 0 or -EIO depending on the "critical" page */
+	kfree(pages);
 	return err;
 }
 
-- 
2.17.0.484.g0c8726318c-goog

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

* Re: [PATCH] isofs compress: Remove VLA usage
  2018-04-04 23:28 [PATCH] isofs compress: Remove VLA usage Kyle Spiers
@ 2018-04-05  7:11 ` kbuild test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kbuild test robot @ 2018-04-05  7:11 UTC (permalink / raw)
  To: Kyle Spiers
  Cc: kbuild-all, jack, arnd, dhowells, viro, gregkh, keescook,
	linux-kernel, Kyle Spiers

[-- Attachment #1: Type: text/plain, Size: 7191 bytes --]

Hi Kyle,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16 next-20180404]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kyle-Spiers/isofs-compress-Remove-VLA-usage/20180405-142535
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

   fs//isofs/compress.c: In function 'zisofs_uncompress_block':
>> fs//isofs/compress.c:83:8: error: implicit declaration of function 'kcalloc'; did you mean 'vzalloc'? [-Werror=implicit-function-declaration]
     bhs = kcalloc(needblocks + 1, sizeof(*bhs), GFP_KERNEL);
           ^~~~~~~
           vzalloc
>> fs//isofs/compress.c:83:6: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     bhs = kcalloc(needblocks + 1, sizeof(*bhs), GFP_KERNEL);
         ^
>> fs//isofs/compress.c:195:2: error: implicit declaration of function 'kfree'; did you mean 'vfree'? [-Werror=implicit-function-declaration]
     kfree(bhs);
     ^~~~~
     vfree
   fs//isofs/compress.c: In function 'zisofs_readpage':
   fs//isofs/compress.c:336:8: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     pages = kcalloc(max_t(unsigned int, zisofs_pages_per_cblock, 1),
           ^
   cc1: some warnings being treated as errors

vim +83 fs//isofs/compress.c

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

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52934 bytes --]

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

end of thread, other threads:[~2018-04-05  7:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-04 23:28 [PATCH] isofs compress: Remove VLA usage Kyle Spiers
2018-04-05  7:11 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.