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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 14 Sep 2022 10:44:16 +0200 From: Jan Kara <jack@...e.cz> To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com> Cc: Yuan Can <yuancan@...wei.com>, jack@...e.com, linux-ext4@...r.kernel.org Subject: Re: [PATCH] fs/ext2: remove unused variable es On Tue 13-09-22 20:20:50, Ritesh Harjani (IBM) wrote: > On 22/09/13 07:11AM, Yuan Can wrote: > > The variable es is never used, remove it. > > No functional change. > > > > Signed-off-by: Yuan Can <yuancan@...wei.com> > > --- > > fs/ext2/ialloc.c | 2 -- > > 1 file changed, 2 deletions(-) > > Hi Yuan, > > Thanks for the patch - > > However while reviewing this, I also looked at ext2_count_free_blocks(). > And then I felt maybe the right thing to do is to print more info when > EXT2FS_DEBUG is enabled which would be to dump both stored counters in the debug message > i.e. (from ext2_super_block -> s_free_**_count, and from ext2_sb_info -> s_free**_counter) I don't have a strong opinion, but yeah, what you suggest makes sense. Honza > diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c > index c17ccc19b938..87c57ddcd2ed 100644 > --- a/fs/ext2/balloc.c > +++ b/fs/ext2/balloc.c > @@ -1475,8 +1475,10 @@ unsigned long ext2_count_free_blocks (struct super_block * sb) > bitmap_count += x; > brelse(bitmap_bh); > } > - printk("ext2_count_free_blocks: stored = %lu, computed = %lu, %lu\n", > - (long)le32_to_cpu(es->s_free_blocks_count), > + printk("ext2_count_free_blocks: stored = %lu, %lu, computed = %lu, %lu\n", > + (unsigned long) le32_to_cpu(es->s_free_blocks_count), > + (unsigned long) > + percpu_counter_read(&EXT2_SB(sb)->s_freeblocks_counter), > desc_count, bitmap_count); > return bitmap_count; > #else > diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c > index 998dd2ac8008..436d5c4d61c0 100644 > --- a/fs/ext2/ialloc.c > +++ b/fs/ext2/ialloc.c > @@ -643,7 +643,8 @@ unsigned long ext2_count_free_inodes (struct super_block * sb) > bitmap_count += x; > } > brelse(bitmap_bh); > - printk("ext2_count_free_inodes: stored = %lu, computed = %lu, %lu\n", > + printk("ext2_count_free_inodes: stored = %lu, %lu, computed = %lu, %lu\n", > + (unsigned long) le32_to_cpu(es->s_free_inodes_count), > (unsigned long) > percpu_counter_read(&EXT2_SB(sb)->s_freeinodes_counter), > desc_count, bitmap_count); > > @Jan, > Please do let me know your thoughts on this. This doesn't changes the functionality, > since the return value remains the same. But it dumps both stored counter values > in debug output, which is what I think the intended behaviour of the print > should be in the first place. > > If this looks correct to you - I can send an official patch fixing this. > > -ritesh > > > > > diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c > > index 998dd2ac8008..951b80a7f7d2 100644 > > --- a/fs/ext2/ialloc.c > > +++ b/fs/ext2/ialloc.c > > @@ -620,11 +620,9 @@ unsigned long ext2_count_free_inodes (struct super_block * sb) > > int i; > > > > #ifdef EXT2FS_DEBUG > > - struct ext2_super_block *es; > > unsigned long bitmap_count = 0; > > struct buffer_head *bitmap_bh = NULL; > > > > - es = EXT2_SB(sb)->s_es; > > for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) { > > unsigned x; > > > > -- > > 2.17.1 > > -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists