| Message ID | 20210128071133.60335-3-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: introduce bio_new() | expand |
On 2021/01/28 16:12, Chaitanya Kulkarni wrote: > Introduce bio_new() helper and use it in blk-lib.c to allocate and > initialize various non-optional or semi-optional members of the bio > along with bio allocation done with bio_alloc(). Here we also calmp the > max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc(). > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > --- > block/blk-lib.c | 6 +----- > include/linux/bio.h | 25 +++++++++++++++++++++++++ > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index fb486a0bdb58..ec29415f00dd 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -14,17 +14,13 @@ struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, > sector_t sect, unsigned op, unsigned opf, > unsigned int nr_pages, gfp_t gfp) > { > - struct bio *new = bio_alloc(gfp, nr_pages); > + struct bio *new = bio_new(bdev, sect, op, opf, gfp, nr_pages); > > if (bio) { > bio_chain(bio, new); > submit_bio(bio); > } > > - new->bi_iter.bi_sector = sect; > - bio_set_dev(new, bdev); > - bio_set_op_attrs(new, op, opf); > - > return new; > } > > diff --git a/include/linux/bio.h b/include/linux/bio.h > index c74857cf1252..2a09ba100546 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -826,5 +826,30 @@ static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb) > if (!is_sync_kiocb(kiocb)) > bio->bi_opf |= REQ_NOWAIT; > } > +/** > + * bio_new - allcate and initialize new bio > + * @bdev: blockdev to issue discard for > + * @sector: start sector > + * @op: REQ_OP_XXX from enum req_opf > + * @op_flags: REQ_XXX from enum req_flag_bits > + * @max_bvecs: maximum bvec to be allocated for this bio > + * @gfp_mask: memory allocation flags (for bio_alloc) > + * > + * Description: > + * Allocates, initializes common members, and returns a new bio. > + */ > +static inline struct bio *bio_new(struct block_device *bdev, sector_t sector, > + unsigned int op, unsigned int op_flags, > + unsigned int max_bvecs, gfp_t gfp_mask) > +{ > + unsigned nr_bvec = clamp_t(unsigned int, max_bvecs, 0, BIO_MAX_PAGES); > + struct bio *bio = bio_alloc(gfp_mask, nr_bvec); I think that depending on the gfp_mask passed, bio can be NULL. So this should be checked. > + > + bio_set_dev(bio, bdev); > + bio->bi_iter.bi_sector = sector; > + bio_set_op_attrs(bio, op, op_flags); This function is obsolete. Open code this. > + > + return bio; > +} > > #endif /* __LINUX_BIO_H */ >
On 2021/01/28 16:21, Damien Le Moal wrote: > On 2021/01/28 16:12, Chaitanya Kulkarni wrote: >> Introduce bio_new() helper and use it in blk-lib.c to allocate and >> initialize various non-optional or semi-optional members of the bio >> along with bio allocation done with bio_alloc(). Here we also calmp the >> max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc(). >> >> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> >> --- >> block/blk-lib.c | 6 +----- >> include/linux/bio.h | 25 +++++++++++++++++++++++++ >> 2 files changed, 26 insertions(+), 5 deletions(-) >> >> diff --git a/block/blk-lib.c b/block/blk-lib.c >> index fb486a0bdb58..ec29415f00dd 100644 >> --- a/block/blk-lib.c >> +++ b/block/blk-lib.c >> @@ -14,17 +14,13 @@ struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, >> sector_t sect, unsigned op, unsigned opf, >> unsigned int nr_pages, gfp_t gfp) >> { >> - struct bio *new = bio_alloc(gfp, nr_pages); >> + struct bio *new = bio_new(bdev, sect, op, opf, gfp, nr_pages); >> >> if (bio) { >> bio_chain(bio, new); >> submit_bio(bio); >> } >> >> - new->bi_iter.bi_sector = sect; >> - bio_set_dev(new, bdev); >> - bio_set_op_attrs(new, op, opf); >> - >> return new; >> } >> >> diff --git a/include/linux/bio.h b/include/linux/bio.h >> index c74857cf1252..2a09ba100546 100644 >> --- a/include/linux/bio.h >> +++ b/include/linux/bio.h >> @@ -826,5 +826,30 @@ static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb) >> if (!is_sync_kiocb(kiocb)) >> bio->bi_opf |= REQ_NOWAIT; >> } >> +/** >> + * bio_new - allcate and initialize new bio >> + * @bdev: blockdev to issue discard for >> + * @sector: start sector >> + * @op: REQ_OP_XXX from enum req_opf >> + * @op_flags: REQ_XXX from enum req_flag_bits >> + * @max_bvecs: maximum bvec to be allocated for this bio >> + * @gfp_mask: memory allocation flags (for bio_alloc) >> + * >> + * Description: >> + * Allocates, initializes common members, and returns a new bio. >> + */ >> +static inline struct bio *bio_new(struct block_device *bdev, sector_t sector, >> + unsigned int op, unsigned int op_flags, >> + unsigned int max_bvecs, gfp_t gfp_mask) >> +{ >> + unsigned nr_bvec = clamp_t(unsigned int, max_bvecs, 0, BIO_MAX_PAGES); >> + struct bio *bio = bio_alloc(gfp_mask, nr_bvec); > > I think that depending on the gfp_mask passed, bio can be NULL. So this should > be checked. > >> + >> + bio_set_dev(bio, bdev); >> + bio->bi_iter.bi_sector = sector; >> + bio_set_op_attrs(bio, op, op_flags); > > This function is obsolete. Open code this. And that also mean that you could remove one argument to bio_new(): combine op and op_flags into "unsigned int opf" > >> + >> + return bio; >> +} >> >> #endif /* __LINUX_BIO_H */ >> > >
On 1/27/21 11:21 PM, Damien Le Moal wrote:
On 2021/01/28 16:12, Chaitanya Kulkarni wrote:
Introduce bio_new() helper and use it in blk-lib.c to allocate and
initialize various non-optional or semi-optional members of the bio
along with bio allocation done with bio_alloc(). Here we also calmp the
max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc().
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com><mailto:chaitanya.kulkarni@wdc.com>
---
block/blk-lib.c | 6 +-----
include/linux/bio.h | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index fb486a0bdb58..ec29415f00dd 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -14,17 +14,13 @@ struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev,
sector_t sect, unsigned op, unsigned opf,
unsigned int nr_pages, gfp_t gfp)
{
- struct bio *new = bio_alloc(gfp, nr_pages);
+ struct bio *new = bio_new(bdev, sect, op, opf, gfp, nr_pages);
if (bio) {
bio_chain(bio, new);
submit_bio(bio);
}
- new->bi_iter.bi_sector = sect;
- bio_set_dev(new, bdev);
- bio_set_op_attrs(new, op, opf);
-
return new;
}
diff --git a/include/linux/bio.h b/include/linux/bio.h
index c74857cf1252..2a09ba100546 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -826,5 +826,30 @@ static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb)
if (!is_sync_kiocb(kiocb))
bio->bi_opf |= REQ_NOWAIT;
}
+/**
+ * bio_new - allcate and initialize new bio
+ * @bdev: blockdev to issue discard for
+ * @sector: start sector
+ * @op: REQ_OP_XXX from enum req_opf
+ * @op_flags: REQ_XXX from enum req_flag_bits
+ * @max_bvecs: maximum bvec to be allocated for this bio
+ * @gfp_mask: memory allocation flags (for bio_alloc)
+ *
+ * Description:
+ * Allocates, initializes common members, and returns a new bio.
+ */
+static inline struct bio *bio_new(struct block_device *bdev, sector_t sector,
+ unsigned int op, unsigned int op_flags,
+ unsigned int max_bvecs, gfp_t gfp_mask)
+{
+ unsigned nr_bvec = clamp_t(unsigned int, max_bvecs, 0, BIO_MAX_PAGES);
+ struct bio *bio = bio_alloc(gfp_mask, nr_bvec);
I think that depending on the gfp_mask passed, bio can be NULL. So this should
be checked.
true, I'll add that check.
+
+ bio_set_dev(bio, bdev);
+ bio->bi_iter.bi_sector = sector;
+ bio_set_op_attrs(bio, op, op_flags);
This function is obsolete. Open code this.
true, will do.
+
+ return bio;
+}
#endif /* __LINUX_BIO_H */
Thanks for the comments Damien.
On 1/27/21 11:27 PM, Damien Le Moal wrote: + + bio_set_dev(bio, bdev); + bio->bi_iter.bi_sector = sector; + bio_set_op_attrs(bio, op, op_flags); This function is obsolete. Open code this. And that also mean that you could remove one argument to bio_new(): combine op and op_flags into "unsigned int opf" I did that initially but kept it separate for RFC, that is much easier than having an extra arg, will change it in V1.
FYI your email is completely unreadable to those not using html. I can't tell what you wrote and what Damien wrote. On Thu, Jan 28, 2021 at 08:33:10AM +0000, Chaitanya Kulkarni wrote: > On 1/27/21 11:21 PM, Damien Le Moal wrote: > > On 2021/01/28 16:12, Chaitanya Kulkarni wrote: > > > Introduce bio_new() helper and use it in blk-lib.c to allocate and > initialize various non-optional or semi-optional members of the bio > along with bio allocation done with bio_alloc(). Here we also calmp the > max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc(). > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com><mailto:chaitanya.kulkarni@wdc.com> > --- > block/blk-lib.c | 6 +----- > include/linux/bio.h | 25 +++++++++++++++++++++++++ > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index fb486a0bdb58..ec29415f00dd 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -14,17 +14,13 @@ struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, > sector_t sect, unsigned op, unsigned opf, > unsigned int nr_pages, gfp_t gfp) > { > - struct bio *new = bio_alloc(gfp, nr_pages); > + struct bio *new = bio_new(bdev, sect, op, opf, gfp, nr_pages); > > if (bio) { > bio_chain(bio, new); > submit_bio(bio); > } > > - new->bi_iter.bi_sector = sect; > - bio_set_dev(new, bdev); > - bio_set_op_attrs(new, op, opf); > - > return new; > } > > diff --git a/include/linux/bio.h b/include/linux/bio.h > index c74857cf1252..2a09ba100546 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -826,5 +826,30 @@ static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb) > if (!is_sync_kiocb(kiocb)) > bio->bi_opf |= REQ_NOWAIT; > } > +/** > + * bio_new - allcate and initialize new bio > + * @bdev: blockdev to issue discard for > + * @sector: start sector > + * @op: REQ_OP_XXX from enum req_opf > + * @op_flags: REQ_XXX from enum req_flag_bits > + * @max_bvecs: maximum bvec to be allocated for this bio > + * @gfp_mask: memory allocation flags (for bio_alloc) > + * > + * Description: > + * Allocates, initializes common members, and returns a new bio. > + */ > +static inline struct bio *bio_new(struct block_device *bdev, sector_t sector, > + unsigned int op, unsigned int op_flags, > + unsigned int max_bvecs, gfp_t gfp_mask) > +{ > + unsigned nr_bvec = clamp_t(unsigned int, max_bvecs, 0, BIO_MAX_PAGES); > + struct bio *bio = bio_alloc(gfp_mask, nr_bvec); > > > I think that depending on the gfp_mask passed, bio can be NULL. So this should > be checked. > > > true, I'll add that check. > > > > > + > + bio_set_dev(bio, bdev); > + bio->bi_iter.bi_sector = sector; > + bio_set_op_attrs(bio, op, op_flags); > > > This function is obsolete. Open code this. > > > true, will do. > > > > > + > + return bio; > +} > > #endif /* __LINUX_BIO_H */ > > > > Thanks for the comments Damien. > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel@oss.oracle.com > https://oss.oracle.com/mailman/listinfo/ocfs2-devel
diff --git a/block/blk-lib.c b/block/blk-lib.c index fb486a0bdb58..ec29415f00dd 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -14,17 +14,13 @@ struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, sector_t sect, unsigned op, unsigned opf, unsigned int nr_pages, gfp_t gfp) { - struct bio *new = bio_alloc(gfp, nr_pages); + struct bio *new = bio_new(bdev, sect, op, opf, gfp, nr_pages); if (bio) { bio_chain(bio, new); submit_bio(bio); } - new->bi_iter.bi_sector = sect; - bio_set_dev(new, bdev); - bio_set_op_attrs(new, op, opf); - return new; } diff --git a/include/linux/bio.h b/include/linux/bio.h index c74857cf1252..2a09ba100546 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -826,5 +826,30 @@ static inline void bio_set_polled(struct bio *bio, struct kiocb *kiocb) if (!is_sync_kiocb(kiocb)) bio->bi_opf |= REQ_NOWAIT; } +/** + * bio_new - allcate and initialize new bio + * @bdev: blockdev to issue discard for + * @sector: start sector + * @op: REQ_OP_XXX from enum req_opf + * @op_flags: REQ_XXX from enum req_flag_bits + * @max_bvecs: maximum bvec to be allocated for this bio + * @gfp_mask: memory allocation flags (for bio_alloc) + * + * Description: + * Allocates, initializes common members, and returns a new bio. + */ +static inline struct bio *bio_new(struct block_device *bdev, sector_t sector, + unsigned int op, unsigned int op_flags, + unsigned int max_bvecs, gfp_t gfp_mask) +{ + unsigned nr_bvec = clamp_t(unsigned int, max_bvecs, 0, BIO_MAX_PAGES); + struct bio *bio = bio_alloc(gfp_mask, nr_bvec); + + bio_set_dev(bio, bdev); + bio->bi_iter.bi_sector = sector; + bio_set_op_attrs(bio, op, op_flags); + + return bio; +} #endif /* __LINUX_BIO_H */
Introduce bio_new() helper and use it in blk-lib.c to allocate and initialize various non-optional or semi-optional members of the bio along with bio allocation done with bio_alloc(). Here we also calmp the max_bvecs for bio with BIO_MAX_PAGES before we pass to bio_alloc(). Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- block/blk-lib.c | 6 +----- include/linux/bio.h | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-)