Sano-san,
(2011/02/14 22:57), Yoshinori Sano wrote:
> 2011年2月14日8:57 Tsutomu Itoh <t-itoh@xxxxxxxxxxxxxx>:
>> (2011/02/12 20:17), Yoshinori Sano wrote:
>>> To make Btrfs code more robust, several return value checks where memory
>>> allocation can fail are introduced. I use BUG_ON where I don't know how
>>> to handle the error properly, which increases the number of using the
>>> notorious BUG_ON, though.
>>>
>>> Signed-off-by: Yoshinori Sano <yoshinori.sano@xxxxxxxxx>
>>> ---
>>> fs/btrfs/compression.c | 6 ++++++
>>> fs/btrfs/extent-tree.c | 2 ++
>>> fs/btrfs/file.c | 8 ++++++--
>>> fs/btrfs/inode.c | 5 +++++
>>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
>>> index 4d2110e..f596554 100644
>>> --- a/fs/btrfs/compression.c
>>> +++ b/fs/btrfs/compression.c
>>> @@ -340,6 +340,8 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 start,
>>>
>>> WARN_ON(start & ((u64)PAGE_CACHE_SIZE - 1));
>>> cb = kmalloc(compressed_bio_size(root, compressed_len), GFP_NOFS);
>>> + if (!cb)
>>> + return -ENOMEM;
>>> atomic_set(&cb->pending_bios, 0);
>>> cb->errors = 0;
>>> cb->inode = inode;
>>> @@ -354,6 +356,10 @@ int btrfs_submit_compressed_write(struct inode *inode, u64 start,
>>> bdev = BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev;
>>>
>>> bio = compressed_bio_alloc(bdev, first_byte, GFP_NOFS);
>>> + if (!bio) {
>>> + kfree(cb);
>>> + return -ENOMEM;
>>> + }
>>> bio->bi_private = cb;
>>> bio->bi_end_io = end_compressed_bio_write;
>>> atomic_inc(&cb->pending_bios);
>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>> index 565e22d..aed16f4 100644
>>> --- a/fs/btrfs/extent-tree.c
>>> +++ b/fs/btrfs/extent-tree.c
>>> @@ -6931,6 +6931,8 @@ static noinline int get_new_locations(struct inode *reloc_inode,
>>> struct disk_extent *old = exts;
>>> max *= 2;
>>> exts = kzalloc(sizeof(*exts) * max, GFP_NOFS);
>>> + if (!exts)
>>> + goto out;
'ret = -ENOMEM' is necessary before 'goto out'.
>>> memcpy(exts, old, sizeof(*exts) * nr);
>>> if (old != *extents)
>>> kfree(old);
>>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
>>> index b0ff34b..4895ad2 100644
>>> --- a/fs/btrfs/file.c
>>> +++ b/fs/btrfs/file.c
>>> @@ -181,10 +181,14 @@ int btrfs_drop_extent_cache(struct inode *inode, u64 start, u64 end,
>>> testend = 0;
>>> }
>>> while (1) {
>>> - if (!split)
>>> + if (!split) {
>>> split = alloc_extent_map(GFP_NOFS);
>>> - if (!split2)
>>> + BUG_ON(!split || IS_ERR(split));
>>
>> alloc_extent_map() returns only the address or NULL.
>> Therefore, I think that check by IS_ERR() is unnecessary.
>>
>> Regards,
>> Itoh
>
> Exactly. IS_ERR is not required.
> I should read the alloc_extent_map' s implementation more deeply.
> Thank you.
Could you please merge my patch(http://marc.info/?l=linux-btrfs&m=129764438122741&w=2)
with your patch, and post it again?
Thanks,
Itoh
>
>>
>>> + }
>>> + if (!split2) {
>>> split2 = alloc_extent_map(GFP_NOFS);
>>> + BUG_ON(!split2 || IS_ERR(split2));
>>> + }
>>>
>>> write_lock(&em_tree->lock);
>>> em = lookup_extent_mapping(em_tree, start, len);
>>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>>> index c9bc0af..40bbe00 100644
>>> --- a/fs/btrfs/inode.c
>>> +++ b/fs/btrfs/inode.c
>>> @@ -287,6 +287,7 @@ static noinline int add_async_extent(struct async_cow *cow,
>>> struct async_extent *async_extent;
>>>
>>> async_extent = kmalloc(sizeof(*async_extent), GFP_NOFS);
>>> + BUG_ON(!async_extent);
>>> async_extent->start = start;
>>> async_extent->ram_size = ram_size;
>>> async_extent->compressed_size = compressed_size;
>>> @@ -384,6 +385,7 @@ again:
>>> (BTRFS_I(inode)->force_compress))) {
>>> WARN_ON(pages);
>>> pages = kzalloc(sizeof(struct page *) * nr_pages, GFP_NOFS);
>>> + BUG_ON(!pages);
>>>
>>> if (BTRFS_I(inode)->force_compress)
>>> compress_type = BTRFS_I(inode)->force_compress;
>>> @@ -644,6 +646,7 @@ retry:
>>> async_extent->ram_size - 1, 0);
>>>
>>> em = alloc_extent_map(GFP_NOFS);
>>> + BUG_ON(!em || IS_ERR(em));
>>> em->start = async_extent->start;
>>> em->len = async_extent->ram_size;
>>> em->orig_start = em->start;
>>> @@ -820,6 +823,7 @@ static noinline int cow_file_range(struct inode *inode,
>>> BUG_ON(ret);
>>>
>>> em = alloc_extent_map(GFP_NOFS);
>>> + BUG_ON(!em || IS_ERR(em));
>>> em->start = start;
>>> em->orig_start = em->start;
>>> ram_size = ins.offset;
>>> @@ -1169,6 +1173,7 @@ out_check:
>>> struct extent_map_tree *em_tree;
>>> em_tree = &BTRFS_I(inode)->extent_tree;
>>> em = alloc_extent_map(GFP_NOFS);
>>> + BUG_ON(!em || IS_ERR(em));
>>> em->start = cur_offset;
>>> em->orig_start = em->start;
>>> em->len = num_bytes;
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
