lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130621163236.GN13932@quack.suse.cz> Date: Fri, 21 Jun 2013 18:32:36 +0200 From: Jan Kara <jack@...e.cz> To: Theodore Ts'o <tytso@....edu> Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>, Jan Kara <jack@...e.cz> Subject: Re: [PATCH 1/2] ext4: remove while loop in mpage_map_and_submit_extent() On Mon 17-06-13 09:58:07, Ted Tso wrote: > The while loop in mpage_map_and_submit_extent() is pointless; this > function is only called if mpd.map.m_len is non-zero, and at the end > of the while loop, mpage_map_and_submit_buffers sets mpd.map.m_len to > be zero on success. (And on a failure, we return out of the while > loop anyway.) This is not true. In case blocksize < pagesize and we were not able to map a full page, mpage_map_and_submit_buffers() changes mpd->map to point to the next extent inside the page that needs mapping. In that case we can take several iterations of the loop. Maybe this fact deserves a comment before the loop but there's a comment before the callsite of mpage_map_and_submit_buffers() which mentions that it will get us next extent to map and also the documentation of mpage_map_and_submit_buffers() explicitely tells that it can update mpd->map. Honza > Hence, the body of the while loop is guaranteed to execute once and > exactly once. This also fixes a gcc warning complaining that err > might be left uninitialized. > > Signed-off-by: "Theodore Ts'o" <tytso@....edu> > Cc: Jan Kara <jack@...e.cz> > --- > fs/ext4/inode.c | 66 ++++++++++++++++++++++++++++----------------------------- > 1 file changed, 32 insertions(+), 34 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 2644679..0e61543 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -2157,44 +2157,42 @@ static int mpage_map_and_submit_extent(handle_t *handle, > > mpd->io_submit.io_end->offset = > ((loff_t)map->m_lblk) << inode->i_blkbits; > - while (map->m_len) { > - err = mpage_map_one_extent(handle, mpd); > - if (err < 0) { > - struct super_block *sb = inode->i_sb; > + err = mpage_map_one_extent(handle, mpd); > + if (err < 0) { > + struct super_block *sb = inode->i_sb; > > - /* > - * Need to commit transaction to free blocks. Let upper > - * layers sort it out. > - */ > - if (err == -ENOSPC && ext4_count_free_clusters(sb)) > - return -ENOSPC; > - > - if (!(EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED)) { > - ext4_msg(sb, KERN_CRIT, > - "Delayed block allocation failed for " > - "inode %lu at logical offset %llu with" > - " max blocks %u with error %d", > - inode->i_ino, > - (unsigned long long)map->m_lblk, > - (unsigned)map->m_len, err); > - ext4_msg(sb, KERN_CRIT, > - "This should not happen!! Data will " > - "be lost\n"); > - if (err == -ENOSPC) > - ext4_print_free_blocks(inode); > - } > - /* invalidate all the pages */ > - mpage_release_unused_pages(mpd, true); > - return err; > - } > /* > - * Update buffer state, submit mapped pages, and get us new > - * extent to map > + * Need to commit transaction to free blocks. Let upper > + * layers sort it out. > */ > - err = mpage_map_and_submit_buffers(mpd); > - if (err < 0) > - return err; > + if (err == -ENOSPC && ext4_count_free_clusters(sb)) > + return -ENOSPC; > + > + if (!(EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED)) { > + ext4_msg(sb, KERN_CRIT, > + "Delayed block allocation failed for " > + "inode %lu at logical offset %llu with" > + " max blocks %u with error %d", > + inode->i_ino, > + (unsigned long long)map->m_lblk, > + (unsigned)map->m_len, err); > + ext4_msg(sb, KERN_CRIT, > + "This should not happen!! Data will " > + "be lost\n"); > + if (err == -ENOSPC) > + ext4_print_free_blocks(inode); > + } > + /* invalidate all the pages */ > + mpage_release_unused_pages(mpd, true); > + return err; > } > + /* > + * Update buffer state, submit mapped pages, and get us new > + * extent to map > + */ > + err = mpage_map_and_submit_buffers(mpd); > + if (err < 0) > + return err; > > /* Update on-disk size after IO is submitted */ > disksize = ((loff_t)mpd->first_page) << PAGE_CACHE_SHIFT; > -- > 1.7.12.rc0.22.gcdd159b > -- Jan Kara <jack@...e.cz> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists