On 04/14/2011 03:46 PM, Amir Goldstein wrote:
On Thu, Apr 14, 2011 at 10:29 AM, Yang Rui Rui<ruirui.r.yang@xxxxxxxxx> wrote:
Hi,
Thanks for your comment. BTW, why linux-kernel mainling list is removed from
cc?
Is it ext4 list prefer or something else?
ext4 list is the place to post fixes for ext4.
LKML need not be bothered with these specific patches.
On 04/14/2011 03:01 PM, Amir Goldstein wrote:
Hi Yang ,
The patch looks correct, but in my opinion a nicer fix would be to set
e4b->bd_bitmap_page = page;
or
e4b->bd_buddy_page = page;
right after assigning a new value to the temp variable 'page'.
and keeping the cleanup code in the error path as it is.
It's a matter of taste and code readability.
I agree with you for common case, but this function is not so readable
already.
Two many if conditions and indent. I would prefer just fix this problem as
this patch.
As I said it's a matter of taste so I'm not going to argue.
One argument in favor of your patch is that it adds fewer lines.
Then rewrite the function as more small size functions for example the get
page part.
If you are going to audit code or rework functions in mballoc.c,
I suggest that you first take look at my patches to remove alloc_semp.
Those already involve some rework in mballoc.c.
See online version at:
https://github.com/amir73il/ext4-snapshots/tree/alloc_semp
Thanks, will take a look.
Amir.
On Thu, Apr 14, 2011 at 9:44 AM, Yang Ruirui<ruirui.r.yang@xxxxxxxxx>
wrote:
Add missing page_cache_release in the error path of ext4_mb_load_buddy
Signed-off-by: Yang Ruirui<ruirui.r.yang@xxxxxxxxx>
--
Thanks
Yang Ruirui
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
