| Message ID | 20240312144532.1044427-3-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] block: move discard checks into the ioctl handler | expand |
On Tue, Mar 12, 2024 at 08:45:28AM -0600, Christoph Hellwig wrote: > +struct bio *bio_chain_and_submit(struct bio *prev, struct bio *new) > { > - struct bio *new = bio_alloc(bdev, nr_pages, opf, gfp); > - > - if (bio) { > - bio_chain(bio, new); > - submit_bio(bio); > + if (prev) { > + bio_chain(prev, new); > + submit_bio(prev); > } > - > return new; > } > +EXPORT_SYMBOL_GPL(bio_chain_and_submit); > + > +struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, > + unsigned int nr_pages, blk_opf_t opf, gfp_t gfp) > +{ > + return bio_chain_and_submit(bio, bio_alloc(bdev, nr_pages, opf, gfp)); > +} I realize you're not changing any behavior here, but I want to ask, is bio_alloc() always guaranteed to return a valid bio? It sure looks like it can return NULL under some uncommon conditions, but I can't find anyone checking the result. So I guess it's safe?
On Tue, Mar 12, 2024 at 08:51:35AM -0600, Keith Busch wrote: > > + > > +struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, > > + unsigned int nr_pages, blk_opf_t opf, gfp_t gfp) > > +{ > > + return bio_chain_and_submit(bio, bio_alloc(bdev, nr_pages, opf, gfp)); > > +} > > I realize you're not changing any behavior here, but I want to ask, is > bio_alloc() always guaranteed to return a valid bio? It sure looks like > it can return NULL under some uncommon conditions, but I can't find > anyone checking the result. So I guess it's safe? bio_alloc can only fail if we don't wait for allocations, that is if __GFP_DIRECT_RECLAIM isn't set. We could an assert here. Or work on killing the gfp_flags argument and just add a bio_alloc_nowait for the few cases that need it.
diff --git a/block/bio.c b/block/bio.c index d24420ed1c4c6f..32ff538b29e564 100644 --- a/block/bio.c +++ b/block/bio.c @@ -345,18 +345,30 @@ void bio_chain(struct bio *bio, struct bio *parent) } EXPORT_SYMBOL(bio_chain); -struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, - unsigned int nr_pages, blk_opf_t opf, gfp_t gfp) +/** + * bio_chain_and_submit - submit a bio after chaining it to another one + * @prev: bio to chain and submit + * @new: bio to chain to + * + * If @prev is non-NULL, chain it to @new and submit it. + * + * Return: @new. + */ +struct bio *bio_chain_and_submit(struct bio *prev, struct bio *new) { - struct bio *new = bio_alloc(bdev, nr_pages, opf, gfp); - - if (bio) { - bio_chain(bio, new); - submit_bio(bio); + if (prev) { + bio_chain(prev, new); + submit_bio(prev); } - return new; } +EXPORT_SYMBOL_GPL(bio_chain_and_submit); + +struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, + unsigned int nr_pages, blk_opf_t opf, gfp_t gfp) +{ + return bio_chain_and_submit(bio, bio_alloc(bdev, nr_pages, opf, gfp)); +} EXPORT_SYMBOL_GPL(blk_next_bio); static void bio_alloc_rescue(struct work_struct *work) diff --git a/include/linux/bio.h b/include/linux/bio.h index 875d792bffff82..643d61b7cb82f7 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -824,5 +824,6 @@ static inline void bio_clear_polled(struct bio *bio) struct bio *blk_next_bio(struct bio *bio, struct block_device *bdev, unsigned int nr_pages, blk_opf_t opf, gfp_t gfp); +struct bio *bio_chain_and_submit(struct bio *prev, struct bio *new); #endif /* __LINUX_BIO_H */
This is basically blk_next_bio just with the bio allocation moved to the caller to allow for more flexible bio handling in the caller. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bio.c | 28 ++++++++++++++++++++-------- include/linux/bio.h | 1 + 2 files changed, 21 insertions(+), 8 deletions(-)