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
--
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

Reply via email to