Re: [PATCH 2/6] Btrfs: use bio_clone_bioset_partial to simplify DIO submit

[Liu Bo <bo.li.liu@oracle.com>]

On Tue, May 16, 2017 at 07:37:37AM -0700, Christoph Hellwig wrote:
> >  }
> >  
> > +struct bio *btrfs_bio_clone_partial(struct bio *orig, gfp_t gfp_mask, int offset, int size)
> > +{
> > +	struct bio *bio;
> > +
> > +	bio = bio_clone_fast(orig, gfp_mask, btrfs_bioset);
> > +	if (bio) {
> 
> bio_clone_fast will never fail when backed by a bioset, which this
> one always is.  Also you always pass GFP_NPFS as the gfp_mask argument,
> it might make sense to hardcode that here.
> 

I see.

> > +		struct btrfs_io_bio *btrfs_bio = btrfs_io_bio(bio);
> > +		btrfs_bio->csum = NULL;
> > +		btrfs_bio->csum_allocated = NULL;
> > +		btrfs_bio->end_io = NULL;
> > +
> > +		bio_trim(bio, (offset >> 9), (size >> 9));
> 
> No need for the inner braces here.
> 
> Last but not least do you even need this as a separate helper?
> 

Not necessary indeed, but I need to access %btrfs_bioset which is
'static' defined in extent_io.c

> > +struct bio *btrfs_bio_clone_partial(struct bio *orig, gfp_t gfp_mask, int offset, int size);
> 
> Over long line, please trim to 80 characters

OK, fixed.

Thanks for the comments.

Thanks,

-liubo