On Mon, Apr 03, 2023 at 06:10:43PM +0200, Jan Kara wrote: > On Thu 23-03-23 10:32:59, Jiapeng Chong wrote: > > Variable 'err' set but not used. > > > > fs/buffer.c:2613:2: warning: Value stored to 'err' is never read. > > > > Reported-by: Abaci Robot <abaci@xxxxxxxxxxxxxxxxx> > > Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4589 > > Signed-off-by: Jiapeng Chong <jiapeng.chong@xxxxxxxxxxxxxxxxx> > > I don't think the patch is quite correct (Christian, please drop it if I'm > correct). See below: Thank you for taking a look at this! > > > diff --git a/fs/buffer.c b/fs/buffer.c > > index d759b105c1e7..b3eb905f87d6 100644 > > --- a/fs/buffer.c > > +++ b/fs/buffer.c > > @@ -2580,7 +2580,7 @@ int block_truncate_page(struct address_space *mapping, > > struct inode *inode = mapping->host; > > struct page *page; > > struct buffer_head *bh; > > - int err; > > + int err = 0; > > > > blocksize = i_blocksize(inode); > > length = offset & (blocksize - 1); > > @@ -2593,9 +2593,8 @@ int block_truncate_page(struct address_space *mapping, > > iblock = (sector_t)index << (PAGE_SHIFT - inode->i_blkbits); > > > > page = grab_cache_page(mapping, index); > > - err = -ENOMEM; > > if (!page) > > - goto out; > > + return -ENOMEM; > > > > if (!page_has_buffers(page)) > > create_empty_buffers(page, blocksize, 0); > > @@ -2609,7 +2608,6 @@ int block_truncate_page(struct address_space *mapping, > > pos += blocksize; > > } > > > > - err = 0; > > if (!buffer_mapped(bh)) { > > WARN_ON(bh->b_size != blocksize); > > err = get_block(inode, iblock, bh, 0); > > @@ -2633,12 +2631,11 @@ int block_truncate_page(struct address_space *mapping, > > > > zero_user(page, offset, length); > > mark_buffer_dirty(bh); > > - err = 0; > > There is: > > if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh)) > err = -EIO; > > above this assignment. So now we'll be returning -EIO if > block_truncate_page() needs to read the block AFAICT. Did this pass fstests > with some filesystem exercising this code (ext2 driver comes to mind)? Hm, the code in current mainline is: if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh)) { err = bh_read(bh, 0); /* Uhhuh. Read error. Complain and punt. */ if (err < 0) goto unlock; } Before e7ea1129afab ("fs/buffer: replace ll_rw_block()") that code really was if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh)) { err = -EIO; ll_rw_block(REQ_OP_READ, 1, &bh); wait_on_buffer(bh); /* Uhhuh. Read error. Complain and punt. */ if (!buffer_uptodate(bh)) goto unlock; } which would've indeed caused this change to return -EIO. Is this still an issue now? Sorry if I'm being dense here.
