| Message ID | 20230712211115.2174650-7-kent.overstreet@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Wed, Jul 12, 2023 at 05:11:01PM -0400, Kent Overstreet wrote: > From: Kent Overstreet <kent.overstreet@gmail.com> > > This reverts 6f822e1b5d9dda3d20e87365de138046e3baa03a - this helper is > used by bcachefs. > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > Cc: Jens Axboe <axboe@kernel.dk> > Cc: linux-block@vger.kernel.org > --- > block/bio.c | 6 +++--- > include/linux/bio.h | 7 ++++++- > 2 files changed, 9 insertions(+), 4 deletions(-) I really don't see any point in offering this in the block layer. By the lack of any other caller it obviously isn't such a generic and always used helper, but more importantly it literally is three lines of code to implement it.
On Mon, Jul 24, 2023 at 10:35:45AM -0700, Christoph Hellwig wrote: > On Wed, Jul 12, 2023 at 05:11:01PM -0400, Kent Overstreet wrote: > > From: Kent Overstreet <kent.overstreet@gmail.com> > > > > This reverts 6f822e1b5d9dda3d20e87365de138046e3baa03a - this helper is > > used by bcachefs. > > > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > > Cc: Jens Axboe <axboe@kernel.dk> > > Cc: linux-block@vger.kernel.org > > --- > > block/bio.c | 6 +++--- > > include/linux/bio.h | 7 ++++++- > > 2 files changed, 9 insertions(+), 4 deletions(-) > > I really don't see any point in offering this in the block layer. By > the lack of any other caller it obviously isn't such a generic and > always used helper, but more importantly it literally is three lines > of code to implement it. And yet, we've had a subtle bug introduced in that code that took quite awhile to be fixed - I'm not pro code duplication in general and I don't think this is a good place to start.
On Mon, Jul 24, 2023 at 10:45:53PM -0400, Kent Overstreet wrote: > And yet, we've had a subtle bug introduced in that code that took quite > awhile to be fixed - I'm not pro code duplication in general and I don't > think this is a good place to start. I'm not sure arguing for adding a helper your can triviall implement yourself really helps to streamline your upstreaming process.
On Wed, Jul 26, 2023 at 06:21:23AM -0700, Christoph Hellwig wrote: > On Mon, Jul 24, 2023 at 10:45:53PM -0400, Kent Overstreet wrote: > > And yet, we've had a subtle bug introduced in that code that took quite > > awhile to be fixed - I'm not pro code duplication in general and I don't > > think this is a good place to start. > > I'm not sure arguing for adding a helper your can triviall implement > yourself really helps to streamline your upstreaming process. I gave you my engineering reasons, you're the one who's arguing. And to make everything perfectly clear: this is code that I originally wrote, and then you started changing without CCing me - your patch that deleted zero_fill_bio_iter() never should've gone in.
diff --git a/block/bio.c b/block/bio.c index e74a04ea14..70b5c987bc 100644 --- a/block/bio.c +++ b/block/bio.c @@ -606,15 +606,15 @@ struct bio *bio_kmalloc(unsigned short nr_vecs, gfp_t gfp_mask) } EXPORT_SYMBOL(bio_kmalloc); -void zero_fill_bio(struct bio *bio) +void zero_fill_bio_iter(struct bio *bio, struct bvec_iter start) { struct bio_vec bv; struct bvec_iter iter; - bio_for_each_segment(bv, bio, iter) + __bio_for_each_segment(bv, bio, iter, start) memzero_bvec(&bv); } -EXPORT_SYMBOL(zero_fill_bio); +EXPORT_SYMBOL(zero_fill_bio_iter); /** * bio_truncate - truncate the bio to small size of @new_size diff --git a/include/linux/bio.h b/include/linux/bio.h index b3e7529ff5..f2620f8d18 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -484,7 +484,12 @@ extern void bio_copy_data_iter(struct bio *dst, struct bvec_iter *dst_iter, extern void bio_copy_data(struct bio *dst, struct bio *src); extern void bio_free_pages(struct bio *bio); void guard_bio_eod(struct bio *bio); -void zero_fill_bio(struct bio *bio); +void zero_fill_bio_iter(struct bio *bio, struct bvec_iter iter); + +static inline void zero_fill_bio(struct bio *bio) +{ + zero_fill_bio_iter(bio, bio->bi_iter); +} static inline void bio_release_pages(struct bio *bio, bool mark_dirty) {