[PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[jing zhang]

From: Jing Zhang <zj.barak@gmail.com>

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 <tytso@mit.edu>
Cc: Andreas Dilger <adilger@sun.com>
Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Jing Zhang <zj.barak@gmail.com>

---

--- 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

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[Aneesh Kumar K. V]

On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <zj.barak@gmail.com> wrote:
> From: Jing Zhang <zj.barak@gmail.com>
> 
> 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 <tytso@mit.edu>
> Cc: Andreas Dilger <adilger@sun.com>
> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Cc: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Jing Zhang <zj.barak@gmail.com>
> 
> ---
> 
> --- 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

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[jing zhang]

2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>:
> On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <zj.barak@gmail.com> wrote:
>> From: Jing Zhang <zj.barak@gmail.com>
>>
>> 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 <tytso@mit.edu>
>> Cc: Andreas Dilger <adilger@sun.com>
>> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>> Cc: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
>> Signed-off-by: Jing Zhang <zj.barak@gmail.com>
>>
>> ---
>>
>> --- 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

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[Aneesh Kumar K. V]

On Fri, 26 Mar 2010 21:44:33 +0800, jing zhang <zj.barak@gmail.com> wrote:
> 2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>:
> > On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <zj.barak@gmail.com> wrote:
> >> From: Jing Zhang <zj.barak@gmail.com>
> >>
> >> 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 <tytso@mit.edu>
> >> Cc: Andreas Dilger <adilger@sun.com>
> >> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> >> Cc: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
> >> Signed-off-by: Jing Zhang <zj.barak@gmail.com>
> >>
> >> ---
> >>
> >> --- 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

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[jing zhang]

2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>:
> On Fri, 26 Mar 2010 21:44:33 +0800, jing zhang <zj.barak@gmail.com> wrote:
>> 2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>:
>> > On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <zj.barak@gmail.com>
>> > wrote:
>> >> From: Jing Zhang <zj.barak@gmail.com>
>> >>
>> >> 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 <tytso@mit.edu>
>> >> Cc: Andreas Dilger <adilger@sun.com>
>> >> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>> >> Cc: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
>> >> Signed-off-by: Jing Zhang <zj.barak@gmail.com>
>> >>
>> >> ---
>> >>
>> >> --- 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

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[tytso]

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 <aneesh.kumar@linux.vnet.ibm.com>:
> > On Fri, 26 Mar 2010 21:44:33 +0800, jing zhang <zj.barak@gmail.com> wrote:
> >> 2010/3/26, Aneesh Kumar K. V <aneesh.kumar@linux.vnet.ibm.com>:
> >> > On Thu, 25 Mar 2010 22:54:21 +0800, jing zhang <zj.barak@gmail.com>
> >> > wrote:
> >> >> From: Jing Zhang <zj.barak@gmail.com>
> >> >>
> >> >> 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 <tytso@mit.edu>
> >> >> Cc: Andreas Dilger <adilger@sun.com>
> >> >> Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> >> >> Cc: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
> >> >> Signed-off-by: Jing Zhang <zj.barak@gmail.com>
> >> >>
> >> >> ---
> >> >>
> >> >> --- 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[jing zhang]

2010/4/5, tytso@mit.edu <tytso@mit.edu>:
> 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

Re: [PATCH] ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

[tytso]

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