Hi Miao,
(2014/07/03 19:22), Miao Xie wrote:
> The caller of btrfs_submit_direct_hook() will put the original dio bio
> when btrfs_submit_direct_hook() return a error number, so we needn't
> put the original bio in btrfs_submit_direct_hook().
>
> Signed-off-by: Miao Xie <mi...@cn.fujitsu.com>
Reviewed-by: Satoru Takeuchi <takeuchi_sat...@jp.fujitsu.com>
Here is the review result, CMIIW.
call trace:
btrfs_submit_direct
-> btrfs_submit_direct_hook
fs/btrfs/inode.c:
===============================================================================
static int btrfs_submit_direct_hook(int rw, struct btrfs_dio_private *dip,
int skip_sum)
{
...
struct bio *orig_bio = dip->orig_bio;
...
map_length = orig_bio->bi_iter.bi_size;
ret = btrfs_map_block(root->fs_info, rw, start_sector << 9,
&map_length, NULL, 0);
if (ret) {
bio_put(orig_bio); # (1)
return -EIO;
}
...
}
...
static void btrfs_submit_direct(int rw, struct bio *dio_bio,
struct inode *inode, loff_t file_offset)
{
dip = kmalloc(sizeof(*dip) + sum_len, GFP_NOFS);
if (!dip) {
ret = -ENOMEM;
goto free_io_bio; # (2)
}
...
ret = btrfs_submit_direct_hook(rw, dip, skip_sum);
if (!ret)
return;
free_io_bio:
bio_put(io_bio); # (3)
...
}
===============================================================================
If btrfs_map_block() fails in btrfs_submit_direct_hook(), it put
orig_bio at (1) and return -EIO. Then caller, btrfs_submit_direct()
free the same bio at (3). Since (3) is also used for other error
handling (2), I consider your way, removing (1), is better.
Thanks,
Satoru
> ---
> fs/btrfs/inode.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a616fa4..15902eb 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7325,10 +7325,8 @@ static int btrfs_submit_direct_hook(int rw, struct
> btrfs_dio_private *dip,
> map_length = orig_bio->bi_iter.bi_size;
> ret = btrfs_map_block(root->fs_info, rw, start_sector << 9,
> &map_length, NULL, 0);
> - if (ret) {
> - bio_put(orig_bio);
> + if (ret)
> return -EIO;
> - }
>
> if (map_length >= orig_bio->bi_iter.bi_size) {
> bio = orig_bio;
> @@ -7345,6 +7343,7 @@ static int btrfs_submit_direct_hook(int rw, struct
> btrfs_dio_private *dip,
> bio = btrfs_dio_bio_alloc(orig_bio->bi_bdev, start_sector, GFP_NOFS);
> if (!bio)
> return -ENOMEM;
> +
> bio->bi_private = dip;
> bio->bi_end_io = btrfs_end_dio_bio;
> atomic_inc(&dip->pending_bios);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
