* [PATCH 0/3] Convert vmcore to use an iov_iter
@ 2021-12-13 0:06 Matthew Wilcox (Oracle)
2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle)
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-12-13 0:06 UTC (permalink / raw)
To: Baoquan He, Vivek Goyal, Dave Young, kexec
Cc: Matthew Wilcox (Oracle),
Tiezhu Yang, linux-kernel, Amit Daniel Kachhap,
Christoph Hellwig, linux-s390, linux-fsdevel
For some reason several people have been sending bad patches to fix
compiler warnings in vmcore recently. Here's how it should be done.
Compile-tested only on x86. As noted in the first patch, s390 should
take this conversion a bit further, but I'm not inclined to do that
work myself.
Matthew Wilcox (Oracle) (3):
vmcore: Convert copy_oldmem_page() to take an iov_iter
vmcore: Convert __read_vmcore to use an iov_iter
vmcore: Convert read_from_oldmem() to take an iov_iter
arch/arm/kernel/crash_dump.c | 14 +---
arch/arm64/kernel/crash_dump.c | 14 +---
arch/ia64/kernel/crash_dump.c | 12 +--
arch/mips/kernel/crash_dump.c | 13 +---
arch/powerpc/kernel/crash_dump.c | 20 +----
arch/riscv/kernel/crash_dump.c | 13 +---
arch/s390/kernel/crash_dump.c | 12 +--
arch/sh/kernel/crash_dump.c | 15 +---
arch/x86/kernel/crash_dump_32.c | 13 +---
arch/x86/kernel/crash_dump_64.c | 31 ++++----
fs/proc/vmcore.c | 130 +++++++++++++------------------
include/linux/crash_dump.h | 19 ++---
12 files changed, 112 insertions(+), 194 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take an iov_iter 2021-12-13 0:06 [PATCH 0/3] Convert vmcore to use an iov_iter Matthew Wilcox (Oracle) @ 2021-12-13 0:06 ` Matthew Wilcox (Oracle) 2021-12-13 1:34 ` Matthew Wilcox ` (2 more replies) 2021-12-13 0:06 ` [PATCH 2/3] vmcore: Convert __read_vmcore to use " Matthew Wilcox (Oracle) 2021-12-13 0:06 ` [PATCH 3/3] vmcore: Convert read_from_oldmem() to take " Matthew Wilcox (Oracle) 2 siblings, 3 replies; 14+ messages in thread From: Matthew Wilcox (Oracle) @ 2021-12-13 0:06 UTC (permalink / raw) To: Baoquan He, Vivek Goyal, Dave Young, kexec Cc: Matthew Wilcox (Oracle), Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel Instead of passing in a 'buf' and 'userbuf' argument, pass in an iov_iter. s390 needs more work to pass the iov_iter down further, or refactor, but I'd be more comfortable if someone who can test on s390 did that work. It's more convenient to convert the whole of read_from_oldmem() to take an iov_iter at the same time, so rename it to read_from_oldmem_iter() and add a temporary read_from_oldmem() wrapper that creates an iov_iter. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- arch/arm/kernel/crash_dump.c | 14 ++------- arch/arm64/kernel/crash_dump.c | 14 ++------- arch/ia64/kernel/crash_dump.c | 12 ++------ arch/mips/kernel/crash_dump.c | 13 ++------ arch/powerpc/kernel/crash_dump.c | 20 +++--------- arch/riscv/kernel/crash_dump.c | 13 ++------ arch/s390/kernel/crash_dump.c | 12 +++++--- arch/sh/kernel/crash_dump.c | 15 +++------ arch/x86/kernel/crash_dump_32.c | 13 ++------ arch/x86/kernel/crash_dump_64.c | 24 ++++++--------- fs/proc/vmcore.c | 53 ++++++++++++++++++++------------ include/linux/crash_dump.h | 9 +++--- 12 files changed, 79 insertions(+), 133 deletions(-) diff --git a/arch/arm/kernel/crash_dump.c b/arch/arm/kernel/crash_dump.c index 53cb92435392..15d7db782f24 100644 --- a/arch/arm/kernel/crash_dump.c +++ b/arch/arm/kernel/crash_dump.c @@ -27,9 +27,8 @@ * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes * copied or negative error in case of failure. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, - int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void *vaddr; @@ -40,14 +39,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user(buf, vaddr + offset, csize)) { - iounmap(vaddr); - return -EFAULT; - } - } else { - memcpy(buf, vaddr + offset, csize); - } + csize = copy_to_iter(vaddr + offset, csize, iter); iounmap(vaddr); return csize; diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c index 58303a9ec32c..991d30aac439 100644 --- a/arch/arm64/kernel/crash_dump.c +++ b/arch/arm64/kernel/crash_dump.c @@ -25,9 +25,8 @@ * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes * copied or negative error in case of failure. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, - int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void *vaddr; @@ -38,14 +37,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user((char __user *)buf, vaddr + offset, csize)) { - memunmap(vaddr); - return -EFAULT; - } - } else { - memcpy(buf, vaddr + offset, csize); - } + csize = copy_to_iter(vaddr + offset, csize, iter); memunmap(vaddr); diff --git a/arch/ia64/kernel/crash_dump.c b/arch/ia64/kernel/crash_dump.c index 0ed3c3dee4cd..bc331272bbd6 100644 --- a/arch/ia64/kernel/crash_dump.c +++ b/arch/ia64/kernel/crash_dump.c @@ -31,21 +31,15 @@ * copying the data to a pre-allocated kernel page and then copying to user * space in non-atomic context. */ -ssize_t -copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void *vaddr; if (!csize) return 0; vaddr = __va(pfn<<PAGE_SHIFT); - if (userbuf) { - if (copy_to_user(buf, (vaddr + offset), csize)) { - return -EFAULT; - } - } else - memcpy(buf, (vaddr + offset), csize); + csize = copy_to_iter(vaddr + offset, csize, iter); return csize; } diff --git a/arch/mips/kernel/crash_dump.c b/arch/mips/kernel/crash_dump.c index 2e50f55185a6..926665c798b6 100644 --- a/arch/mips/kernel/crash_dump.c +++ b/arch/mips/kernel/crash_dump.c @@ -15,8 +15,8 @@ * Copy a page from "oldmem". For this page, there is no pte mapped * in the current kernel. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void *vaddr; @@ -24,14 +24,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, return 0; vaddr = kmap_local_pfn(pfn); - - if (!userbuf) { - memcpy(buf, vaddr + offset, csize); - } else { - if (copy_to_user(buf, vaddr + offset, csize)) - csize = -EFAULT; - } - + csize = copy_to_iter(vaddr + offset, csize, iter); kunmap_local(vaddr); return csize; diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c index 5693e1c67c2b..541bcc01490c 100644 --- a/arch/powerpc/kernel/crash_dump.c +++ b/arch/powerpc/kernel/crash_dump.c @@ -68,18 +68,6 @@ void __init setup_kdump_trampoline(void) } #endif /* CONFIG_NONSTATIC_KERNEL */ -static size_t copy_oldmem_vaddr(void *vaddr, char *buf, size_t csize, - unsigned long offset, int userbuf) -{ - if (userbuf) { - if (copy_to_user((char __user *)buf, (vaddr + offset), csize)) - return -EFAULT; - } else - memcpy(buf, (vaddr + offset), csize); - - return csize; -} - /** * copy_oldmem_page - copy one page from "oldmem" * @pfn: page frame number to be copied @@ -93,8 +81,8 @@ static size_t copy_oldmem_vaddr(void *vaddr, char *buf, size_t csize, * Copy a page from "oldmem". For this page, there is no pte mapped * in the current kernel. We stitch up a pte, similar to kmap_atomic. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void *vaddr; phys_addr_t paddr; @@ -107,10 +95,10 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (memblock_is_region_memory(paddr, csize)) { vaddr = __va(paddr); - csize = copy_oldmem_vaddr(vaddr, buf, csize, offset, userbuf); + csize = copy_to_iter(vaddr + offset, csize, iter); } else { vaddr = ioremap_cache(paddr, PAGE_SIZE); - csize = copy_oldmem_vaddr(vaddr, buf, csize, offset, userbuf); + csize = copy_to_iter(vaddr + offset, csize, iter); iounmap(vaddr); } diff --git a/arch/riscv/kernel/crash_dump.c b/arch/riscv/kernel/crash_dump.c index 86cc0ada5752..f4dffb8c7eb7 100644 --- a/arch/riscv/kernel/crash_dump.c +++ b/arch/riscv/kernel/crash_dump.c @@ -20,9 +20,8 @@ * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes * copied or negative error in case of failure. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, - int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void *vaddr; @@ -33,13 +32,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user((char __user *)buf, vaddr + offset, csize)) { - memunmap(vaddr); - return -EFAULT; - } - } else - memcpy(buf, vaddr + offset, csize); + csize = copy_to_iter(vaddr + offset, csize, iter); memunmap(vaddr); return csize; diff --git a/arch/s390/kernel/crash_dump.c b/arch/s390/kernel/crash_dump.c index 785d54c9350c..09bf559f2c5b 100644 --- a/arch/s390/kernel/crash_dump.c +++ b/arch/s390/kernel/crash_dump.c @@ -214,8 +214,8 @@ static int copy_oldmem_user(void __user *dst, void *src, size_t count) /* * Copy one page from "oldmem" */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, size_t csize, + unsigned long offset) { void *src; int rc; @@ -223,10 +223,12 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, if (!csize) return 0; src = (void *) (pfn << PAGE_SHIFT) + offset; - if (userbuf) - rc = copy_oldmem_user((void __force __user *) buf, src, csize); + + /* XXX: pass the iov_iter down to a common function */ + if (iter_is_iovec(iter)) + rc = copy_oldmem_user(iter->iov->iov_base, src, csize); else - rc = copy_oldmem_kernel((void *) buf, src, csize); + rc = copy_oldmem_kernel(iter->kvec->iov_base, src, csize); return rc; } diff --git a/arch/sh/kernel/crash_dump.c b/arch/sh/kernel/crash_dump.c index 5b41b59698c1..26081861cb62 100644 --- a/arch/sh/kernel/crash_dump.c +++ b/arch/sh/kernel/crash_dump.c @@ -23,8 +23,8 @@ * Copy a page from "oldmem". For this page, there is no pte mapped * in the current kernel. We stitch up a pte, similar to kmap_atomic. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { void __iomem *vaddr; @@ -32,15 +32,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, return 0; vaddr = ioremap(pfn << PAGE_SHIFT, PAGE_SIZE); - - if (userbuf) { - if (copy_to_user((void __user *)buf, (vaddr + offset), csize)) { - iounmap(vaddr); - return -EFAULT; - } - } else - memcpy(buf, (vaddr + offset), csize); - + csize = copy_to_iter(vaddr + offset, csize, iter); iounmap(vaddr); + return csize; } diff --git a/arch/x86/kernel/crash_dump_32.c b/arch/x86/kernel/crash_dump_32.c index 5fcac46aaf6b..23d38ecf5de2 100644 --- a/arch/x86/kernel/crash_dump_32.c +++ b/arch/x86/kernel/crash_dump_32.c @@ -42,8 +42,8 @@ static inline bool is_crashed_pfn_valid(unsigned long pfn) * Copy a page from "oldmem". For this page, there might be no pte mapped * in the current kernel. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, size_t csize, + unsigned long offset) { void *vaddr; @@ -54,14 +54,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, return -EFAULT; vaddr = kmap_local_pfn(pfn); - - if (!userbuf) { - memcpy(buf, vaddr + offset, csize); - } else { - if (copy_to_user(buf, vaddr + offset, csize)) - csize = -EFAULT; - } - + csize = copy_to_iter(vaddr + offset, csize, iter); kunmap_local(vaddr); return csize; diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c index a7f617a3981d..8d91e5f2e14d 100644 --- a/arch/x86/kernel/crash_dump_64.c +++ b/arch/x86/kernel/crash_dump_64.c @@ -12,8 +12,8 @@ #include <linux/io.h> #include <linux/cc_platform.h> -static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf, +static ssize_t __copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset, bool encrypted) { void *vaddr; @@ -29,13 +29,7 @@ static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, if (!vaddr) return -ENOMEM; - if (userbuf) { - if (copy_to_user((void __user *)buf, vaddr + offset, csize)) { - iounmap((void __iomem *)vaddr); - return -EFAULT; - } - } else - memcpy(buf, vaddr + offset, csize); + csize = copy_to_iter(vaddr + offset, csize, iter); set_iounmap_nonlazy(); iounmap((void __iomem *)vaddr); @@ -55,10 +49,10 @@ static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, * Copy a page from the old kernel's memory. For this page, there is no pte * mapped in the current kernel. We stitch up a pte, similar to kmap_atomic. */ -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf) +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, size_t csize, + unsigned long offset) { - return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, false); + return __copy_oldmem_page(iter, pfn, csize, offset, false); } /** @@ -66,10 +60,10 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize, * memory with the encryption mask set to accommodate kdump on SME-enabled * machines. */ -ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf) +ssize_t copy_oldmem_page_encrypted(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset) { - return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true); + return __copy_oldmem_page(iter, pfn, csize, offset, true); } ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos) diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c index 509f85148fee..049bafe9c007 100644 --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -132,9 +132,8 @@ 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(char *buf, size_t count, - u64 *ppos, int userbuf, - bool encrypted) +ssize_t read_from_oldmem_iter(struct iov_iter *iter, size_t count, + u64 *ppos, bool encrypted) { unsigned long pfn, offset; size_t nr_bytes; @@ -155,29 +154,23 @@ ssize_t read_from_oldmem(char *buf, size_t count, /* If pfn is not ram, return zeros for sparse dump files */ if (!pfn_is_ram(pfn)) { - tmp = 0; - if (!userbuf) - memset(buf, 0, nr_bytes); - else if (clear_user(buf, nr_bytes)) - tmp = -EFAULT; + tmp = iov_iter_zero(nr_bytes, iter); } else { if (encrypted) - tmp = copy_oldmem_page_encrypted(pfn, buf, + tmp = copy_oldmem_page_encrypted(iter, pfn, nr_bytes, - offset, - userbuf); + offset); else - tmp = copy_oldmem_page(pfn, buf, nr_bytes, - offset, userbuf); + tmp = copy_oldmem_page(iter, pfn, nr_bytes, + offset); } - if (tmp < 0) { + if (tmp < nr_bytes) { up_read(&vmcore_cb_rwsem); - return tmp; + return -EFAULT; } *ppos += nr_bytes; count -= nr_bytes; - buf += nr_bytes; read += nr_bytes; ++pfn; offset = 0; @@ -187,6 +180,27 @@ ssize_t read_from_oldmem(char *buf, 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 */ @@ -231,11 +245,10 @@ int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, /* * Architectures which support memory encryption override this. */ -ssize_t __weak -copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize, - unsigned long offset, int userbuf) +ssize_t __weak copy_oldmem_page_encrypted(struct iov_iter *iter, + unsigned long pfn, size_t csize, unsigned long offset) { - return copy_oldmem_page(pfn, buf, csize, offset, userbuf); + return copy_oldmem_page(iter, pfn, csize, offset); } /* diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h index 620821549b23..a1cf7d5c03c7 100644 --- a/include/linux/crash_dump.h +++ b/include/linux/crash_dump.h @@ -24,11 +24,10 @@ extern int remap_oldmem_pfn_range(struct vm_area_struct *vma, unsigned long from, unsigned long pfn, unsigned long size, pgprot_t prot); -extern ssize_t copy_oldmem_page(unsigned long, char *, size_t, - unsigned long, int); -extern ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, - size_t csize, unsigned long offset, - int userbuf); +ssize_t copy_oldmem_page(struct iov_iter *i, unsigned long pfn, size_t csize, + unsigned long offset); +ssize_t copy_oldmem_page_encrypted(struct iov_iter *iter, unsigned long pfn, + size_t csize, unsigned long offset); void vmcore_cleanup(void); -- 2.33.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take an iov_iter 2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle) @ 2021-12-13 1:34 ` Matthew Wilcox 2021-12-13 2:33 ` kernel test robot 2021-12-13 7:57 ` Christoph Hellwig 2 siblings, 0 replies; 14+ messages in thread From: Matthew Wilcox @ 2021-12-13 1:34 UTC (permalink / raw) To: Baoquan He, Vivek Goyal, Dave Young, kexec Cc: Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel On Mon, Dec 13, 2021 at 12:06:34AM +0000, Matthew Wilcox (Oracle) wrote: > +++ b/arch/arm/kernel/crash_dump.c > @@ -27,9 +27,8 @@ > * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes > * copied or negative error in case of failure. > */ > -ssize_t copy_oldmem_page(unsigned long pfn, char *buf, > - size_t csize, unsigned long offset, > - int userbuf) > +ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, > + size_t csize, unsigned long offset) > { > void *vaddr; > I forgot to mention that I didn't adjust the kernel-doc. I'm tempted to remove it entirely, to be honest. It's not included into the rst files, and having the documentation repeated ten times across arch directories isn't useful. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take an iov_iter 2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle) 2021-12-13 1:34 ` Matthew Wilcox @ 2021-12-13 2:33 ` kernel test robot 2021-12-13 7:57 ` Christoph Hellwig 2 siblings, 0 replies; 14+ messages in thread From: kernel test robot @ 2021-12-13 2:33 UTC (permalink / raw) To: Matthew Wilcox (Oracle), Baoquan He, Vivek Goyal, Dave Young, kexec Cc: kbuild-all, Matthew Wilcox (Oracle), Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390 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/202112131020.pSJ77Cjj-lkp@intel.com/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/e1684ad72e2cce57ac90ae1270668753f1aa6c60 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 e1684ad72e2cce57ac90ae1270668753f1aa6c60 # 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 arch/riscv/kernel/ fs/proc/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): arch/riscv/kernel/crash_dump.c: In function 'copy_oldmem_page': >> arch/riscv/kernel/crash_dump.c:35:17: error: implicit declaration of function 'copy_to_iter'; did you mean 'copy_to_user'? [-Werror=implicit-function-declaration] 35 | csize = copy_to_iter(vaddr + offset, csize, iter); | ^~~~~~~~~~~~ | copy_to_user cc1: some warnings being treated as errors -- fs/proc/vmcore.c:135:9: warning: no previous prototype for 'read_from_oldmem_iter' [-Wmissing-prototypes] 135 | ssize_t read_from_oldmem_iter(struct iov_iter *iter, size_t count, | ^~~~~~~~~~~~~~~~~~~~~ fs/proc/vmcore.c: In function 'read_from_oldmem_iter': >> 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 'read_from_oldmem': >> fs/proc/vmcore.c:187:25: error: storage size of 'iter' isn't known 187 | struct iov_iter iter; | ^~~~ fs/proc/vmcore.c:188:22: error: storage size of 'iov' isn't known 188 | struct iovec iov; | ^~~ >> fs/proc/vmcore.c:189:21: error: storage size of 'kvec' isn't known 189 | struct kvec kvec; | ^~~~ fs/proc/vmcore.c:194:17: error: implicit declaration of function 'iov_iter_init'; did you mean 'klist_iter_init'? [-Werror=implicit-function-declaration] 194 | iov_iter_init(&iter, READ, &iov, 1, count); | ^~~~~~~~~~~~~ | klist_iter_init >> fs/proc/vmcore.c:198:17: error: implicit declaration of function 'iov_iter_kvec' [-Werror=implicit-function-declaration] 198 | iov_iter_kvec(&iter, READ, &kvec, 1, count); | ^~~~~~~~~~~~~ fs/proc/vmcore.c:189:21: warning: unused variable 'kvec' [-Wunused-variable] 189 | struct kvec kvec; | ^~~~ fs/proc/vmcore.c:188:22: warning: unused variable 'iov' [-Wunused-variable] 188 | struct iovec iov; | ^~~ fs/proc/vmcore.c:187:25: warning: unused variable 'iter' [-Wunused-variable] 187 | struct iov_iter iter; | ^~~~ fs/proc/vmcore.c:202:1: error: control reaches end of non-void function [-Werror=return-type] 202 | } | ^ cc1: some warnings being treated as errors vim +35 arch/riscv/kernel/crash_dump.c 10 11 /** 12 * copy_oldmem_page() - copy one page from old kernel memory 13 * @pfn: page frame number to be copied 14 * @buf: buffer where the copied page is placed 15 * @csize: number of bytes to copy 16 * @offset: offset in bytes into the page 17 * @userbuf: if set, @buf is in a user address space 18 * 19 * This function copies one page from old kernel memory into buffer pointed by 20 * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes 21 * copied or negative error in case of failure. 22 */ 23 ssize_t copy_oldmem_page(struct iov_iter *iter, unsigned long pfn, 24 size_t csize, unsigned long offset) 25 { 26 void *vaddr; 27 28 if (!csize) 29 return 0; 30 31 vaddr = memremap(__pfn_to_phys(pfn), PAGE_SIZE, MEMREMAP_WB); 32 if (!vaddr) 33 return -ENOMEM; 34 > 35 csize = copy_to_iter(vaddr + offset, csize, iter); --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take an iov_iter 2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle) 2021-12-13 1:34 ` Matthew Wilcox 2021-12-13 2:33 ` kernel test robot @ 2021-12-13 7:57 ` Christoph Hellwig 2021-12-16 9:28 ` Heiko Carstens 2 siblings, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2021-12-13 7:57 UTC (permalink / raw) To: Matthew Wilcox (Oracle) Cc: Baoquan He, Vivek Goyal, Dave Young, kexec, Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel On Mon, Dec 13, 2021 at 12:06:34AM +0000, Matthew Wilcox (Oracle) wrote: > Instead of passing in a 'buf' and 'userbuf' argument, pass in an iov_iter. > s390 needs more work to pass the iov_iter down further, or refactor, > but I'd be more comfortable if someone who can test on s390 did that work. > > It's more convenient to convert the whole of read_from_oldmem() to > take an iov_iter at the same time, so rename it to read_from_oldmem_iter() > and add a temporary read_from_oldmem() wrapper that creates an iov_iter. This looks pretty reasonable. s390 could use some love from people that know the code, and yes, the kerneldoc comments should go away. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take an iov_iter 2021-12-13 7:57 ` Christoph Hellwig @ 2021-12-16 9:28 ` Heiko Carstens 0 siblings, 0 replies; 14+ messages in thread From: Heiko Carstens @ 2021-12-16 9:28 UTC (permalink / raw) To: Christoph Hellwig Cc: Matthew Wilcox (Oracle), Baoquan He, Vivek Goyal, Dave Young, kexec, Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, linux-s390, linux-fsdevel, Vasily Gorbik, Alexander Gordeev On Mon, Dec 13, 2021 at 08:57:25AM +0100, Christoph Hellwig wrote: > On Mon, Dec 13, 2021 at 12:06:34AM +0000, Matthew Wilcox (Oracle) wrote: > > Instead of passing in a 'buf' and 'userbuf' argument, pass in an iov_iter. > > s390 needs more work to pass the iov_iter down further, or refactor, > > but I'd be more comfortable if someone who can test on s390 did that work. > > > > It's more convenient to convert the whole of read_from_oldmem() to > > take an iov_iter at the same time, so rename it to read_from_oldmem_iter() > > and add a temporary read_from_oldmem() wrapper that creates an iov_iter. > > This looks pretty reasonable. s390 could use some love from people that > know the code, and yes, the kerneldoc comments should go away. Sure, we will take care of this. Added to ToDo list. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] vmcore: Convert __read_vmcore to use an iov_iter 2021-12-13 0:06 [PATCH 0/3] Convert vmcore to use an iov_iter Matthew Wilcox (Oracle) 2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle) @ 2021-12-13 0:06 ` Matthew Wilcox (Oracle) 2021-12-13 3:24 ` kernel test robot 2021-12-13 8:00 ` Christoph Hellwig 2021-12-13 0:06 ` [PATCH 3/3] vmcore: Convert read_from_oldmem() to take " Matthew Wilcox (Oracle) 2 siblings, 2 replies; 14+ messages in thread From: Matthew Wilcox (Oracle) @ 2021-12-13 0:06 UTC (permalink / raw) To: Baoquan He, Vivek Goyal, Dave Young, kexec Cc: Matthew Wilcox (Oracle), Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel This gets rid of copy_to() and let us use proc_read_iter() instead of proc_read(). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/proc/vmcore.c | 83 ++++++++++++++++++------------------------------ 1 file changed, 31 insertions(+), 52 deletions(-) diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c index 049bafe9c007..443cbaf16ec8 100644 --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -251,22 +251,8 @@ ssize_t __weak copy_oldmem_page_encrypted(struct iov_iter *iter, return copy_oldmem_page(iter, pfn, csize, offset); } -/* - * Copy to either kernel or user space - */ -static int copy_to(void *target, void *src, size_t size, int userbuf) -{ - if (userbuf) { - if (copy_to_user((char __user *) target, src, size)) - return -EFAULT; - } else { - memcpy(target, src, size); - } - return 0; -} - #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP -static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf) +static int vmcoredd_copy_dumps(struct iov_iter *iter, u64 start, size_t size) { struct vmcoredd_node *dump; u64 offset = 0; @@ -279,14 +265,13 @@ static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf) if (start < offset + dump->size) { tsz = min(offset + (u64)dump->size - start, (u64)size); buf = dump->buf + start - offset; - if (copy_to(dst, buf, tsz, userbuf)) { + if (copy_to_iter(buf, tsz, iter) < tsz) { ret = -EFAULT; goto out_unlock; } size -= tsz; start += tsz; - dst += tsz; /* Leave now if buffer filled already */ if (!size) @@ -342,33 +327,30 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct *vma, unsigned long dst, /* Read from the ELF header and then the crash dump. On error, negative value is * returned otherwise number of bytes read are returned. */ -static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, - int userbuf) +static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos) { ssize_t acc = 0, tmp; size_t tsz; u64 start; struct vmcore *m = NULL; - if (buflen == 0 || *fpos >= vmcore_size) + if (iter->count == 0 || *fpos >= vmcore_size) return 0; - /* trim buflen to not go beyond EOF */ - if (buflen > vmcore_size - *fpos) - buflen = vmcore_size - *fpos; + /* trim iter to not go beyond EOF */ + if (iter->count > vmcore_size - *fpos) + iter->count = vmcore_size - *fpos; /* Read ELF core header */ if (*fpos < elfcorebuf_sz) { - tsz = min(elfcorebuf_sz - (size_t)*fpos, buflen); - if (copy_to(buffer, elfcorebuf + *fpos, tsz, userbuf)) + tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); + if (copy_to_iter(elfcorebuf + *fpos, tsz, iter) < tsz) return -EFAULT; - buflen -= tsz; *fpos += tsz; - buffer += tsz; acc += tsz; /* leave now if filled buffer already */ - if (buflen == 0) + if (iter->count == 0) return acc; } @@ -389,35 +371,31 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, /* Read device dumps */ if (*fpos < elfcorebuf_sz + vmcoredd_orig_sz) { tsz = min(elfcorebuf_sz + vmcoredd_orig_sz - - (size_t)*fpos, buflen); + (size_t)*fpos, iter->count); start = *fpos - elfcorebuf_sz; - if (vmcoredd_copy_dumps(buffer, start, tsz, userbuf)) + if (vmcoredd_copy_dumps(iter, start, tsz)) return -EFAULT; - buflen -= tsz; *fpos += tsz; - buffer += tsz; acc += tsz; /* leave now if filled buffer already */ - if (!buflen) + if (!iter->count) return acc; } #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */ /* Read remaining elf notes */ - tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, buflen); + tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); kaddr = elfnotes_buf + *fpos - elfcorebuf_sz - vmcoredd_orig_sz; - if (copy_to(buffer, kaddr, tsz, userbuf)) + if (copy_to_iter(kaddr, tsz, iter) < tsz) return -EFAULT; - buflen -= tsz; *fpos += tsz; - buffer += tsz; acc += tsz; /* leave now if filled buffer already */ - if (buflen == 0) + if (iter->count == 0) return acc; } @@ -425,19 +403,17 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, if (*fpos < m->offset + m->size) { tsz = (size_t)min_t(unsigned long long, m->offset + m->size - *fpos, - buflen); + iter->count); start = m->paddr + *fpos - m->offset; - tmp = read_from_oldmem(buffer, tsz, &start, - userbuf, cc_platform_has(CC_ATTR_MEM_ENCRYPT)); + tmp = read_from_oldmem_iter(iter, tsz, &start, + cc_platform_has(CC_ATTR_MEM_ENCRYPT)); if (tmp < 0) return tmp; - buflen -= tsz; *fpos += tsz; - buffer += tsz; acc += tsz; /* leave now if filled buffer already */ - if (buflen == 0) + if (iter->count == 0) return acc; } } @@ -445,15 +421,14 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos, return acc; } -static ssize_t read_vmcore(struct file *file, char __user *buffer, - size_t buflen, loff_t *fpos) +static ssize_t read_vmcore(struct kiocb *iocb, struct iov_iter *iter) { - return __read_vmcore((__force char *) buffer, buflen, fpos, 1); + return __read_vmcore(iter, &iocb->ki_pos); } /* * The vmcore fault handler uses the page cache and fills data using the - * standard __vmcore_read() function. + * standard __read_vmcore() function. * * On s390 the fault handler is used for memory regions that can't be mapped * directly with remap_pfn_range(). @@ -463,9 +438,10 @@ static vm_fault_t mmap_vmcore_fault(struct vm_fault *vmf) #ifdef CONFIG_S390 struct address_space *mapping = vmf->vma->vm_file->f_mapping; pgoff_t index = vmf->pgoff; + struct iov_iter iter; + struct kvec kvec; struct page *page; loff_t offset; - char *buf; int rc; page = find_or_create_page(mapping, index, GFP_KERNEL); @@ -473,8 +449,11 @@ static vm_fault_t mmap_vmcore_fault(struct vm_fault *vmf) return VM_FAULT_OOM; if (!PageUptodate(page)) { offset = (loff_t) index << PAGE_SHIFT; - buf = __va((page_to_pfn(page) << PAGE_SHIFT)); - rc = __read_vmcore(buf, PAGE_SIZE, &offset, 0); + kvec.iov_base = page_address(page); + kvec.iov_len = PAGE_SIZE; + iov_iter_kvec(&iter, READ, &kvec, 1, PAGE_SIZE); + + rc = __read_vmcore(&iter, &offset); if (rc < 0) { unlock_page(page); put_page(page); @@ -724,7 +703,7 @@ static int mmap_vmcore(struct file *file, struct vm_area_struct *vma) static const struct proc_ops vmcore_proc_ops = { .proc_open = open_vmcore, - .proc_read = read_vmcore, + .proc_read_iter = read_vmcore, .proc_lseek = default_llseek, .proc_mmap = mmap_vmcore, }; -- 2.33.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] vmcore: Convert __read_vmcore to use an iov_iter 2021-12-13 0:06 ` [PATCH 2/3] vmcore: Convert __read_vmcore to use " Matthew Wilcox (Oracle) @ 2021-12-13 3:24 ` kernel test robot 2021-12-13 8:00 ` Christoph Hellwig 1 sibling, 0 replies; 14+ messages in thread From: kernel test robot @ 2021-12-13 3:24 UTC (permalink / raw) To: Matthew Wilcox (Oracle), Baoquan He, Vivek Goyal, Dave Young, kexec Cc: kbuild-all, Matthew Wilcox (Oracle), Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390 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/202112131103.3ExA0BMN-lkp@intel.com/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/687563a8e516282784ed87ca7ed3eca900b42192 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 687563a8e516282784ed87ca7ed3eca900b42192 # 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 <lkp@intel.com> All errors (new ones prefixed by >>): fs/proc/vmcore.c:135:9: warning: no previous prototype for 'read_from_oldmem_iter' [-Wmissing-prototypes] 135 | ssize_t read_from_oldmem_iter(struct iov_iter *iter, size_t count, | ^~~~~~~~~~~~~~~~~~~~~ fs/proc/vmcore.c: In function 'read_from_oldmem_iter': 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 'read_from_oldmem': fs/proc/vmcore.c:187:25: error: storage size of 'iter' isn't known 187 | struct iov_iter iter; | ^~~~ fs/proc/vmcore.c:188:22: error: storage size of 'iov' isn't known 188 | struct iovec iov; | ^~~ fs/proc/vmcore.c:189:21: error: storage size of 'kvec' isn't known 189 | struct kvec kvec; | ^~~~ fs/proc/vmcore.c:194:17: error: implicit declaration of function 'iov_iter_init'; did you mean 'klist_iter_init'? [-Werror=implicit-function-declaration] 194 | iov_iter_init(&iter, READ, &iov, 1, count); | ^~~~~~~~~~~~~ | klist_iter_init fs/proc/vmcore.c:198:17: error: implicit declaration of function 'iov_iter_kvec' [-Werror=implicit-function-declaration] 198 | iov_iter_kvec(&iter, READ, &kvec, 1, count); | ^~~~~~~~~~~~~ fs/proc/vmcore.c:189:21: warning: unused variable 'kvec' [-Wunused-variable] 189 | struct kvec kvec; | ^~~~ fs/proc/vmcore.c:188:22: warning: unused variable 'iov' [-Wunused-variable] 188 | struct iovec iov; | ^~~ fs/proc/vmcore.c:187:25: warning: unused variable 'iter' [-Wunused-variable] 187 | struct iov_iter iter; | ^~~~ fs/proc/vmcore.c: In function '__read_vmcore': >> fs/proc/vmcore.c:337:17: error: invalid use of undefined type 'struct iov_iter' 337 | if (iter->count == 0 || *fpos >= vmcore_size) | ^~ fs/proc/vmcore.c:341:17: error: invalid use of undefined type 'struct iov_iter' 341 | if (iter->count > vmcore_size - *fpos) | ^~ fs/proc/vmcore.c:342:21: error: invalid use of undefined type 'struct iov_iter' 342 | 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:346:62: error: invalid use of undefined type 'struct iov_iter' 346 | 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:346:23: note: in expansion of macro 'min' 346 | 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:346:62: error: invalid use of undefined type 'struct iov_iter' 346 | 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), \ | ^~~~~~~~~~ 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:346:23: note: in expansion of macro 'min' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~~ 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:346:62: error: invalid use of undefined type 'struct iov_iter' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:28:34: note: in definition of macro '__cmp' 28 | #define __cmp(x, y, op) ((x) op (y) ? (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:346:23: note: in expansion of macro 'min' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~~ fs/proc/vmcore.c:346:62: error: invalid use of undefined type 'struct iov_iter' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:28:46: note: in definition of macro '__cmp' 28 | #define __cmp(x, y, op) ((x) op (y) ? (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:346:23: note: in expansion of macro 'min' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~~ fs/proc/vmcore.c:346:62: error: invalid use of undefined type 'struct iov_iter' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:32:24: note: in definition of macro '__cmp_once' 32 | typeof(y) unique_y = (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:346:23: note: in expansion of macro 'min' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~~ fs/proc/vmcore.c:346:62: error: invalid use of undefined type 'struct iov_iter' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:32:39: note: in definition of macro '__cmp_once' 32 | typeof(y) unique_y = (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:346:23: note: in expansion of macro 'min' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~~ include/linux/minmax.h:36:9: error: first argument to '__builtin_choose_expr' not a constant 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:346:23: note: in expansion of macro 'min' 346 | tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); | ^~~ >> fs/proc/vmcore.c:347:21: error: implicit declaration of function 'copy_to_iter'; did you mean 'copy_to_user'? [-Werror=implicit-function-declaration] 347 | if (copy_to_iter(elfcorebuf + *fpos, tsz, iter) < tsz) | ^~~~~~~~~~~~ | copy_to_user fs/proc/vmcore.c:353:25: error: invalid use of undefined type 'struct iov_iter' 353 | if (iter->count == 0) | ^~ 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:389:76: error: invalid use of undefined type 'struct iov_iter' 389 | tsz = min(elfcorebuf_sz + elfnotes_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:389:23: note: in expansion of macro 'min' 389 | tsz = min(elfcorebuf_sz + elfnotes_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:389:76: error: invalid use of undefined type 'struct iov_iter' 389 | tsz = min(elfcorebuf_sz + elfnotes_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), \ | ^~~~~~~~~~ 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:389:23: note: in expansion of macro 'min' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~~ 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:389:76: error: invalid use of undefined type 'struct iov_iter' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:28:34: note: in definition of macro '__cmp' 28 | #define __cmp(x, y, op) ((x) op (y) ? (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:389:23: note: in expansion of macro 'min' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~~ fs/proc/vmcore.c:389:76: error: invalid use of undefined type 'struct iov_iter' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:28:46: note: in definition of macro '__cmp' 28 | #define __cmp(x, y, op) ((x) op (y) ? (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:389:23: note: in expansion of macro 'min' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~~ fs/proc/vmcore.c:389:76: error: invalid use of undefined type 'struct iov_iter' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~ include/linux/minmax.h:32:24: note: in definition of macro '__cmp_once' 32 | typeof(y) unique_y = (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:389:23: note: in expansion of macro 'min' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); | ^~~ fs/proc/vmcore.c:389:76: error: invalid use of undefined type 'struct iov_iter' 389 | tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); vim +337 fs/proc/vmcore.c 326 327 /* Read from the ELF header and then the crash dump. On error, negative value is 328 * returned otherwise number of bytes read are returned. 329 */ 330 static ssize_t __read_vmcore(struct iov_iter *iter, loff_t *fpos) 331 { 332 ssize_t acc = 0, tmp; 333 size_t tsz; 334 u64 start; 335 struct vmcore *m = NULL; 336 > 337 if (iter->count == 0 || *fpos >= vmcore_size) 338 return 0; 339 340 /* trim iter to not go beyond EOF */ 341 if (iter->count > vmcore_size - *fpos) 342 iter->count = vmcore_size - *fpos; 343 344 /* Read ELF core header */ 345 if (*fpos < elfcorebuf_sz) { 346 tsz = min(elfcorebuf_sz - (size_t)*fpos, iter->count); > 347 if (copy_to_iter(elfcorebuf + *fpos, tsz, iter) < tsz) 348 return -EFAULT; 349 *fpos += tsz; 350 acc += tsz; 351 352 /* leave now if filled buffer already */ 353 if (iter->count == 0) 354 return acc; 355 } 356 357 /* Read Elf note segment */ 358 if (*fpos < elfcorebuf_sz + elfnotes_sz) { 359 void *kaddr; 360 361 /* We add device dumps before other elf notes because the 362 * other elf notes may not fill the elf notes buffer 363 * completely and we will end up with zero-filled data 364 * between the elf notes and the device dumps. Tools will 365 * then try to decode this zero-filled data as valid notes 366 * and we don't want that. Hence, adding device dumps before 367 * the other elf notes ensure that zero-filled data can be 368 * avoided. 369 */ 370 #ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP 371 /* Read device dumps */ 372 if (*fpos < elfcorebuf_sz + vmcoredd_orig_sz) { 373 tsz = min(elfcorebuf_sz + vmcoredd_orig_sz - 374 (size_t)*fpos, iter->count); 375 start = *fpos - elfcorebuf_sz; 376 if (vmcoredd_copy_dumps(iter, start, tsz)) 377 return -EFAULT; 378 379 *fpos += tsz; 380 acc += tsz; 381 382 /* leave now if filled buffer already */ 383 if (!iter->count) 384 return acc; 385 } 386 #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */ 387 388 /* Read remaining elf notes */ 389 tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, iter->count); 390 kaddr = elfnotes_buf + *fpos - elfcorebuf_sz - vmcoredd_orig_sz; 391 if (copy_to_iter(kaddr, tsz, iter) < tsz) 392 return -EFAULT; 393 394 *fpos += tsz; 395 acc += tsz; 396 397 /* leave now if filled buffer already */ 398 if (iter->count == 0) 399 return acc; 400 } 401 402 list_for_each_entry(m, &vmcore_list, list) { 403 if (*fpos < m->offset + m->size) { 404 tsz = (size_t)min_t(unsigned long long, 405 m->offset + m->size - *fpos, 406 iter->count); 407 start = m->paddr + *fpos - m->offset; 408 tmp = read_from_oldmem_iter(iter, tsz, &start, 409 cc_platform_has(CC_ATTR_MEM_ENCRYPT)); 410 if (tmp < 0) 411 return tmp; 412 *fpos += tsz; 413 acc += tsz; 414 415 /* leave now if filled buffer already */ 416 if (iter->count == 0) 417 return acc; 418 } 419 } 420 421 return acc; 422 } 423 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] vmcore: Convert __read_vmcore to use an iov_iter 2021-12-13 0:06 ` [PATCH 2/3] vmcore: Convert __read_vmcore to use " Matthew Wilcox (Oracle) 2021-12-13 3:24 ` kernel test robot @ 2021-12-13 8:00 ` Christoph Hellwig 1 sibling, 0 replies; 14+ messages in thread From: Christoph Hellwig @ 2021-12-13 8:00 UTC (permalink / raw) To: Matthew Wilcox (Oracle) Cc: Baoquan He, Vivek Goyal, Dave Young, kexec, Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel On Mon, Dec 13, 2021 at 12:06:35AM +0000, Matthew Wilcox (Oracle) wrote: > + /* trim iter to not go beyond EOF */ > + if (iter->count > vmcore_size - *fpos) > + iter->count = vmcore_size - *fpos; Nit: iov_iter_truncate() Otherwise this looks good from a cursory view. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] vmcore: Convert read_from_oldmem() to take an iov_iter 2021-12-13 0:06 [PATCH 0/3] Convert vmcore to use an iov_iter Matthew Wilcox (Oracle) 2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle) 2021-12-13 0:06 ` [PATCH 2/3] vmcore: Convert __read_vmcore to use " Matthew Wilcox (Oracle) @ 2021-12-13 0:06 ` Matthew Wilcox (Oracle) 2021-12-13 4:04 ` kernel test robot 2021-12-13 8:02 ` Christoph Hellwig 2 siblings, 2 replies; 14+ messages in thread From: Matthew Wilcox (Oracle) @ 2021-12-13 0:06 UTC (permalink / raw) To: Baoquan He, Vivek Goyal, Dave Young, kexec Cc: Matthew Wilcox (Oracle), Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel 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) <willy@infradead.org> --- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] vmcore: Convert read_from_oldmem() to take an iov_iter 2021-12-13 0:06 ` [PATCH 3/3] vmcore: Convert read_from_oldmem() to take " Matthew Wilcox (Oracle) @ 2021-12-13 4:04 ` kernel test robot 2021-12-13 8:02 ` Christoph Hellwig 1 sibling, 0 replies; 14+ messages in thread From: kernel test robot @ 2021-12-13 4:04 UTC (permalink / raw) To: Matthew Wilcox (Oracle), Baoquan He, Vivek Goyal, Dave Young, kexec Cc: kbuild-all, Matthew Wilcox (Oracle), Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390 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/202112131249.lfVULc7X-lkp@intel.com/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 <lkp@intel.com> 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/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] vmcore: Convert read_from_oldmem() to take an iov_iter 2021-12-13 0:06 ` [PATCH 3/3] vmcore: Convert read_from_oldmem() to take " Matthew Wilcox (Oracle) 2021-12-13 4:04 ` kernel test robot @ 2021-12-13 8:02 ` Christoph Hellwig 2021-12-13 9:29 ` Baoquan He 2021-12-13 13:42 ` Matthew Wilcox 1 sibling, 2 replies; 14+ messages in thread From: Christoph Hellwig @ 2021-12-13 8:02 UTC (permalink / raw) To: Matthew Wilcox (Oracle) Cc: Baoquan He, Vivek Goyal, Dave Young, kexec, Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, Christoph Hellwig, linux-s390, linux-fsdevel > > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] vmcore: Convert read_from_oldmem() to take an iov_iter 2021-12-13 8:02 ` Christoph Hellwig @ 2021-12-13 9:29 ` Baoquan He 2021-12-13 13:42 ` Matthew Wilcox 1 sibling, 0 replies; 14+ messages in thread From: Baoquan He @ 2021-12-13 9:29 UTC (permalink / raw) To: Christoph Hellwig Cc: Matthew Wilcox (Oracle), Vivek Goyal, Dave Young, kexec, Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, linux-s390, linux-fsdevel 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/20190718032858.28744-1-bauerman@linux.ibm.com/ 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] vmcore: Convert read_from_oldmem() to take an iov_iter 2021-12-13 8:02 ` Christoph Hellwig 2021-12-13 9:29 ` Baoquan He @ 2021-12-13 13:42 ` Matthew Wilcox 1 sibling, 0 replies; 14+ messages in thread From: Matthew Wilcox @ 2021-12-13 13:42 UTC (permalink / raw) To: Christoph Hellwig Cc: Baoquan He, Vivek Goyal, Dave Young, kexec, Tiezhu Yang, linux-kernel, Amit Daniel Kachhap, linux-s390, linux-fsdevel 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 ;-) ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-12-16 9:29 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-13 0:06 [PATCH 0/3] Convert vmcore to use an iov_iter Matthew Wilcox (Oracle) 2021-12-13 0:06 ` [PATCH 1/3] vmcore: Convert copy_oldmem_page() to take " Matthew Wilcox (Oracle) 2021-12-13 1:34 ` Matthew Wilcox 2021-12-13 2:33 ` kernel test robot 2021-12-13 7:57 ` Christoph Hellwig 2021-12-16 9:28 ` Heiko Carstens 2021-12-13 0:06 ` [PATCH 2/3] vmcore: Convert __read_vmcore to use " Matthew Wilcox (Oracle) 2021-12-13 3:24 ` kernel test robot 2021-12-13 8:00 ` Christoph Hellwig 2021-12-13 0:06 ` [PATCH 3/3] vmcore: Convert read_from_oldmem() to take " Matthew Wilcox (Oracle) 2021-12-13 4:04 ` kernel test robot 2021-12-13 8:02 ` Christoph Hellwig 2021-12-13 9:29 ` Baoquan He 2021-12-13 13:42 ` Matthew Wilcox
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).