diff mbox series

[06/20] block: Bring back zero_fill_bio_iter

Message ID 20230712211115.2174650-7-kent.overstreet@linux.dev
State New
Headers
Series bcachefs prereqs patch series |

Commit Message

Kent Overstreet July 12, 2023, 9:11 p.m. UTC 
  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(-)

Comments

Christoph Hellwig July 24, 2023, 5:35 p.m. UTC | #1 
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.
Kent Overstreet July 25, 2023, 2:45 a.m. UTC | #2 
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.
Christoph Hellwig July 26, 2023, 1:21 p.m. UTC | #3 
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.
Kent Overstreet Aug. 1, 2023, 7:05 p.m. UTC | #4 
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 mbox series

Patch

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