On Wed, Jun 22, 2022 at 6:56 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> ... doing revert if we end up not using some pages
>
With the version from latest vfs.git#for-next as from [1] (which
differs from this one) I see with LLVM-14:
5618: clang -Wp,-MMD,block/.bio.o.d -nostdinc -I./arch/x86/include
-I./arch/x86/include/generated -I./include -I./arch/x86/include/uapi
-I./arch/x86/include/generate
d/uapi -I./include/uapi -I./include/generated/uapi -include
./include/linux/compiler-version.h -include ./include/linux/kconfig.h
-include ./include/linux/compiler_typ
es.h -D__KERNEL__ -Qunused-arguments -fmacro-prefix-map=./= -Wall
-Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
-fno-common -fshort-wchar -fno-
PIE -Werror=implicit-function-declaration -Werror=implicit-int
-Werror=return-type -Wno-format-security -std=gnu11
--target=x86_64-linux-gnu -fintegrated-as -Werror=un
known-warning-option -Werror=ignored-optimization-argument -mno-sse
-mno-mmx -mno-sse2 -mno-3dnow -mno-avx -fcf-protection=none -m64
-falign-loops=1 -mno-80387 -mno-fp
-ret-in-387 -mstack-alignment=8 -mskip-rax-setup -mtune=generic
-mno-red-zone -mcmodel=kernel -Wno-sign-compare
-fno-asynchronous-unwind-tables -mretpoline-external-th
unk -fno-delete-null-pointer-checks -Wno-frame-address
-Wno-address-of-packed-member -O2 -Wframe-larger-than=2048
-fstack-protector-strong -Wimplicit-fallthrough -Wno-
gnu -Wno-unused-but-set-variable -Wno-unused-const-variable
-fno-stack-clash-protection -pg -mfentry -DCC_USING_NOP_MCOUNT
-DCC_USING_FENTRY -fno-lto -flto=thin -fspli
t-lto-unit -fvisibility=hidden -Wdeclaration-after-statement -Wvla
-Wno-pointer-sign -Wcast-function-type -fno-strict-overflow
-fno-stack-check -Werror=date-time -Werr
or=incompatible-pointer-types -Wno-initializer-overrides -Wno-format
-Wno-sign-compare -Wno-format-zero-length -Wno-pointer-to-enum-cast
-Wno-tautological-constant-out
-of-range-compare -Wno-unaligned-access -g -gdwarf-5
-DKBUILD_MODFILE='"block/bio"' -DKBUILD_BASENAME='"bio"'
-DKBUILD_MODNAME='"bio"' -D__KBUILD_MODNAME=kmod_bio -
c -o block/bio.o block/bio.c
[ ... ]
5635:block/bio.c:1232:6: warning: variable 'i' is used uninitialized
whenever 'if' condition is true [-Wsometimes-uninitialized]
5636- if (unlikely(!size)) {
5637- ^~~~~~~~~~~~~~~
5638-./include/linux/compiler.h:78:22: note: expanded from macro 'unlikely'
5639-# define unlikely(x) __builtin_expect(!!(x), 0)
5640- ^~~~~~~~~~~~~~~~~~~~~~~~~~
5641-block/bio.c:1254:9: note: uninitialized use occurs here
5642- while (i < nr_pages)
5643- ^
5644-block/bio.c:1232:2: note: remove the 'if' if its condition is always false
5645- if (unlikely(!size)) {
5646- ^~~~~~~~~~~~~~~~~~~~~~
5647-block/bio.c:1202:17: note: initialize the variable 'i' to silence
this warning
5648- unsigned len, i;
5649- ^
5650- = 0
Patch from [1] is attached.
Regards,
-Sedat-
[1] https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/commit/block/bio.c?h=for-next&id=9a6469060316674230c0666c5706f7097e9278bb
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> block/bio.c | 15 ++++++---------
> block/blk-map.c | 7 ++++---
> 2 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 51c99f2c5c90..01ab683e67be 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1190,7 +1190,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> BUILD_BUG_ON(PAGE_PTRS_PER_BVEC < 2);
> pages += entries_left * (PAGE_PTRS_PER_BVEC - 1);
>
> - size = iov_iter_get_pages(iter, pages, LONG_MAX, nr_pages, &offset);
> + size = iov_iter_get_pages2(iter, pages, LONG_MAX, nr_pages, &offset);
> if (unlikely(size <= 0))
> return size ? size : -EFAULT;
>
> @@ -1205,6 +1205,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> } else {
> if (WARN_ON_ONCE(bio_full(bio, len))) {
> bio_put_pages(pages + i, left, offset);
> + iov_iter_revert(iter, left);
> return -EINVAL;
> }
> __bio_add_page(bio, page, len, offset);
> @@ -1212,7 +1213,6 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> offset = 0;
> }
>
> - iov_iter_advance(iter, size);
> return 0;
> }
>
> @@ -1227,7 +1227,6 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
> ssize_t size, left;
> unsigned len, i;
> size_t offset;
> - int ret = 0;
>
> if (WARN_ON_ONCE(!max_append_sectors))
> return 0;
> @@ -1240,7 +1239,7 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
> BUILD_BUG_ON(PAGE_PTRS_PER_BVEC < 2);
> pages += entries_left * (PAGE_PTRS_PER_BVEC - 1);
>
> - size = iov_iter_get_pages(iter, pages, LONG_MAX, nr_pages, &offset);
> + size = iov_iter_get_pages2(iter, pages, LONG_MAX, nr_pages, &offset);
> if (unlikely(size <= 0))
> return size ? size : -EFAULT;
>
> @@ -1252,16 +1251,14 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
> if (bio_add_hw_page(q, bio, page, len, offset,
> max_append_sectors, &same_page) != len) {
> bio_put_pages(pages + i, left, offset);
> - ret = -EINVAL;
> - break;
> + iov_iter_revert(iter, left);
> + return -EINVAL;
> }
> if (same_page)
> put_page(page);
> offset = 0;
> }
> -
> - iov_iter_advance(iter, size - left);
> - return ret;
> + return 0;
> }
>
> /**
> diff --git a/block/blk-map.c b/block/blk-map.c
> index df8b066cd548..7196a6b64c80 100644
> --- a/block/blk-map.c
> +++ b/block/blk-map.c
> @@ -254,7 +254,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
> size_t offs, added = 0;
> int npages;
>
> - bytes = iov_iter_get_pages_alloc(iter, &pages, LONG_MAX, &offs);
> + bytes = iov_iter_get_pages_alloc2(iter, &pages, LONG_MAX, &offs);
> if (unlikely(bytes <= 0)) {
> ret = bytes ? bytes : -EFAULT;
> goto out_unmap;
> @@ -284,7 +284,6 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
> bytes -= n;
> offs = 0;
> }
> - iov_iter_advance(iter, added);
> }
> /*
> * release the pages we didn't map into the bio, if any
> @@ -293,8 +292,10 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
> put_page(pages[j++]);
> kvfree(pages);
> /* couldn't stuff something into bio? */
> - if (bytes)
> + if (bytes) {
> + iov_iter_revert(iter, bytes);
> break;
> + }
> }
>
> ret = blk_rq_append_bio(rq, bio);
> --
> 2.30.2
>
From 9a6469060316674230c0666c5706f7097e9278bb Mon Sep 17 00:00:00 2001
From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Thu, 9 Jun 2022 10:37:57 -0400
Subject: [PATCH] block: convert to advancing variants of
iov_iter_get_pages{,_alloc}()
... doing revert if we end up not using some pages
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
block/bio.c | 25 ++++++++++++++-----------
block/blk-map.c | 7 ++++---
2 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 082436736d69..d3bc05ed0783 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1200,7 +1200,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
struct page **pages = (struct page **)bv;
ssize_t size, left;
unsigned len, i;
- size_t offset;
+ size_t offset, trim;
int ret = 0;
/*
@@ -1218,16 +1218,19 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
* result to ensure the bio's total size is correct. The remainder of
* the iov data will be picked up in the next bio iteration.
*/
- size = iov_iter_get_pages(iter, pages, UINT_MAX - bio->bi_iter.bi_size,
+ size = iov_iter_get_pages2(iter, pages, UINT_MAX - bio->bi_iter.bi_size,
nr_pages, &offset);
- if (size > 0) {
- nr_pages = DIV_ROUND_UP(offset + size, PAGE_SIZE);
- size = ALIGN_DOWN(size, bdev_logical_block_size(bio->bi_bdev));
- } else
- nr_pages = 0;
-
- if (unlikely(size <= 0)) {
- ret = size ? size : -EFAULT;
+ if (unlikely(size <= 0))
+ return size ? size : -EFAULT;
+
+ nr_pages = DIV_ROUND_UP(offset + size, PAGE_SIZE);
+
+ trim = size & (bdev_logical_block_size(bio->bi_bdev) - 1);
+ iov_iter_revert(iter, trim);
+
+ size -= trim;
+ if (unlikely(!size)) {
+ ret = -EFAULT;
goto out;
}
@@ -1246,7 +1249,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
offset = 0;
}
- iov_iter_advance(iter, size - left);
+ iov_iter_revert(iter, left);
out:
while (i < nr_pages)
put_page(pages[i++]);
diff --git a/block/blk-map.c b/block/blk-map.c
index df8b066cd548..7196a6b64c80 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -254,7 +254,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
size_t offs, added = 0;
int npages;
- bytes = iov_iter_get_pages_alloc(iter, &pages, LONG_MAX, &offs);
+ bytes = iov_iter_get_pages_alloc2(iter, &pages, LONG_MAX, &offs);
if (unlikely(bytes <= 0)) {
ret = bytes ? bytes : -EFAULT;
goto out_unmap;
@@ -284,7 +284,6 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
bytes -= n;
offs = 0;
}
- iov_iter_advance(iter, added);
}
/*
* release the pages we didn't map into the bio, if any
@@ -293,8 +292,10 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter,
put_page(pages[j++]);
kvfree(pages);
/* couldn't stuff something into bio? */
- if (bytes)
+ if (bytes) {
+ iov_iter_revert(iter, bytes);
break;
+ }
}
ret = blk_rq_append_bio(rq, bio);
--
2.36.1
