On Fri, Aug 07, 2020 at 09:41:14PM +0800, Ming Lei wrote:
> On Fri, Aug 07, 2020 at 01:38:54PM +0100, Al Viro wrote:
> > FWIW, my preference would be to have for_each_bvec() advance past zero-length
> > segments; I'll need to go through its uses elsewhere in the tree first, though
> > (after I grab some sleep),
>
> Usually block layer doesn't allow/support zero bvec, however we can make
> for_each_bvec() to support it only.
>
> Tetsuo, can you try the following patch?
>
> diff --git a/include/linux/bvec.h b/include/linux/bvec.h
> index ac0c7299d5b8..b03c793dd28d 100644
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -117,11 +117,19 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv,
> return true;
> }
>
> +static inline void bvec_iter_skip_zero_vec(const struct bio_vec *bv,
> + struct bvec_iter *iter)
> +{
> + iter->bi_idx++;
> + iter->bi_bvec_done = 0;
> +}
> +
> #define for_each_bvec(bvl, bio_vec, iter, start) \
> for (iter = (start); \
> (iter).bi_size && \
> - ((bvl = bvec_iter_bvec((bio_vec), (iter))), 1); \
> - bvec_iter_advance((bio_vec), &(iter), (bvl).bv_len))
> + ((bvl = bvec_iter_bvec((bio_vec), (iter))), 1); \
> + (bvl).bv_len ? bvec_iter_advance((bio_vec), &(iter), (bvl).bv_len) : \
> + bvec_iter_skip_zero_vec((bio_vec), &(iter)))
Uhm, bvec_iter_advance() already skips over zero length bio_vecs.
while (bytes && bytes >= bv[idx].bv_len) {
bytes -= bv[idx].bv_len;
idx++;
}
The problem is when the _first_ bio_vec is zero length. Maybe something more
like this (which doesn't even compile, but hopefully makes my point):
@@ -86,12 +86,24 @@ struct bvec_iter_all {
(mp_bvec_iter_page((bvec), (iter)) + \
mp_bvec_iter_page_idx((bvec), (iter)))
-#define bvec_iter_bvec(bvec, iter) \
-((struct bio_vec) { \
- .bv_page = bvec_iter_page((bvec), (iter)), \
- .bv_len = bvec_iter_len((bvec), (iter)), \
- .bv_offset = bvec_iter_offset((bvec), (iter)), \
-})
+static inline bool bvec_iter_bvec(struct bio_vec *bv, struct bio_vec *bvec,
+ struct bvec_iter *iter)
+{
+ unsigned int idx = iter->bi_idx;
+
+ if (!iter->bi_size)
+ return false;
+
+ while (!bv[idx].bv_len)
+ idx++;
+ iter->bi_idx = idx;
+
+ bv->bv_page = bvec_iter_page(bvec, *iter);
+ bv->bv_len = bvec_iter_len(bvec, *iter);
+ bv->bv_offset = bvec_iter_offset(bvec, *iter);
+
+ return true;
+}
static inline bool bvec_iter_advance(const struct bio_vec *bv,
struct bvec_iter *iter, unsigned bytes)
@@ -119,8 +131,7 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv,
#define for_each_bvec(bvl, bio_vec, iter, start) \
for (iter = (start); \
- (iter).bi_size && \
- ((bvl = bvec_iter_bvec((bio_vec), (iter))), 1); \
+ bvec_iter_bvec(&(bvl), (bio_vec), &(iter)); \
bvec_iter_advance((bio_vec), &(iter), (bvl).bv_len))
/* for iterating one bio from start to end */
(I find the whole bvec handling a mess of confusing macros and would
welcome more of it being inline functions, in general).
