(2011/04/19 20:12), David Sterba wrote:

Thanks for your review.

> Hi,
> 
> there are more unchecked kmallocs, in inode.c:btrfs_add_delayed_iput()
> 
> 2030        delayed = kmalloc(sizeof(*delayed), GFP_NOFS | __GFP_NOFAIL);

I think that it doesn't fail ordinary when __GFP_NOFAIL is specified...

> 
> and in extent-tree.c:relocate_one_extent()
> 
> 7992        new_extents = kmalloc(sizeof(*new_extents),
> 7993                                                 GFP_NOFS);
> 
> the value is checked later, new_extents is passed to get_new_locations
> and there it's checked, but no other callers pass potential NULL and the
> check fits here and can be dropped from get_new_locations; there's a
> little chance that get_new_locations will be able to succesfully
> allocate the same data a jiffy later.

Yes, therefore I did not check 'new_extents'.

Thanks,
Tsutomu


> 
> IMO the check can be safely added here and get_new_locations cleaned up
> later.
> 
> Feel free to fold the changes into your patch.
> 
> I did not find any more unchecked allocatinos.
> 
> 
> dave
> 
> 
> On Tue, Apr 19, 2011 at 02:27:08PM +0900, Tsutomu Itoh wrote:
>> The check on the return value of kmalloc() in inode.c is added.
>>
>> Signed-off-by: Tsutomu Itoh <[email protected]>
>> ---
>>  fs/btrfs/inode.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index a4157cf..c718d27 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -953,6 +953,7 @@ static int cow_file_range_async(struct inode *inode, 
>> struct page *locked_page,
>>                       1, 0, NULL, GFP_NOFS);
>>      while (start < end) {
>>              async_cow = kmalloc(sizeof(*async_cow), GFP_NOFS);
>> +            BUG_ON(!async_cow);
>>              async_cow->inode = inode;
>>              async_cow->root = root;
>>              async_cow->locked_page = locked_page;
>> @@ -5001,6 +5002,8 @@ static noinline int uncompress_inline(struct 
>> btrfs_path *path,
>>      inline_size = btrfs_file_extent_inline_item_len(leaf,
>>                                      btrfs_item_nr(leaf, path->slots[0]));
>>      tmp = kmalloc(inline_size, GFP_NOFS);
>> +    if (!tmp)
>> +            return -ENOMEM;
>>      ptr = btrfs_file_extent_inline_start(item);
>>  
>>      read_extent_buffer(leaf, tmp, ptr, inline_size);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to [email protected]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to