Re: [PATCH] ext4: release page cache in ext4_mb_load_buddy error path

[Yang Rui Rui <ruirui.r.yang@tieto.com>]

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@tieto.com>  wrote:
>> Add missing page_cache_release in the error path of ext4_mb_load_buddy
>>
>> Signed-off-by: Yang Ruirui<ruirui.r.yang@tieto.com>
>> ---
>> �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@vger.kernel.org
>> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html