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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxx>
SUSE Labs, CR
