ext4: simplify the code a bit
RFC 1/1 ext4: simplify the code a bit
Hi guys,
I noticed the following warning in a linux-next build:
fs/ext4/inode.c: In function 'ext4_da_writepages':fs/ext4/inode.c:2212:6: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]
In tracking this down, I followed the call chains and discovered that io_submit_init_bio() only ever returned 0. Switching that to return void and following back up the chain lead to the following patch. This makes it far more obvious that by the end of the loop in mpage_map_and_submit_extent(), err must be zero, which allows the final removal of err2 and the check that caused the above warning.
This does remove the above warning and simplifies the code a bit, but may be removing error infrastructure that may be used in the future.
--- fs/ext4/ext4.h | 8 ++++---- fs/ext4/inode.c | 46 +++++++++++++--------------------------------- fs/ext4/page-io.c | 35 ++++++----------------------------- 3 files changed, 23 insertions(+), 66 deletions(-)
This is against today's ext4 tree dev branch.
fs/ext4/ext4.h +4 -4
@@ -2660,10 +2660,10 @@ extern void ext4_io_submit_init(struct ext4_io_submit *io,extern void ext4_end_io_rsv_work (struct work_struct *work);extern void ext4_end_io_unrsv_work (struct work_struct *work);extern void ext4_io_submit (struct ext4_io_submit *io); -extern int ext4_bio_write_page (struct ext4_io_submit *io, -struct page *page, -int len, -struct writeback_control *wbc); +extern void ext4_bio_write_page (struct ext4_io_submit *io, +struct page *page, +int len, +struct writeback_control *wbc);/* mmp.c */ extern int ext4_multi_mount_protect (struct super_block *,ext4_fsblk_t );
fs/ext4/inode.c +13 -33
129 lines not on this page · raw message
fs/ext4/page-io.c +6 -29
@@ -365,7 +365,7 @@ void ext4_io_submit_init(struct ext4_io_submit *io, io->io_end =NULL ; } -static int io_submit_init_bio (struct ext4_io_submit *io, +static void io_submit_init_bio (struct ext4_io_submit *io,struct buffer_head *bh) {int nvecs =bio_get_nr_vecs (bh->b_bdev);
@@ -378,10 +378,9 @@ static int io_submit_init_bio(struct ext4_io_submit *io, bio->bi_private =ext4_get_io_end (io->io_end); io->io_bio = bio; io->io_next_block = bh->b_blocknr; -return 0 ; } -static int io_submit_add_bh (struct ext4_io_submit *io, +static void io_submit_add_bh (struct ext4_io_submit *io,struct inode *inode,struct buffer_head *bh) {
@@ -391,19 +390,15 @@ static int io_submit_add_bh(struct ext4_io_submit *io, submit_and_retry:ext4_io_submit (io); } -if (io->io_bio ==NULL ) { - ret =io_submit_init_bio (io, bh); -if (ret) -return ret; - } +if (io->io_bio ==NULL ) +io_submit_init_bio (io, bh); ret =bio_add_page (io->io_bio, bh->b_page, bh->b_size,bh_offset (bh));if (ret != bh->b_size) goto submit_and_retry; io->io_next_block++; -return 0 ; } -int ext4_bio_write_page (struct ext4_io_submit *io, +void ext4_bio_write_page (struct ext4_io_submit *io,struct page *page,int len,struct writeback_control *wbc)
@@ -411,7 +406,6 @@ int ext4_bio_write_page(struct ext4_io_submit *io,struct inode *inode = page->mapping->host;unsigned block_start, blocksize;struct buffer_head *bh, *head; -int ret =0 ;int nr_submitted =0 ; blocksize =1 << inode->i_blkbits;
@@ -471,30 +465,13 @@ int ext4_bio_write_page(struct ext4_io_submit *io,do {if (!buffer_async_write (bh))continue ; - ret =io_submit_add_bh (io, inode, bh); -if (ret) { -/* -* We only get here on ENOMEM. Not much else -* we can do but mark the page as dirty, and -* better luck next time. -*/ -redirty_page_for_writepage (wbc, page); -break ; - } +io_submit_add_bh (io, inode, bh); nr_submitted++;clear_buffer_dirty (bh); }while ((bh = bh->b_this_page) != head); -/* Error stopped previous loop? Clean up buffers... */ -if (ret) { -do { -clear_buffer_async_write (bh); - bh = bh->b_this_page; - }while (bh != head); - }unlock_page (page);/* Nothing submitted - we have to end page writeback */ if (!nr_submitted)end_page_writeback (page); -return ret; }
Signature · 5 lines
1.8.1 -- Cheers, Stephen Rothwell [email protected]
Theodore Ts'o
maintainerPATCH 1/1 ext4: add check to io_submit_init_bio
Quoted 4 lines · Stephen Rothwell
In tracking this down, I followed the call chains and discovered that
io_submit_init_bio()only ever returned 0. Switching that to return void and following back up the chain lead to the following patch.
Actually, that's a bug. We're missing an check in that function. Thanks for looking at this code. (I'll fix the uninitialized err warning another way.)
- Ted
commit 7745883f99c0e81e6bc71527d23ca363eb125a3f
Author: Theodore Ts'o <[email protected]> Date: Thu Jun 6 10:18:22 2013 -0400
ext4: add check to io_submit_init_bio
The bio_alloc() function can return NULL if the memory allocation fails. So we need to check for this.
fs/ext4/page-io.c +2 -0
@@ -372,6 +372,8 @@ static int io_submit_init_bio(struct ext4_io_submit *io,struct bio *bio; bio =bio_alloc (GFP_NOIO ,min (nvecs,BIO_MAX_PAGES )); +if (!bio) +return -ENOMEM ; bio->bi_sector = bh->b_blocknr * (bh->b_size >>9 ); bio->bi_bdev = bh->b_bdev; bio->bi_end_io = ext4_end_bio;
Re: [PATCH] ext4: add check to io_submit_init_bio
Hi Ted,
Quoted 10 lines · On Thu, 6 Jun 2013 10:19:41 -0400 Theodore Ts'o <[email protected]> wrote:
Quoted 4 lines · Stephen Rothwell
In tracking this down, I followed the call chains and discovered that
io_submit_init_bio()only ever returned 0. Switching that to return void and following back up the chain lead to the following patch.Actually, that's a bug. We're missing an check in that function. Thanks for looking at this code. (I'll fix the uninitialized err warning another way.)
Glad to be of service.
Signature · 2 lines
Cheers, Stephen Rothwell [email protected]