| Message ID | 1404382933-26672-4-git-send-email-miaox@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 <miaox@cn.fujitsu.com> Reviewed-by: Satoru Takeuchi <takeuchi_satoru@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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 <miaox@cn.fujitsu.com> --- fs/btrfs/inode.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)