| Message ID | 0a165ac0-1912-aebd-8a0d-b42e7cd1aea1@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | [v3] ext4: fix log printing of ext4_mb_regular_allocator() | expand |
On Aug 14, 2020, at 6:10 PM, brookxu <brookxu.cn@gmail.com> wrote: > > Fix log printing of ext4_mb_regular_allocator()，it may be an > unintentional behavior. > > V3: > It may be better to add a comma between start and len, which is > convenient for script processing. > > V2: > Add more valuable information, such as group, start, len, lost. > > Signed-off-by: Chunguang Xu <brookxu@tencent.com> A very minor cleanup possible, but not really worthwhile to resubmit if Ted wants to apply it. Either way, it looks good. Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > fs/ext4/mballoc.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index c0a331e..70b110f 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -2218,6 +2218,7 @@ static int ext4_mb_good_group_nolock(struct ext4_allocation_context *ac, > struct ext4_sb_info *sbi; > struct super_block *sb; > struct ext4_buddy e4b; > + unsigned int lost; > > sb = ac->ac_sb; > sbi = EXT4_SB(sb); > @@ -2341,22 +2342,24 @@ static int ext4_mb_good_group_nolock(struct ext4_allocation_context *ac, > * We've been searching too long. Let's try to allocate > * the best chunk we've found so far > */ > - > ext4_mb_try_best_found(ac, &e4b); > if (ac->ac_status != AC_STATUS_FOUND) { > /* > * Someone more lucky has already allocated it. > * The only thing we can do is just take first > * found block(s) > - printk(KERN_DEBUG "EXT4-fs: someone won our chunk\n"); > */ > + lost = (unsigned int)atomic_inc_return(&sbi->s_mb_lost_chunks); (minor) no need for a typecast here? The return type is already "int", so it would be better to declare "int lost" and that would make the line a bit shorter. > + mb_debug(sb, "lost chunk, group: %u, start: %d, len: %d, lost: %u\n", > + ac->ac_b_ex.fe_group, ac->ac_b_ex.fe_start, > + ac->ac_b_ex.fe_len, lost); > + > ac->ac_b_ex.fe_group = 0; > ac->ac_b_ex.fe_start = 0; > ac->ac_b_ex.fe_len = 0; > ac->ac_status = AC_STATUS_CONTINUE; > ac->ac_flags |= EXT4_MB_HINT_FIRST; > cr = 3; > - atomic_inc(&sbi->s_mb_lost_chunks); > goto repeat; > } > } > -- > 1.8.3.1 > > Cheers, Andreas
On 8/15/20 5:40 AM, brookxu wrote: > Fix log printing of ext4_mb_regular_allocator()，it may be an > unintentional behavior. > > V3: > It may be better to add a comma between start and len, which is > convenient for script processing. > > V2: > Add more valuable information, such as group, start, len, lost. > > Signed-off-by: Chunguang Xu <brookxu@tencent.com> LGTM, please feel free to add: Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com> > --- > fs/ext4/mballoc.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > index c0a331e..70b110f 100644 > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c > @@ -2218,6 +2218,7 @@ static int ext4_mb_good_group_nolock(struct ext4_allocation_context *ac, > struct ext4_sb_info *sbi; > struct super_block *sb; > struct ext4_buddy e4b; > + unsigned int lost; > > sb = ac->ac_sb; > sbi = EXT4_SB(sb); > @@ -2341,22 +2342,24 @@ static int ext4_mb_good_group_nolock(struct ext4_allocation_context *ac, > * We've been searching too long. Let's try to allocate > * the best chunk we've found so far > */ > - > ext4_mb_try_best_found(ac, &e4b); > if (ac->ac_status != AC_STATUS_FOUND) { > /* > * Someone more lucky has already allocated it. > * The only thing we can do is just take first > * found block(s) > - printk(KERN_DEBUG "EXT4-fs: someone won our chunk\n"); > */ > + lost = (unsigned int)atomic_inc_return(&sbi->s_mb_lost_chunks); > + mb_debug(sb, "lost chunk, group: %u, start: %d, len: %d, lost: %u\n", > + ac->ac_b_ex.fe_group, ac->ac_b_ex.fe_start, > + ac->ac_b_ex.fe_len, lost); > + > ac->ac_b_ex.fe_group = 0; > ac->ac_b_ex.fe_start = 0; > ac->ac_b_ex.fe_len = 0; > ac->ac_status = AC_STATUS_CONTINUE; > ac->ac_flags |= EXT4_MB_HINT_FIRST; > cr = 3; > - atomic_inc(&sbi->s_mb_lost_chunks); > goto repeat; > } > } >
On Sat, Aug 15, 2020 at 08:10:44AM +0800, brookxu wrote: > Fix log printing of ext4_mb_regular_allocator()，it may be an > unintentional behavior. Thanks, applied with Andreas's suggested improvement and with a reworded commit description. - Ted
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index c0a331e..70b110f 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -2218,6 +2218,7 @@ static int ext4_mb_good_group_nolock(struct ext4_allocation_context *ac, struct ext4_sb_info *sbi; struct super_block *sb; struct ext4_buddy e4b; + unsigned int lost; sb = ac->ac_sb; sbi = EXT4_SB(sb); @@ -2341,22 +2342,24 @@ static int ext4_mb_good_group_nolock(struct ext4_allocation_context *ac, * We've been searching too long. Let's try to allocate * the best chunk we've found so far */ - ext4_mb_try_best_found(ac, &e4b); if (ac->ac_status != AC_STATUS_FOUND) { /* * Someone more lucky has already allocated it. * The only thing we can do is just take first * found block(s) - printk(KERN_DEBUG "EXT4-fs: someone won our chunk\n"); */ + lost = (unsigned int)atomic_inc_return(&sbi->s_mb_lost_chunks); + mb_debug(sb, "lost chunk, group: %u, start: %d, len: %d, lost: %u\n", + ac->ac_b_ex.fe_group, ac->ac_b_ex.fe_start, + ac->ac_b_ex.fe_len, lost); + ac->ac_b_ex.fe_group = 0; ac->ac_b_ex.fe_start = 0; ac->ac_b_ex.fe_len = 0; ac->ac_status = AC_STATUS_CONTINUE; ac->ac_flags |= EXT4_MB_HINT_FIRST; cr = 3; - atomic_inc(&sbi->s_mb_lost_chunks); goto repeat; } }
Fix log printing of ext4_mb_regular_allocator()，it may be an unintentional behavior. V3: It may be better to add a comma between start and len, which is convenient for script processing. V2: Add more valuable information, such as group, start, len, lost. Signed-off-by: Chunguang Xu <brookxu@tencent.com> --- fs/ext4/mballoc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)