2020-08-07 14:08:23

by brookxu.cn

[permalink] [raw]
Subject: [PATCH] ext4: fix log printing of ext4_mb_regular_allocator()

Fix log printing of ext4_mb_regular_allocator(),it may be an
unintentional behavior.

Signed-off-by: Chunguang Xu <[email protected]>
---
fs/ext4/mballoc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 5d4a1be..b0da525 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2324,15 +2324,14 @@ 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");
*/
+ mb_debug(sb, "EXT4-fs: someone won our chunk\n");
ac->ac_b_ex.fe_group = 0;
ac->ac_b_ex.fe_start = 0;
ac->ac_b_ex.fe_len = 0;
--
1.8.3.1


2020-08-12 19:36:53

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix log printing of ext4_mb_regular_allocator()

On Aug 7, 2020, at 8:01 AM, brookxu <[email protected]> wrote:
>
> Fix log printing of ext4_mb_regular_allocator(),it may be an
> unintentional behavior.
>
> Signed-off-by: Chunguang Xu <[email protected]>

The debug message would probably be more useful if it included some
actual information (PID, status, fe_group, fe_start, fe_len), but
that isn't necessarily a problem with this patch itself.

Reviewed-by: Andreas Dilger <[email protected]>


> ---
> fs/ext4/mballoc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 5d4a1be..b0da525 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2324,15 +2324,14 @@ 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");
> */
> + mb_debug(sb, "EXT4-fs: someone won our chunk\n");
> ac->ac_b_ex.fe_group = 0;
> ac->ac_b_ex.fe_start = 0;
> ac->ac_b_ex.fe_len = 0;
> --
> 1.8.3.1


Cheers, Andreas






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP

2020-08-13 14:05:32

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix log printing of ext4_mb_regular_allocator()



On 8/7/20 7:31 PM, brookxu wrote:
> Fix log printing of ext4_mb_regular_allocator(),it may be an
> unintentional behavior.
>
> Signed-off-by: Chunguang Xu <[email protected]>
> ---
> fs/ext4/mballoc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 5d4a1be..b0da525 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2324,15 +2324,14 @@ 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");
> */
> + mb_debug(sb, "EXT4-fs: someone won our chunk\n");

mb_debug() already adds "EXT4-fs" string. So we need not add that here.
but maybe we can add "sbi->s_mb_lost_chunks" in this msg, which may be
helpful debug msg if too many lost chunks.


> ac->ac_b_ex.fe_group = 0;
> ac->ac_b_ex.fe_start = 0;
> ac->ac_b_ex.fe_len = 0;
>

2020-08-14 01:15:04

by brookxu.cn

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix log printing of ext4_mb_regular_allocator()

This suggestion is good, I try to update it in the next version.

thanks.

Andreas Dilger wrote on 2020/8/13 3:36:
> On Aug 7, 2020, at 8:01 AM, brookxu <[email protected]> wrote:
>>
>> Fix log printing of ext4_mb_regular_allocator(),it may be an
>> unintentional behavior.
>>
>> Signed-off-by: Chunguang Xu <[email protected]>
>
> The debug message would probably be more useful if it included some
> actual information (PID, status, fe_group, fe_start, fe_len), but
> that isn't necessarily a problem with this patch itself.
>
> Reviewed-by: Andreas Dilger <[email protected]>
>
>
>> ---
>> fs/ext4/mballoc.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 5d4a1be..b0da525 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -2324,15 +2324,14 @@ 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");
>> */
>> + mb_debug(sb, "EXT4-fs: someone won our chunk\n");
>> ac->ac_b_ex.fe_group = 0;
>> ac->ac_b_ex.fe_start = 0;
>> ac->ac_b_ex.fe_len = 0;
>> --
>> 1.8.3.1
>
>
> Cheers, Andreas
>
>
>
>
>

2020-08-14 01:20:13

by brookxu.cn

[permalink] [raw]
Subject: Re: [PATCH] ext4: fix log printing of ext4_mb_regular_allocator()

This maybe a problem, I try to update it in the next version, thank you.

Ritesh Harjani wrote on 2020/8/13 22:04:
>
>
> On 8/7/20 7:31 PM, brookxu wrote:
>> Fix log printing of ext4_mb_regular_allocator(),it may be an
>> unintentional behavior.
>>
>> Signed-off-by: Chunguang Xu <[email protected]>
>> ---
>>   fs/ext4/mballoc.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 5d4a1be..b0da525 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -2324,15 +2324,14 @@ 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");
>>                */
>> +            mb_debug(sb, "EXT4-fs: someone won our chunk\n");
>
> mb_debug() already adds "EXT4-fs" string. So we need not add that here.
> but maybe we can add "sbi->s_mb_lost_chunks" in this msg, which may be
> helpful debug msg if too many lost chunks.
>
>
>>               ac->ac_b_ex.fe_group = 0;
>>               ac->ac_b_ex.fe_start = 0;
>>               ac->ac_b_ex.fe_len = 0;
>>