| Message ID | 20180611194806.13222-2-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 11, 2018 at 09:48:01PM +0200, Christoph Hellwig wrote: > This abstracts out a way to reuse a bio without destroying the > data pointers. What is the point of this? What "data pointers" does it not destroy? > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/bio.c | 20 ++++++++++++++++++++ > include/linux/bio.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/block/bio.c b/block/bio.c > index 70c4e1b6dd45..fa1b7ab50784 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -308,6 +308,26 @@ void bio_reset(struct bio *bio) > } > EXPORT_SYMBOL(bio_reset); > > +/** > + * bio_reuse - prepare a bio for reuse > + * @bio: bio to reuse > + * > + * Prepares an already setup and possible used bio for reusing it another > + * time. Compared to bio_reset() this preserves the bio size and the > + * layout and contents of the bio vectors. > + */ > +void bio_reuse(struct bio *bio) > +{ > + unsigned int size = bio->bi_iter.bi_size; > + unsigned short vcnt = bio->bi_vcnt; > + > + bio_reset(bio); > + > + bio->bi_iter.bi_size = size; > + bio->bi_vcnt = vcnt; > +} > +EXPORT_SYMBOL_GPL(bio_reuse); > + > static struct bio *__bio_chain_endio(struct bio *bio) > { > struct bio *parent = bio->bi_private; > diff --git a/include/linux/bio.h b/include/linux/bio.h > index f08f5fe7bd08..15c871ab50db 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -475,6 +475,7 @@ extern void bio_init(struct bio *bio, struct bio_vec *table, > unsigned short max_vecs); > extern void bio_uninit(struct bio *); > extern void bio_reset(struct bio *); > +void bio_reuse(struct bio *); > void bio_chain(struct bio *, struct bio *); > > extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int); > -- > 2.17.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bcache" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 12, 2018 at 02:16:30AM -0400, Kent Overstreet wrote: > On Mon, Jun 11, 2018 at 09:48:01PM +0200, Christoph Hellwig wrote: > > This abstracts out a way to reuse a bio without destroying the > > data pointers. > > What is the point of this? What "data pointers" does it not destroy? It keeps bi_vcnt and bi_size intact in addition to bi_max_vecs/bi_io_vec already kept by the existing bio_reset. The befit is that you don't need to rebuild bi_vcnt, bio_io_vec and bi_size when reusing the bio.
On Wed, Jun 13, 2018 at 09:32:04AM +0200, Christoph Hellwig wrote: > On Tue, Jun 12, 2018 at 02:16:30AM -0400, Kent Overstreet wrote: > > On Mon, Jun 11, 2018 at 09:48:01PM +0200, Christoph Hellwig wrote: > > > This abstracts out a way to reuse a bio without destroying the > > > data pointers. > > > > What is the point of this? What "data pointers" does it not destroy? > > It keeps bi_vcnt and bi_size intact in addition to bi_max_vecs/bi_io_vec > already kept by the existing bio_reset. > > The befit is that you don't need to rebuild bi_vcnt, bio_io_vec and > bi_size when reusing the bio. bi_size is not immutable though, it will usually be modified by drivers when you submit a bio. I see what you're trying to do, but your approach is busted given the way the block layer works today. You'd have to save bio->bi_iter before submitting the bio and restore it afterwards for it to work.
On Wed, Jun 13, 2018 at 04:54:41AM -0400, Kent Overstreet wrote: > bi_size is not immutable though, it will usually be modified by drivers when you > submit a bio. > > I see what you're trying to do, but your approach is busted given the way the > block layer works today. You'd have to save bio->bi_iter before submitting the > bio and restore it afterwards for it to work. For bi_size, agreed this needs fixing. bi_sector is always restored already by the callers, and the remaining fields are zeroed by bio_reset, which does the right thing.
On Wed, Jun 13, 2018 at 03:59:15PM +0200, Christoph Hellwig wrote: > On Wed, Jun 13, 2018 at 04:54:41AM -0400, Kent Overstreet wrote: > > bi_size is not immutable though, it will usually be modified by drivers when you > > submit a bio. > > > > I see what you're trying to do, but your approach is busted given the way the > > block layer works today. You'd have to save bio->bi_iter before submitting the > > bio and restore it afterwards for it to work. > > For bi_size, agreed this needs fixing. bi_sector is always restored > already by the callers, and the remaining fields are zeroed by bio_reset, > which does the right thing. This still shouldn't be a new helper. If the caller is restoring bi_sector they can restore bi_size as well, and restoring only part of bi_iter should not be encouraged as it's not safe in general - it is a quite reasonable thing to want to restore a bio to the state it was pre submission (e.g. for bouncing) and what you're doing is definitely _not_ safe in general.
diff --git a/block/bio.c b/block/bio.c index 70c4e1b6dd45..fa1b7ab50784 100644 --- a/block/bio.c +++ b/block/bio.c @@ -308,6 +308,26 @@ void bio_reset(struct bio *bio) } EXPORT_SYMBOL(bio_reset); +/** + * bio_reuse - prepare a bio for reuse + * @bio: bio to reuse + * + * Prepares an already setup and possible used bio for reusing it another + * time. Compared to bio_reset() this preserves the bio size and the + * layout and contents of the bio vectors. + */ +void bio_reuse(struct bio *bio) +{ + unsigned int size = bio->bi_iter.bi_size; + unsigned short vcnt = bio->bi_vcnt; + + bio_reset(bio); + + bio->bi_iter.bi_size = size; + bio->bi_vcnt = vcnt; +} +EXPORT_SYMBOL_GPL(bio_reuse); + static struct bio *__bio_chain_endio(struct bio *bio) { struct bio *parent = bio->bi_private; diff --git a/include/linux/bio.h b/include/linux/bio.h index f08f5fe7bd08..15c871ab50db 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -475,6 +475,7 @@ extern void bio_init(struct bio *bio, struct bio_vec *table, unsigned short max_vecs); extern void bio_uninit(struct bio *); extern void bio_reset(struct bio *); +void bio_reuse(struct bio *); void bio_chain(struct bio *, struct bio *); extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
This abstracts out a way to reuse a bio without destroying the data pointers. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bio.c | 20 ++++++++++++++++++++ include/linux/bio.h | 1 + 2 files changed, 21 insertions(+)