diff mbox

ext4: remove unnecessary operation in ext4_mb_normalize_group_request()

Message ID ac8f92701003250754o682aa32dw9bb51f36f0fa433b@mail.gmail.com
State Rejected, archived
Headers show

Commit Message

jing zhang March 25, 2010, 2:54 p.m. UTC 
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>

---

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

Comments

Aneesh Kumar K.V March 26, 2010, 8:08 a.m. UTC | #1 
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
--
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
jing zhang March 26, 2010, 1:44 p.m. UTC | #2 
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
--
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
Aneesh Kumar K.V March 26, 2010, 2:27 p.m. UTC | #3 
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
--
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
jing zhang March 26, 2010, 2:37 p.m. UTC | #4 
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
Theodore Ts'o April 5, 2010, 12:48 p.m. UTC | #5 
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
--
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
jing zhang April 6, 2010, 2:05 p.m. UTC | #6 
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
--
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
Theodore Ts'o April 6, 2010, 5:01 p.m. UTC | #7 
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
--
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
diff mbox

Patch

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