Remove the read_from_oldmem() wrapper introduced earlier and convert
all the remaining callers to pass an iov_iter.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
arch/x86/kernel/crash_dump_64.c | 7 +++++-
fs/proc/vmcore.c | 40 +++++++++++++--------------------
include/linux/crash_dump.h | 10 ++++-----
3 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
index 8d91e5f2e14d..991e09b80852 100644
--- a/arch/x86/kernel/crash_dump_64.c
+++ b/arch/x86/kernel/crash_dump_64.c
@@ -68,6 +68,11 @@ ssize_t copy_oldmem_page_encrypted(struct iov_iter *iter, unsigned long pfn,
ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
{
- return read_from_oldmem(buf, count, ppos, 0,
+ struct kvec kvec = { .iov_base = buf, .iov_len = count };
+ struct iov_iter iter;
+
+ iov_iter_kvec(&iter, READ, &kvec, 1, count);
+
+ return read_from_oldmem(&iter, count, ppos,
cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
}
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 443cbaf16ec8..986474f02a02 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -132,7 +132,7 @@ static int open_vmcore(struct inode *inode, struct file *file)
}
/* Reads a page from the oldmem device from given offset. */
-ssize_t read_from_oldmem_iter(struct iov_iter *iter, size_t count,
+ssize_t read_from_oldmem(struct iov_iter *iter, size_t count,
u64 *ppos, bool encrypted)
{
unsigned long pfn, offset;
@@ -180,27 +180,6 @@ ssize_t read_from_oldmem_iter(struct iov_iter *iter, size_t count,
return read;
}
-ssize_t read_from_oldmem(char *buf, size_t count,
- u64 *ppos, int userbuf,
- bool encrypted)
-{
- struct iov_iter iter;
- struct iovec iov;
- struct kvec kvec;
-
- if (userbuf) {
- iov.iov_base = (__force void __user *)buf;
- iov.iov_len = count;
- iov_iter_init(&iter, READ, &iov, 1, count);
- } else {
- kvec.iov_base = buf;
- kvec.iov_len = count;
- iov_iter_kvec(&iter, READ, &kvec, 1, count);
- }
-
- return read_from_oldmem_iter(&iter, count, ppos, encrypted);
-}
-
/*
* Architectures may override this function to allocate ELF header in 2nd kernel
*/
@@ -220,7 +199,12 @@ void __weak elfcorehdr_free(unsigned long long addr)
*/
ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
{
- return read_from_oldmem(buf, count, ppos, 0, false);
+ struct kvec kvec = { .iov_base = buf, .iov_len = count };
+ struct iov_iter iter;
+
+ iov_iter_kvec(&iter, READ, &kvec, 1, count);
+
+ return read_from_oldmem(&iter, count, ppos, false);
}
/*
@@ -228,7 +212,13 @@ ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
*/
ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos)
{
- return read_from_oldmem(buf, count, ppos, 0, cc_platform_has(CC_ATTR_MEM_ENCRYPT));
+ struct kvec kvec = { .iov_base = buf, .iov_len = count };
+ struct iov_iter iter;
+
+ iov_iter_kvec(&iter, READ, &kvec, 1, count);
+
+ return read_from_oldmem(&iter, count, ppos,
+ cc_platform_has(CC_ATTR_MEM_ENCRYPT));
}
/*
@@ -405,7 +395,7 @@ static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos)
m->offset + m->size - *fpos,
iter->count);
start = m->paddr + *fpos - m->offset;
- tmp = read_from_oldmem_iter(iter, tsz, &start,
+ tmp = read_from_oldmem(iter, tsz, &start,
cc_platform_has(CC_ATTR_MEM_ENCRYPT));
if (tmp < 0)
return tmp;
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index a1cf7d5c03c7..0f3a656293b0 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -134,13 +134,11 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data)
#endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
#ifdef CONFIG_PROC_VMCORE
-ssize_t read_from_oldmem(char *buf, size_t count,
- u64 *ppos, int userbuf,
- bool encrypted);
+ssize_t read_from_oldmem(struct iov_iter *iter, size_t count,
+ u64 *ppos, bool encrypted);
#else
-static inline ssize_t read_from_oldmem(char *buf, size_t count,
- u64 *ppos, int userbuf,
- bool encrypted)
+static inline ssize_t read_from_oldmem(struct iov_iter *iter, size_t count,
+ u64 *ppos, bool encrypted)
{
return -EOPNOTSUPP;
}
--
2.33.0
Hi "Matthew,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on arm64/for-next/core powerpc/next s390/features linus/master v5.16-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Convert-vmcore-to-use-an-iov_iter/20211213-080748
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git e463a09af2f0677b9485a7e8e4e70b396b2ffb6f
config: riscv-randconfig-r012-20211213 (https://download.01.org/0day-ci/archive/20211213/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/22576d6aef6fb4cffad0a4e85953662c147dfe66
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Convert-vmcore-to-use-an-iov_iter/20211213-080748
git checkout 22576d6aef6fb4cffad0a4e85953662c147dfe66
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash fs/proc/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
fs/proc/vmcore.c: In function 'read_from_oldmem':
fs/proc/vmcore.c:157:31: error: implicit declaration of function 'iov_iter_zero' [-Werror=implicit-function-declaration]
157 | tmp = iov_iter_zero(nr_bytes, iter);
| ^~~~~~~~~~~~~
fs/proc/vmcore.c: In function 'elfcorehdr_read':
>> fs/proc/vmcore.c:202:16: error: variable 'kvec' has initializer but incomplete type
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~
>> fs/proc/vmcore.c:202:31: error: 'struct kvec' has no member named 'iov_base'
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~~~~~
>> fs/proc/vmcore.c:202:42: warning: excess elements in struct initializer
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~
fs/proc/vmcore.c:202:42: note: (near initialization for 'kvec')
>> fs/proc/vmcore.c:202:48: error: 'struct kvec' has no member named 'iov_len'
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~~~~
fs/proc/vmcore.c:202:58: warning: excess elements in struct initializer
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~~
fs/proc/vmcore.c:202:58: note: (near initialization for 'kvec')
fs/proc/vmcore.c:202:21: error: storage size of 'kvec' isn't known
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~
fs/proc/vmcore.c:203:25: error: storage size of 'iter' isn't known
203 | struct iov_iter iter;
| ^~~~
fs/proc/vmcore.c:205:9: error: implicit declaration of function 'iov_iter_kvec' [-Werror=implicit-function-declaration]
205 | iov_iter_kvec(&iter, READ, &kvec, 1, count);
| ^~~~~~~~~~~~~
fs/proc/vmcore.c:203:25: warning: unused variable 'iter' [-Wunused-variable]
203 | struct iov_iter iter;
| ^~~~
fs/proc/vmcore.c:202:21: warning: unused variable 'kvec' [-Wunused-variable]
202 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~
fs/proc/vmcore.c: In function 'elfcorehdr_read_notes':
fs/proc/vmcore.c:215:16: error: variable 'kvec' has initializer but incomplete type
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~
fs/proc/vmcore.c:215:31: error: 'struct kvec' has no member named 'iov_base'
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~~~~~
fs/proc/vmcore.c:215:42: warning: excess elements in struct initializer
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~
fs/proc/vmcore.c:215:42: note: (near initialization for 'kvec')
fs/proc/vmcore.c:215:48: error: 'struct kvec' has no member named 'iov_len'
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~~~~
fs/proc/vmcore.c:215:58: warning: excess elements in struct initializer
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~~
fs/proc/vmcore.c:215:58: note: (near initialization for 'kvec')
fs/proc/vmcore.c:215:21: error: storage size of 'kvec' isn't known
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~
fs/proc/vmcore.c:216:25: error: storage size of 'iter' isn't known
216 | struct iov_iter iter;
| ^~~~
fs/proc/vmcore.c:216:25: warning: unused variable 'iter' [-Wunused-variable]
fs/proc/vmcore.c:215:21: warning: unused variable 'kvec' [-Wunused-variable]
215 | struct kvec kvec = { .iov_base = buf, .iov_len = count };
| ^~~~
fs/proc/vmcore.c: In function '__read_vmcore':
fs/proc/vmcore.c:327:17: error: invalid use of undefined type 'struct iov_iter'
327 | if (iter->count == 0 || *fpos >= vmcore_size)
| ^~
fs/proc/vmcore.c:331:17: error: invalid use of undefined type 'struct iov_iter'
331 | if (iter->count > vmcore_size - *fpos)
| ^~
fs/proc/vmcore.c:332:21: error: invalid use of undefined type 'struct iov_iter'
332 | iter->count = vmcore_size - *fpos;
| ^~
In file included from include/linux/kernel.h:17,
from include/linux/cpumask.h:10,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/spinlock.h:62,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from fs/proc/vmcore.c:11:
fs/proc/vmcore.c:336:62: error: invalid use of undefined type 'struct iov_iter'
336 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count);
| ^~
include/linux/minmax.h:20:46: note: in definition of macro '__typecheck'
20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^
include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp'
36 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~
include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp'
45 | #define min(x, y) __careful_cmp(x, y, <)
| ^~~~~~~~~~~~~
fs/proc/vmcore.c:336:23: note: in expansion of macro 'min'
336 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count);
| ^~~
In file included from arch/riscv/include/asm/bug.h:10,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from fs/proc/vmcore.c:11:
fs/proc/vmcore.c:336:62: error: invalid use of undefined type 'struct iov_iter'
336 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count);
| ^~
include/linux/const.h:12:55: note: in definition of macro '__is_constexpr'
12 | (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
| ^
include/linux/minmax.h:26:39: note: in expansion of macro '__no_side_effects'
26 | (__typecheck(x, y) && __no_side_effects(x, y))
| ^~~~~~~~~~~~~~~~~
include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp'
36 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~
vim +/kvec +202 fs/proc/vmcore.c
196
197 /*
198 * Architectures may override this function to read from ELF header
199 */
200 ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
201 {
> 202 struct kvec kvec = { .iov_base = buf, .iov_len = count };
203 struct iov_iter iter;
204
205 iov_iter_kvec(&iter, READ, &kvec, 1, count);
206
207 return read_from_oldmem(&iter, count, ppos, false);
208 }
209
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
>
> ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> {
> - return read_from_oldmem(buf, count, ppos, 0,
> + struct kvec kvec = { .iov_base = buf, .iov_len = count };
> + struct iov_iter iter;
> +
> + iov_iter_kvec(&iter, READ, &kvec, 1, count);
> +
> + return read_from_oldmem(&iter, count, ppos,
> cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
> }
elfcorehdr_read should probably also take an iov_iter while we're at it.
I also don't quite understand why we even need the arch overrides for it,
but that would require some digging into the history of this interface.
On 12/13/21 at 09:02am, Christoph Hellwig wrote:
> >
> > ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> > {
> > - return read_from_oldmem(buf, count, ppos, 0,
> > + struct kvec kvec = { .iov_base = buf, .iov_len = count };
> > + struct iov_iter iter;
> > +
> > + iov_iter_kvec(&iter, READ, &kvec, 1, count);
> > +
> > + return read_from_oldmem(&iter, count, ppos,
> > cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
> > }
>
> elfcorehdr_read should probably also take an iov_iter while we're at it.
>
> I also don't quite understand why we even need the arch overrides for it,
> but that would require some digging into the history of this interface.
>
Below patchset removing sec_active() from generic code added this arch
override on x86_64. Before that, s390 and arm64 have had arch overrides.
And arm64 says "elfcorehdr_read() is simple as the region is always mapped."
in commit e62aaeac42 "arm64: kdump: provide /proc/vmcore file".
[v3,0/6] Remove x86-specific code from generic headers
https://patchwork.kernel.org/project/linux-fsdevel/cover/[email protected]/
5cbdaeefb655 s390/mm: Remove sev_active() function
ae7eb82a92fa fs/core/vmcore: Move sev_active() reference to x86 arch code
284e21fab2cf x86, s390/mm: Move sme_active() and sme_me_mask to x86-specific header
e740815a97e2 dma-mapping: Remove dma_check_mask()
47e5d8f9ed34 swiotlb: Remove call to sme_active()
0c9c1d563975 x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
On Mon, Dec 13, 2021 at 09:02:57AM +0100, Christoph Hellwig wrote:
> >
> > ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> > {
> > - return read_from_oldmem(buf, count, ppos, 0,
> > + struct kvec kvec = { .iov_base = buf, .iov_len = count };
> > + struct iov_iter iter;
> > +
> > + iov_iter_kvec(&iter, READ, &kvec, 1, count);
> > +
> > + return read_from_oldmem(&iter, count, ppos,
> > cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
> > }
>
> elfcorehdr_read should probably also take an iov_iter while we're at it.
I don't like how that looks. For example:
@@ -1246,11 +1247,14 @@ static int __init parse_crash_elf64_headers(void)
int rc=0;
Elf64_Ehdr ehdr;
u64 addr;
+ struct iov_iter iter;
+ struct kvec kvec = { .iov_base = &ehdr, .iov_len = sizeof(ehdr) };
addr = elfcorehdr_addr;
/* Read Elf header */
- rc = elfcorehdr_read((char *)&ehdr, sizeof(Elf64_Ehdr), &addr);
+ iov_iter_kvec(&iter, READ, &kvec, 1, sizeof(ehdr));
+ rc = elfcorehdr_read(&iter, &addr);
if (rc < 0)
return rc;
@@ -1277,7 +1281,10 @@ static int __init parse_crash_elf64_headers(void)
if (!elfcorebuf)
return -ENOMEM;
addr = elfcorehdr_addr;
- rc = elfcorehdr_read(elfcorebuf, elfcorebuf_sz_orig, &addr);
+ kvec.iov_base = elfcorebuf;
+ kvec.iov_len = elfcorebuf_sz_orig;
+ iov_iter_kvec(&iter, READ, &kvec, 1, elfcorebuf_sz_orig);
+ rc = elfcorehdr_read(&iter, &addr);
if (rc < 0)
goto fail;
I don't think this makes the code clearer, and it's already pretty ugly
code. I'd rather leave constructing the iov_iter to elfcorehdr_read().
(Same remarks apply to elfcorehdr_read_notes())
> I also don't quite understand why we even need the arch overrides for it,
> but that would require some digging into the history of this interface.
I decided I didn't want to know ;-)