* [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all
@ 2019-04-08 11:02 Ming Lei
2019-04-08 11:20 ` Johannes Thumshirn
2019-04-08 14:19 ` Jens Axboe
0 siblings, 2 replies; 5+ messages in thread
From: Ming Lei @ 2019-04-08 11:02 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, Qu Wenruo, linux-btrfs, linux-fsdevel,
Omar Sandoval, Christoph Hellwig
Commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
iterate over multi-page bvec") changes bio_for_each_segment_all()
to use for-inside-for.
This way breaks all bio_for_each_segment_all() call with error out
branch via 'break', since now 'break' can only break from the inner
loop.
Fixes this issue by implementing bio_for_each_segment_all() via
single 'for' loop, and now the logic is very similar with normal
bvec iterator.
Cc: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: linux-btrfs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: Omar Sandoval <osandov@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
Reported-and-Tested-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Fixes: 6dc4f100c175 ("block: allow bio_for_each_segment_all() to iterate over multi-page bvec")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
V3:
- don't rename for avoiding conflict with hch's patches
V2:
- address comments: avoid explicated expressiones in macro, naming
include/linux/bio.h | 20 ++++++++++++--------
include/linux/bvec.h | 14 ++++++++++----
2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h
index bb6090aa165d..e584673c1881 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -120,19 +120,23 @@ static inline bool bio_full(struct bio *bio)
return bio->bi_vcnt >= bio->bi_max_vecs;
}
-#define mp_bvec_for_each_segment(bv, bvl, i, iter_all) \
- for (bv = bvec_init_iter_all(&iter_all); \
- (iter_all.done < (bvl)->bv_len) && \
- (mp_bvec_next_segment((bvl), &iter_all), 1); \
- iter_all.done += bv->bv_len, i += 1)
+static inline bool bio_next_segment(const struct bio *bio,
+ struct bvec_iter_all *iter)
+{
+ if (iter->idx >= bio->bi_vcnt)
+ return false;
+
+ bvec_advance(&bio->bi_io_vec[iter->idx], iter);
+ return true;
+}
/*
* drivers should _never_ use the all version - the bio may have been split
* before it got to the driver and the driver won't own all of it
*/
-#define bio_for_each_segment_all(bvl, bio, i, iter_all) \
- for (i = 0, iter_all.idx = 0; iter_all.idx < (bio)->bi_vcnt; iter_all.idx++) \
- mp_bvec_for_each_segment(bvl, &((bio)->bi_io_vec[iter_all.idx]), i, iter_all)
+#define bio_for_each_segment_all(bvl, bio, i, iter) \
+ for (i = 0, bvl = bvec_init_iter_all(&iter); \
+ bio_next_segment((bio), &iter); i++)
static inline void bio_advance_iter(struct bio *bio, struct bvec_iter *iter,
unsigned bytes)
diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index f6275c4da13a..3bc91879e1e2 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -145,18 +145,18 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv,
static inline struct bio_vec *bvec_init_iter_all(struct bvec_iter_all *iter_all)
{
- iter_all->bv.bv_page = NULL;
iter_all->done = 0;
+ iter_all->idx = 0;
return &iter_all->bv;
}
-static inline void mp_bvec_next_segment(const struct bio_vec *bvec,
- struct bvec_iter_all *iter_all)
+static inline void bvec_advance(const struct bio_vec *bvec,
+ struct bvec_iter_all *iter_all)
{
struct bio_vec *bv = &iter_all->bv;
- if (bv->bv_page) {
+ if (iter_all->done) {
bv->bv_page = nth_page(bv->bv_page, 1);
bv->bv_offset = 0;
} else {
@@ -165,6 +165,12 @@ static inline void mp_bvec_next_segment(const struct bio_vec *bvec,
}
bv->bv_len = min_t(unsigned int, PAGE_SIZE - bv->bv_offset,
bvec->bv_len - iter_all->done);
+ iter_all->done += bv->bv_len;
+
+ if (iter_all->done == bvec->bv_len) {
+ iter_all->idx++;
+ iter_all->done = 0;
+ }
}
/*
--
2.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all
2019-04-08 11:02 [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all Ming Lei
@ 2019-04-08 11:20 ` Johannes Thumshirn
2019-04-08 14:19 ` Jens Axboe
1 sibling, 0 replies; 5+ messages in thread
From: Johannes Thumshirn @ 2019-04-08 11:20 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Qu Wenruo, linux-btrfs, linux-fsdevel,
Omar Sandoval, Christoph Hellwig
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all
2019-04-08 11:02 [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all Ming Lei
2019-04-08 11:20 ` Johannes Thumshirn
@ 2019-04-08 14:19 ` Jens Axboe
2019-04-11 13:26 ` David Sterba
1 sibling, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2019-04-08 14:19 UTC (permalink / raw)
To: Ming Lei
Cc: linux-block, Qu Wenruo, linux-btrfs, linux-fsdevel,
Omar Sandoval, Christoph Hellwig
On 4/8/19 5:02 AM, Ming Lei wrote:
> Commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
> iterate over multi-page bvec") changes bio_for_each_segment_all()
> to use for-inside-for.
>
> This way breaks all bio_for_each_segment_all() call with error out
> branch via 'break', since now 'break' can only break from the inner
> loop.
>
> Fixes this issue by implementing bio_for_each_segment_all() via
> single 'for' loop, and now the logic is very similar with normal
> bvec iterator.
Applied, thanks Ming.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all
2019-04-08 14:19 ` Jens Axboe
@ 2019-04-11 13:26 ` David Sterba
2019-04-11 15:07 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: David Sterba @ 2019-04-11 13:26 UTC (permalink / raw)
To: Jens Axboe
Cc: Ming Lei, linux-block, Qu Wenruo, linux-btrfs, linux-fsdevel,
Omar Sandoval, Christoph Hellwig
On Mon, Apr 08, 2019 at 08:19:59AM -0600, Jens Axboe wrote:
> On 4/8/19 5:02 AM, Ming Lei wrote:
> > Commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
> > iterate over multi-page bvec") changes bio_for_each_segment_all()
> > to use for-inside-for.
> >
> > This way breaks all bio_for_each_segment_all() call with error out
> > branch via 'break', since now 'break' can only break from the inner
> > loop.
> >
> > Fixes this issue by implementing bio_for_each_segment_all() via
> > single 'for' loop, and now the logic is very similar with normal
> > bvec iterator.
>
> Applied, thanks Ming.
What's the merge target please? This affects btrfs testing so I wonder
if we should start adding workarounds or just wait for the next rc.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all
2019-04-11 13:26 ` David Sterba
@ 2019-04-11 15:07 ` Jens Axboe
0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2019-04-11 15:07 UTC (permalink / raw)
To: dsterba, Ming Lei, linux-block, Qu Wenruo, linux-btrfs,
linux-fsdevel, Omar Sandoval, Christoph Hellwig
On 4/11/19 7:26 AM, David Sterba wrote:
> On Mon, Apr 08, 2019 at 08:19:59AM -0600, Jens Axboe wrote:
>> On 4/8/19 5:02 AM, Ming Lei wrote:
>>> Commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
>>> iterate over multi-page bvec") changes bio_for_each_segment_all()
>>> to use for-inside-for.
>>>
>>> This way breaks all bio_for_each_segment_all() call with error out
>>> branch via 'break', since now 'break' can only break from the inner
>>> loop.
>>>
>>> Fixes this issue by implementing bio_for_each_segment_all() via
>>> single 'for' loop, and now the logic is very similar with normal
>>> bvec iterator.
>>
>> Applied, thanks Ming.
>
> What's the merge target please? This affects btrfs testing so I wonder
> if we should start adding workarounds or just wait for the next rc.
> Thanks.
It's queued up in my for-linus, so it's for this series. It'll go
upstream tomorrow.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-11 15:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 11:02 [PATCH V3] block: don't use for-inside-for in bio_for_each_segment_all Ming Lei
2019-04-08 11:20 ` Johannes Thumshirn
2019-04-08 14:19 ` Jens Axboe
2019-04-11 13:26 ` David Sterba
2019-04-11 15:07 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).