Re: [PATCH] isofs compress: Remove VLA usage

[kbuild test robot]

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,
45loff_t block_end, int pcount,
46struct page **pages, unsigned 
poffset,
47int *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  

Re: [PATCH] isofs compress: Remove VLA usage

[kbuild test robot]

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,
45loff_t block_end, int pcount,
46struct page **pages, unsigned 
poffset,
47int *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  

[PATCH] isofs compress: Remove VLA usage

[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 
---
 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

[PATCH] isofs compress: Remove VLA usage

[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 
---
 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