* [PATCH] fs/ext2: remove unused variable es @ 2022-09-13 7:11 Yuan Can 2022-09-13 14:50 ` Ritesh Harjani (IBM) 0 siblings, 1 reply; 3+ messages in thread From: Yuan Can @ 2022-09-13 7:11 UTC (permalink / raw) To: jack, linux-ext4; +Cc: yuancan The variable es is never used, remove it. No functional change. Signed-off-by: Yuan Can <yuancan@huawei.com> --- fs/ext2/ialloc.c | 2 -- 1 file changed, 2 deletions(-) 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fs/ext2: remove unused variable es 2022-09-13 7:11 [PATCH] fs/ext2: remove unused variable es Yuan Can @ 2022-09-13 14:50 ` Ritesh Harjani (IBM) 2022-09-14 8:44 ` Jan Kara 0 siblings, 1 reply; 3+ messages in thread From: Ritesh Harjani (IBM) @ 2022-09-13 14:50 UTC (permalink / raw) To: Yuan Can; +Cc: jack, linux-ext4 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@huawei.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) 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 > ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fs/ext2: remove unused variable es 2022-09-13 14:50 ` Ritesh Harjani (IBM) @ 2022-09-14 8:44 ` Jan Kara 0 siblings, 0 replies; 3+ messages in thread From: Jan Kara @ 2022-09-14 8:44 UTC (permalink / raw) To: Ritesh Harjani (IBM); +Cc: Yuan Can, jack, linux-ext4 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@huawei.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@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-14 8:44 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-13 7:11 [PATCH] fs/ext2: remove unused variable es Yuan Can 2022-09-13 14:50 ` Ritesh Harjani (IBM) 2022-09-14 8:44 ` Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox