From: Jing Zhang <[email protected]>
Date: Wed Mar 25 22:55:04 2010
Checking bug seems not at right place, and the function itself should
be inlined.
Cc: Theodore Ts'o <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: Dave Kleikamp <[email protected]>
Cc: "Aneesh Kumar K. V" <[email protected]>
Signed-off-by: Jing Zhang <[email protected]>
---
--- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
+++ ext4_mm_leak/mballoc-10.c 2010-03-25 22:44:00.000000000 +0800
@@ -2786,9 +2786,7 @@ out_err:
static void ext4_mb_normalize_group_request(struct ext4_allocation_context *ac)
{
struct super_block *sb = ac->ac_sb;
- struct ext4_locality_group *lg = ac->ac_lg;
- BUG_ON(lg == NULL);
if (EXT4_SB(sb)->s_stripe)
ac->ac_g_ex.fe_len = EXT4_SB(sb)->s_stripe;
else
On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <[email protected]> wrote:
> From: Jing Zhang <[email protected]>
>
> Date: Wed Mar 25 22:55:04 2010
>
> Checking bug seems not at right place, and the function itself should
> be inlined.
>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: Dave Kleikamp <[email protected]>
> Cc: "Aneesh Kumar K. V" <[email protected]>
> Signed-off-by: Jing Zhang <[email protected]>
>
> ---
>
> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
> +++ ext4_mm_leak/mballoc-10.c 2010-03-25 22:44:00.000000000 +0800
> @@ -2786,9 +2786,7 @@ out_err:
> static void ext4_mb_normalize_group_request(struct ext4_allocation_context *ac)
> {
> struct super_block *sb = ac->ac_sb;
> - struct ext4_locality_group *lg = ac->ac_lg;
>
> - BUG_ON(lg == NULL);
> if (EXT4_SB(sb)->s_stripe)
> ac->ac_g_ex.fe_len = EXT4_SB(sb)->s_stripe;
> else
That BUG_ON is to ensure that the allocation context is actually having
a locality group which is needed for group allocation request.
-aneesh
2010/3/26, Aneesh Kumar K. V <[email protected]>:
> On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <[email protected]> wrote:
>> From: Jing Zhang <[email protected]>
>>
>> Date: Wed Mar 25 22:55:04 2010
>>
>> Checking bug seems not at right place, and the function itself should
>> be inlined.
>>
>> Cc: Theodore Ts'o <[email protected]>
>> Cc: Andreas Dilger <[email protected]>
>> Cc: Dave Kleikamp <[email protected]>
>> Cc: "Aneesh Kumar K. V" <[email protected]>
>> Signed-off-by: Jing Zhang <[email protected]>
>>
>> ---
>>
>> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
>> +++ ext4_mm_leak/mballoc-10.c 2010-03-25 22:44:00.000000000 +0800
>> @@ -2786,9 +2786,7 @@ out_err:
>> static void ext4_mb_normalize_group_request(struct
>> ext4_allocation_context *ac)
>> {
>> struct super_block *sb = ac->ac_sb;
>> - struct ext4_locality_group *lg = ac->ac_lg;
>>
>> - BUG_ON(lg == NULL);
>> if (EXT4_SB(sb)->s_stripe)
>> ac->ac_g_ex.fe_len = EXT4_SB(sb)->s_stripe;
>> else
>
> That BUG_ON is to ensure that the allocation context is actually having
> a locality group which is needed for group allocation request.
>
> -aneesh
>
Please check
1, the 3 lines at the end of ext4_mb_group_or_file()
2, the function name of this patch
- zj
On Fri, 26 Mar 2010 21:44:33 +0800, jing zhang <[email protected]> wrote:
> 2010/3/26, Aneesh Kumar K. V <[email protected]>:
> > On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <[email protected]> wrote:
> >> From: Jing Zhang <[email protected]>
> >>
> >> Date: Wed Mar 25 22:55:04 2010
> >>
> >> Checking bug seems not at right place, and the function itself should
> >> be inlined.
> >>
> >> Cc: Theodore Ts'o <[email protected]>
> >> Cc: Andreas Dilger <[email protected]>
> >> Cc: Dave Kleikamp <[email protected]>
> >> Cc: "Aneesh Kumar K. V" <[email protected]>
> >> Signed-off-by: Jing Zhang <[email protected]>
> >>
> >> ---
> >>
> >> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
> >> +++ ext4_mm_leak/mballoc-10.c 2010-03-25 22:44:00.000000000 +0800
> >> @@ -2786,9 +2786,7 @@ out_err:
> >> static void ext4_mb_normalize_group_request(struct
> >> ext4_allocation_context *ac)
> >> {
> >> struct super_block *sb = ac->ac_sb;
> >> - struct ext4_locality_group *lg = ac->ac_lg;
> >>
> >> - BUG_ON(lg == NULL);
> >> if (EXT4_SB(sb)->s_stripe)
> >> ac->ac_g_ex.fe_len = EXT4_SB(sb)->s_stripe;
> >> else
> >
> > That BUG_ON is to ensure that the allocation context is actually having
> > a locality group which is needed for group allocation request.
> >
> > -aneesh
> >
>
> Please check
> 1, the 3 lines at the end of ext4_mb_group_or_file()
> 2, the function name of this patch
>
What i wanted to mention was the BUG_ON is there to ensure that we don't
call ext4_mb_normalize_group_request on non group enabled allocation
context by programming mistake. Doing that would cause ac_g_ex.fe_len
to change. So that BUG_ON is there to capture a programming error.
-aneesh
2010/3/26, Aneesh Kumar K. V <[email protected]>:
> On Fri, 26 Mar 2010 21:44:33 +0800, jing zhang <[email protected]> wrote:
>> 2010/3/26, Aneesh Kumar K. V <[email protected]>:
>> > On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <[email protected]>
>> > wrote:
>> >> From: Jing Zhang <[email protected]>
>> >>
>> >> Date: Wed Mar 25 22:55:04 2010
>> >>
>> >> Checking bug seems not at right place, and the function itself should
>> >> be inlined.
>> >>
>> >> Cc: Theodore Ts'o <[email protected]>
>> >> Cc: Andreas Dilger <[email protected]>
>> >> Cc: Dave Kleikamp <[email protected]>
>> >> Cc: "Aneesh Kumar K. V" <[email protected]>
>> >> Signed-off-by: Jing Zhang <[email protected]>
>> >>
>> >> ---
>> >>
>> >> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
>> >> +++ ext4_mm_leak/mballoc-10.c 2010-03-25 22:44:00.000000000 +0800
>> >> @@ -2786,9 +2786,7 @@ out_err:
>> >> static void ext4_mb_normalize_group_request(struct
>> >> ext4_allocation_context *ac)
>> >> {
>> >> struct super_block *sb = ac->ac_sb;
>> >> - struct ext4_locality_group *lg = ac->ac_lg;
>> >>
>> >> - BUG_ON(lg == NULL);
>> >> if (EXT4_SB(sb)->s_stripe)
>> >> ac->ac_g_ex.fe_len = EXT4_SB(sb)->s_stripe;
>> >> else
>> >
>> > That BUG_ON is to ensure that the allocation context is actually having
>> > a locality group which is needed for group allocation request.
>> >
>> > -aneesh
>> >
>>
>> Please check
>> 1, the 3 lines at the end of ext4_mb_group_or_file()
>> 2, the function name of this patch
>>
>
> What i wanted to mention was the BUG_ON is there to ensure that we don't
> call ext4_mb_normalize_group_request on non group enabled allocation
> context by programming mistake. Doing that would cause ac_g_ex.fe_len
> to change. So that BUG_ON is there to capture a programming error.
>
> -aneesh
>
Thank you, Aneesh, for good explanation in patience.
Again, good weekend.
Still in work?
- zj
This patch has been marked as rejected in patchwork.
-- Ted
On Fri, Mar 26, 2010 at 10:37:49PM +0800, jing zhang wrote:
> 2010/3/26, Aneesh Kumar K. V <[email protected]>:
> > On Fri, 26 Mar 2010 21:44:33 +0800, jing zhang <[email protected]> wrote:
> >> 2010/3/26, Aneesh Kumar K. V <[email protected]>:
> >> > On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <[email protected]>
> >> > wrote:
> >> >> From: Jing Zhang <[email protected]>
> >> >>
> >> >> Date: Wed Mar 25 22:55:04 2010
> >> >>
> >> >> Checking bug seems not at right place, and the function itself should
> >> >> be inlined.
> >> >>
> >> >> Cc: Theodore Ts'o <[email protected]>
> >> >> Cc: Andreas Dilger <[email protected]>
> >> >> Cc: Dave Kleikamp <[email protected]>
> >> >> Cc: "Aneesh Kumar K. V" <[email protected]>
> >> >> Signed-off-by: Jing Zhang <[email protected]>
> >> >>
> >> >> ---
> >> >>
> >> >> --- linux-2.6.32/fs/ext4/mballoc.c 2009-12-03 11:51:22.000000000 +0800
> >> >> +++ ext4_mm_leak/mballoc-10.c 2010-03-25 22:44:00.000000000 +0800
> >> >> @@ -2786,9 +2786,7 @@ out_err:
> >> >> static void ext4_mb_normalize_group_request(struct
> >> >> ext4_allocation_context *ac)
> >> >> {
> >> >> struct super_block *sb = ac->ac_sb;
> >> >> - struct ext4_locality_group *lg = ac->ac_lg;
> >> >>
> >> >> - BUG_ON(lg == NULL);
> >> >> if (EXT4_SB(sb)->s_stripe)
> >> >> ac->ac_g_ex.fe_len = EXT4_SB(sb)->s_stripe;
> >> >> else
> >> >
> >> > That BUG_ON is to ensure that the allocation context is actually having
> >> > a locality group which is needed for group allocation request.
> >> >
> >> > -aneesh
> >> >
> >>
> >> Please check
> >> 1, the 3 lines at the end of ext4_mb_group_or_file()
> >> 2, the function name of this patch
> >>
> >
> > What i wanted to mention was the BUG_ON is there to ensure that we don't
> > call ext4_mb_normalize_group_request on non group enabled allocation
> > context by programming mistake. Doing that would cause ac_g_ex.fe_len
> > to change. So that BUG_ON is there to capture a programming error.
> >
> > -aneesh
> >
>
> Thank you, Aneesh, for good explanation in patience.
>
> Again, good weekend.
>
> Still in work?
>
> - zj
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
2010/4/5, [email protected] <[email protected]>:
> This patch has been marked as rejected in patchwork.
>
> -- Ted
I accept what you decided, Mr. Theodore Ts'o.
And I want to learn a little about the score on the patchwork web
site, so please guide me to it if you like.
Thank you all, great maintainers and developers of GNU Linux, for
reviewing the patch I delivered.
- zj
On Tue, Apr 06, 2010 at 10:05:33PM +0800, jing zhang wrote:
>
> And I want to learn a little about the score on the patchwork web
> site, so please guide me to it if you like.
http://patchwork.ozlabs.org/project/linux-ext4/list/
Yes, there's a huge backlog. In general the most recent patches are
the ones that I worry about. The oldest ones are there more for
archeological digging more than anything else. Patch submitters
generally will ping me to remind me of a specific patch if there
hasn't been some kind of response in a few weeks...
- Ted