Hi,
Thanks for your comment. BTW, why linux-kernel mainling list is removed from cc?
Is it ext4 list prefer or something else?
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.
Then rewrite the function as more small size functions for example the get page part.
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>
---
ïfs/ext4/mballoc.c | ï ï2 ++
ï1 file changed, 2 insertions(+)
--- linux-2.6.orig/fs/ext4/mballoc.c ï ï2011-04-14 14:04:48.000000000 +0800
+++ linux-2.6/fs/ext4/mballoc.c 2011-04-14 14:33:28.702958245 +0800
@@ -1273,6 +1273,8 @@ repeat_load_buddy:
ï ï ï ïreturn 0;
ïerr:
+ ï ï ï if (page)
+ ï ï ï ï ï ï ï page_cache_release(page);
ï ï ï ïif (e4b->bd_bitmap_page)
ï ï ï ï ï ï ï ïpage_cache_release(e4b->bd_bitmap_page);
ï ï ï ïif (e4b->bd_buddy_page)
--
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
--
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
