Re: [PATCH] powerpc/sstep: Fix array out of bound warning
On 1/28/21 10:50 PM, Naveen N. Rao wrote:
On 2021/01/15 11:46AM, Ravi Bangoria wrote:
Compiling kernel with -Warray-bounds throws below warning:
In function 'emulate_vsx_store':
warning: array subscript is above array bounds [-Warray-bounds]
buf.d[2] = byterev_8(reg->d[1]);
~^~~
buf.d[3] = byterev_8(reg->d[0]);
~^~~
Fix it by converting local variable 'union vsx_reg buf' into an array.
Also consider function argument 'union vsx_reg *reg' as array instead
of pointer because callers are actually passing an array to it.
I think you should change the function prototype to reflect this.
However, while I agree with this change in principle, it looks to be a
lot of code churn for a fairly narrow use. Perhaps we should just
address the specific bug. Something like the below (not tested)?
Yes, this would serve the same purpose and it's more compact as well. Sent v2.
@@ -818,13 +818,15 @@ void emulate_vsx_store(struct instruction_op *op, const
union vsx_reg *reg,
break;
if (rev) {
/* reverse 32 bytes */
- buf.d[0] = byterev_8(reg->d[3]);
- buf.d[1] = byterev_8(reg->d[2]);
- buf.d[2] = byterev_8(reg->d[1]);
- buf.d[3] = byterev_8(reg->d[0]);
- reg =
+ union vsx_reg buf32[2];
+ buf32[0].d[0] = byterev_8(reg[1].d[1]);
+ buf32[0].d[1] = byterev_8(reg[1].d[0]);
+ buf32[1].d[0] = byterev_8(reg[0].d[1]);
+ buf32[1].d[1] = byterev_8(reg[0].d[0]);
+ memcpy(mem, buf32, size);
+ } else {
+ memcpy(mem, reg, size);
}
- memcpy(mem, reg, size);
break;
case 16:
/* stxv, stxvx, stxvl, stxvll */
- Naveen
[PATCH v2] powerpc/sstep: Fix array out of bound warning
Compiling kernel with -Warray-bounds throws below warning:
In function 'emulate_vsx_store':
warning: array subscript is above array bounds [-Warray-bounds]
buf.d[2] = byterev_8(reg->d[1]);
~^~~
buf.d[3] = byterev_8(reg->d[0]);
~^~~
Fix it by using temporary array variable 'union vsx_reg buf32[]' in
that code block. Also, with element_size = 32, 'union vsx_reg *reg'
is an array of size 2. So, use 'reg' as an array instead of pointer
in the same code block.
Fixes: af99da74333b ("powerpc/sstep: Support VSX vector paired storage access
instructions")
Suggested-by: Naveen N. Rao
Signed-off-by: Ravi Bangoria
---
v1: http://lore.kernel.org/r/20210115061620.692500-1-ravi.bango...@linux.ibm.com
v1->v2:
- Change code only in the affected block
arch/powerpc/lib/sstep.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index bf7a7d62ae8b..ede093e96234 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -818,13 +818,15 @@ void emulate_vsx_store(struct instruction_op *op, const
union vsx_reg *reg,
break;
if (rev) {
/* reverse 32 bytes */
- buf.d[0] = byterev_8(reg->d[3]);
- buf.d[1] = byterev_8(reg->d[2]);
- buf.d[2] = byterev_8(reg->d[1]);
- buf.d[3] = byterev_8(reg->d[0]);
- reg =
+ union vsx_reg buf32[2];
+ buf32[0].d[0] = byterev_8(reg[1].d[1]);
+ buf32[0].d[1] = byterev_8(reg[1].d[0]);
+ buf32[1].d[0] = byterev_8(reg[0].d[1]);
+ buf32[1].d[1] = byterev_8(reg[0].d[0]);
+ memcpy(mem, buf32, size);
+ } else {
+ memcpy(mem, reg, size);
}
- memcpy(mem, reg, size);
break;
case 16:
/* stxv, stxvx, stxvl, stxvll */
--
2.26.2
Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
On Thu, Jan 28, 2021 at 03:44:21PM +0100, Christophe Leroy wrote: > > > Le 28/01/2021 à 15:42, Jens Axboe a écrit : > > On 1/28/21 6:52 AM, Zorro Lang wrote: > > > On Wed, Jan 27, 2021 at 08:06:37PM -0700, Jens Axboe wrote: > > > > On 1/27/21 8:13 PM, Zorro Lang wrote: > > > > > On Thu, Jan 28, 2021 at 10:18:07AM +1000, Nicholas Piggin wrote: > > > > > > Excerpts from Jens Axboe's message of January 28, 2021 5:29 am: > > > > > > > On 1/27/21 9:38 AM, Christophe Leroy wrote: > > > > > > > > > > > > > > > > > > > > > > > > Le 27/01/2021 à 15:56, Zorro Lang a écrit : > > > > > > > > > On powerpc, io_uring test hit below KUAP fault on > > > > > > > > > __do_page_fault. > > > > > > > > > The fail source line is: > > > > > > > > > > > > > > > > > > if (unlikely(!is_user && bad_kernel_fault(regs, > > > > > > > > > error_code, address, is_write))) > > > > > > > > > return SIGSEGV; > > > > > > > > > > > > > > > > > > The is_user() is based on user_mod(regs) only. This's not > > > > > > > > > suit for > > > > > > > > > io_uring, where the helper thread can assume the user app > > > > > > > > > identity > > > > > > > > > and could perform this fault just fine. So turn to use mm to > > > > > > > > > decide > > > > > > > > > if this is valid or not. > > > > > > > > > > > > > > > > I don't understand why testing is_user would be an issue. KUAP > > > > > > > > purpose > > > > > > > > it to block any unallowed access from kernel to user memory > > > > > > > > (Equivalent to SMAP on x86). So it really must be based on > > > > > > > > MSR_PR bit, > > > > > > > > that is what is_user provides. > > > > > > > > > > > > > > > > If the kernel access is legitimate, kernel should have opened > > > > > > > > userspace access then you shouldn't get this "Bug: Read fault > > > > > > > > blocked > > > > > > > > by KUAP!". > > > > > > > > > > > > > > > > As far as I understand, the fault occurs in > > > > > > > > iov_iter_fault_in_readable() which calls > > > > > > > > fault_in_pages_readable() And > > > > > > > > fault_in_pages_readable() uses __get_user() so it is a > > > > > > > > legitimate > > > > > > > > access and you really should get a KUAP fault. > > > > > > > > > > > > > > > > So the problem is somewhere else, I think you proposed patch > > > > > > > > just > > > > > > > > hides the problem, it doesn't fix it. > > > > > > > > > > > > > > If we do kthread_use_mm(), can we agree that the user access is > > > > > > > valid? > > > > > > > > > > > > Yeah the io uring code is fine, provided it uses the uaccess > > > > > > primitives > > > > > > like any other kernel code. It's looking more like a an > > > > > > arch/powerpc bug. > > > > > > > > > > > > > We should be able to copy to/from user space, and including > > > > > > > faults, if > > > > > > > that's been done and the new mm assigned. Because it really > > > > > > > should be. > > > > > > > If SMAP was a problem on x86, we would have seen it long ago. > > > > > > > > > > > > > > I'm assuming this may be breakage related to the recent uaccess > > > > > > > changes > > > > > > > related to set_fs and friends? Or maybe recent changes on the > > > > > > > powerpc > > > > > > > side? > > > > > > > > > > > > > > Zorro, did 5.10 work? > > > > > > > > > > > > Would be interesting to know. > > > > > > > > > > Sure Nick and Jens, which 5.10 rc? version do you want to know ? Or > > > > > any git > > > > > commit(be the HEAD) in 5.10 phase? > > > > > > > > I forget which versions had what series of this, but 5.10 final - and if > > > > that fails, then 5.9 final. IIRC, 5.9 was pre any of these changes, and > > > > 5.10 definitely has them. > > > > > > I justed built linux v5.10 with same .config file, and gave it same test. > > > v5.10 (HEAD=2c85ebc57b Linux 5.10) can't reproduce this bug: > > > > > > # ./check generic/013 generic/051 > > > FSTYP -- xfs (non-debug) > > > PLATFORM -- Linux/ppc64le ibm-p9z-xxx- 5.10.0 #3 SMP Thu Jan 28 > > > 04:12:14 EST 2021 > > > MKFS_OPTIONS -- -f -m > > > crc=1,finobt=1,reflink=1,rmapbt=1,bigtime=1,inobtcount=1 /dev/sda3 > > > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 > > > /mnt/xfstests/scratch > > > > > > generic/013 138s ... 77s > > > generic/051 103s ... 143s > > > Ran: generic/013 generic/051 > > > Passed all 2 tests > > > > Thanks for testing that, so I think it's safe to conclude that there's a > > regression in powerpc fault handling for kthreads that use > > kthread_use_mm in this release. A bisect would definitely find it, but > > might be pointless if Christophe or Nick already have an idea of what it > > is. > > > > I don't have any idea yet, but I'd be curious to see the vmlinux binary > matching the reported Oops. I just upload the vmlinux and .config file, the vmlinux it too big, I have to upload it to my google store and share the link as below: config file:
Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)
On Wed, Jan 20, 2021 at 06:57:24PM +, Christophe Leroy wrote:
> Talitos Security Engine AESU considers any input
> data size that is not a multiple of 16 bytes to be an error.
> This is not a problem in general, except for Counter mode
> that is a stream cipher and can have an input of any size.
>
> Test Manager for ctr(aes) fails on 4th test vector which has
> a length of 499 while all previous vectors which have a 16 bytes
> multiple length succeed.
>
> As suggested by Freescale, round up the input data length to the
> nearest 16 bytes.
>
> Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
> Signed-off-by: Christophe Leroy
> ---
> drivers/crypto/talitos.c | 28
> drivers/crypto/talitos.h | 1 +
> 2 files changed, 17 insertions(+), 12 deletions(-)
All applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 04/13] module: use RCU to synchronize find_module
On Thu, Jan 28, 2021 at 05:50:56PM -0300, Thiago Jung Bauermann wrote:
> > struct module *find_module(const char *name)
> > {
> > - module_assert_mutex();
>
> Does it make sense to replace the assert above with the warn below (untested)?
>
> RCU_LOCKDEP_WARN(rcu_read_lock_sched_held());
One caller actually holds module_mutex still. And find_module_all,
which implements the actual logic already asserts that either
module_mutex is held or rcu_read_lock, so I don't tink we need an
extra one here.
[PATCH net v2] ibmvnic: device remove has higher precedence over reset
Returning -EBUSY in ibmvnic_remove() does not actually hold the
removal procedure since driver core doesn't care for the return
value (see __device_release_driver() in drivers/base/dd.c
calling dev->bus->remove()) though vio_bus_remove
(in arch/powerpc/platforms/pseries/vio.c) records the
return value and passes it on. [1]
During the device removal precedure, checking for resetting
bit is dropped so that we can continue executing all the
cleanup calls in the rest of the remove function. Otherwise,
it can cause latent memory leaks and kernel crashes.
[1]
https://lore.kernel.org/linuxppc-dev/20210117101242.dpwayq6wdgfdz...@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53
Reported-by: Uwe Kleine-König
Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during device
reset")
Signed-off-by: Lijun Pan
---
v2: drop v1's deletion of REMOVING check in __ibmvnic_reset.
drivers/net/ethernet/ibm/ibmvnic.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 9778c83150f1..e19fa8bc763c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -5438,11 +5438,6 @@ static int ibmvnic_remove(struct vio_dev *dev)
unsigned long flags;
spin_lock_irqsave(>state_lock, flags);
- if (test_bit(0, >resetting)) {
- spin_unlock_irqrestore(>state_lock, flags);
- return -EBUSY;
- }
-
adapter->state = VNIC_REMOVING;
spin_unlock_irqrestore(>state_lock, flags);
--
2.23.0
Re: [PATCH 04/13] module: use RCU to synchronize find_module
Hi Christoph,
Christoph Hellwig writes:
> diff --git a/kernel/module.c b/kernel/module.c
> index 981302f616b411..6772fb2680eb3e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -668,7 +668,6 @@ static struct module *find_module_all(const char *name,
> size_t len,
>
> struct module *find_module(const char *name)
> {
> - module_assert_mutex();
Does it make sense to replace the assert above with the warn below (untested)?
RCU_LOCKDEP_WARN(rcu_read_lock_sched_held());
> return find_module_all(name, strlen(name), false);
> }
--
Thiago Jung Bauermann
IBM Linux Technology Center
Re: [PATCH] vio: make remove callback return void
Hello Sukadev, On 1/28/21 8:07 PM, Sukadev Bhattiprolu wrote: Slightly off-topic, should ndo_stop() also return a void? Its return value seems to be mostly ignored and [...] I don't know enough about the network stack to tell. Probably it's a good idea to start a separate thread for this and address this to the netdev list only. Best regards Uwe OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] vio: make remove callback return void
Uwe Kleine-König [u...@kleine-koenig.org] wrote: > The driver core ignores the return value of struct bus_type::remove() > because there is only little that can be done. To simplify the quest to > make this function return void, let struct vio_driver::remove() return > void, too. All users already unconditionally return 0, this commit makes > it obvious that returning an error code is a bad idea and makes it > obvious for future driver authors that returning an error code isn't > intended. Slightly off-topic, should ndo_stop() also return a void? Its return value seems to be mostly ignored and __dev_close_many() has: /* * Call the device specific close. This cannot fail. * Only if device is UP * * We allow it to be called even after a DETACH hot-plug * event. */ if (ops->ndo_stop) ops->ndo_stop(dev); Sukadev
Re: [PATCH 18/27] parisc: syscalls: switch to generic syscalltbl.sh
On 1/28/21 1:51 AM, Masahiro Yamada wrote: > As of v5.11-rc1, 12 architectures duplicate similar shell scripts in > order to generate syscall table headers. My goal is to unify them into > the single scripts/syscalltbl.sh. > > This commit converts parisc to use scripts/syscalltbl.sh. This also > unifies syscall_table_64.h and syscall_table_c32.h. > > Signed-off-by: Masahiro Yamada Thanks for the cleanup! You may add: Acked-by: Helge Deller # parisc Helge > --- > > arch/parisc/include/asm/Kbuild| 1 - > arch/parisc/kernel/syscall.S | 16 +- > arch/parisc/kernel/syscalls/Makefile | 19 > arch/parisc/kernel/syscalls/syscalltbl.sh | 36 --- > 4 files changed, 12 insertions(+), 60 deletions(-) > delete mode 100644 arch/parisc/kernel/syscalls/syscalltbl.sh > > diff --git a/arch/parisc/include/asm/Kbuild b/arch/parisc/include/asm/Kbuild > index 4406475a2304..e6e7f74c8ac9 100644 > --- a/arch/parisc/include/asm/Kbuild > +++ b/arch/parisc/include/asm/Kbuild > @@ -1,7 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 > generated-y += syscall_table_32.h > generated-y += syscall_table_64.h > -generated-y += syscall_table_c32.h > generic-y += kvm_para.h > generic-y += mcs_spinlock.h > generic-y += user.h > diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S > index 322503780db6..3f24a0af1e04 100644 > --- a/arch/parisc/kernel/syscall.S > +++ b/arch/parisc/kernel/syscall.S > @@ -919,24 +919,24 @@ ENTRY(lws_table) > END(lws_table) > /* End of lws table */ > > +#ifdef CONFIG_64BIT > +#define __SYSCALL_WITH_COMPAT(nr, native, compat)__SYSCALL(nr, compat) > +#else > +#define __SYSCALL_WITH_COMPAT(nr, native, compat)__SYSCALL(nr, native) > +#endif > #define __SYSCALL(nr, entry) ASM_ULONG_INSN entry > .align 8 > ENTRY(sys_call_table) > .export sys_call_table,data > -#ifdef CONFIG_64BIT > -#include/* Compat syscalls */ > -#else > -#include /* 32-bit native syscalls */ > -#endif > +#include /* 32-bit syscalls */ > END(sys_call_table) > > #ifdef CONFIG_64BIT > .align 8 > ENTRY(sys_call_table64) > -#include /* 64-bit native syscalls */ > +#include /* 64-bit syscalls */ > END(sys_call_table64) > #endif > -#undef __SYSCALL > > /* > All light-weight-syscall atomic operations > @@ -961,5 +961,3 @@ END(lws_lock_start) > .previous > > .end > - > - > diff --git a/arch/parisc/kernel/syscalls/Makefile > b/arch/parisc/kernel/syscalls/Makefile > index 556fe30a6c8f..77fea5beb9be 100644 > --- a/arch/parisc/kernel/syscalls/Makefile > +++ b/arch/parisc/kernel/syscalls/Makefile > @@ -7,7 +7,7 @@ _dummy := $(shell [ -d '$(uapi)' ] || mkdir -p '$(uapi)') > \ > > syscall := $(srctree)/$(src)/syscall.tbl > syshdr := $(srctree)/$(src)/syscallhdr.sh > -systbl := $(srctree)/$(src)/syscalltbl.sh > +systbl := $(srctree)/scripts/syscalltbl.sh > > quiet_cmd_syshdr = SYSHDR $@ >cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \ > @@ -16,10 +16,7 @@ quiet_cmd_syshdr = SYSHDR $@ > '$(syshdr_offset_$(basetarget))' > > quiet_cmd_systbl = SYSTBL $@ > - cmd_systbl = $(CONFIG_SHELL) '$(systbl)' '$<' '$@' \ > -'$(systbl_abis_$(basetarget))' \ > -'$(systbl_abi_$(basetarget))'\ > -'$(systbl_offset_$(basetarget))' > + cmd_systbl = $(CONFIG_SHELL) $(systbl) $< $@ $(abis) > > syshdr_abis_unistd_32 := common,32 > $(uapi)/unistd_32.h: $(syscall) $(syshdr) FORCE > @@ -29,23 +26,17 @@ syshdr_abis_unistd_64 := common,64 > $(uapi)/unistd_64.h: $(syscall) $(syshdr) FORCE > $(call if_changed,syshdr) > > -systbl_abis_syscall_table_32 := common,32 > +$(kapi)/syscall_table_32.h: abis := common,32 > $(kapi)/syscall_table_32.h: $(syscall) $(systbl) FORCE > $(call if_changed,systbl) > > -systbl_abis_syscall_table_64 := common,64 > +$(kapi)/syscall_table_64.h: abis := common,64 > $(kapi)/syscall_table_64.h: $(syscall) $(systbl) FORCE > $(call if_changed,systbl) > > -systbl_abis_syscall_table_c32 := common,32 > -systbl_abi_syscall_table_c32 := c32 > -$(kapi)/syscall_table_c32.h: $(syscall) $(systbl) FORCE > - $(call if_changed,systbl) > - > uapisyshdr-y += unistd_32.h unistd_64.h > kapisyshdr-y += syscall_table_32.h \ > -syscall_table_64.h \ > -syscall_table_c32.h > +syscall_table_64.h > > uapisyshdr-y := $(addprefix $(uapi)/, $(uapisyshdr-y)) > kapisyshdr-y := $(addprefix $(kapi)/, $(kapisyshdr-y)) > diff --git a/arch/parisc/kernel/syscalls/syscalltbl.sh > b/arch/parisc/kernel/syscalls/syscalltbl.sh > deleted file mode 100644 > index f7393a7b18aa.. > --- a/arch/parisc/kernel/syscalls/syscalltbl.sh > +++ /dev/null > @@ -1,36 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > - > -in="$1" > -out="$2" >
[PATCH 13/13] module: remove EXPORT_UNUSED_SYMBOL*
EXPORT_UNUSED_SYMBOL* is not actually used anywhere. Remove the unused functionality as we generally just remove unused code anyway. Signed-off-by: Christoph Hellwig --- arch/arm/configs/bcm2835_defconfig | 1 - arch/arm/configs/mxs_defconfig | 1 - arch/mips/configs/nlm_xlp_defconfig | 1 - arch/mips/configs/nlm_xlr_defconfig | 1 - arch/parisc/configs/generic-32bit_defconfig | 1 - arch/parisc/configs/generic-64bit_defconfig | 1 - arch/powerpc/configs/ppc6xx_defconfig | 1 - arch/s390/configs/debug_defconfig | 1 - arch/s390/configs/defconfig | 1 - arch/sh/configs/edosk7760_defconfig | 1 - arch/sh/configs/sdk7780_defconfig | 1 - arch/x86/configs/i386_defconfig | 1 - arch/x86/configs/x86_64_defconfig | 1 - arch/x86/tools/relocs.c | 4 +- include/asm-generic/vmlinux.lds.h | 28 include/linux/export.h | 8 --- include/linux/module.h | 12 init/Kconfig| 17 - kernel/module.c | 71 ++--- scripts/checkpatch.pl | 6 +- scripts/mod/modpost.c | 39 +-- scripts/mod/modpost.h | 2 - scripts/module.lds.S| 4 -- tools/include/linux/export.h| 2 - 24 files changed, 13 insertions(+), 193 deletions(-) diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig index 44ff9cd88d8161..d6c6c2e031c43a 100644 --- a/arch/arm/configs/bcm2835_defconfig +++ b/arch/arm/configs/bcm2835_defconfig @@ -177,7 +177,6 @@ CONFIG_BOOT_PRINTK_DELAY=y CONFIG_DYNAMIC_DEBUG=y CONFIG_DEBUG_INFO=y # CONFIG_ENABLE_MUST_CHECK is not set -CONFIG_UNUSED_SYMBOLS=y CONFIG_DEBUG_MEMORY_INIT=y CONFIG_LOCKUP_DETECTOR=y CONFIG_SCHED_TRACER=y diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig index a9c6f32a9b1c9d..ca32446b187f5d 100644 --- a/arch/arm/configs/mxs_defconfig +++ b/arch/arm/configs/mxs_defconfig @@ -164,7 +164,6 @@ CONFIG_FONTS=y CONFIG_PRINTK_TIME=y CONFIG_DEBUG_INFO=y CONFIG_FRAME_WARN=2048 -CONFIG_UNUSED_SYMBOLS=y CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_KERNEL=y CONFIG_SOFTLOCKUP_DETECTOR=y diff --git a/arch/mips/configs/nlm_xlp_defconfig b/arch/mips/configs/nlm_xlp_defconfig index 72a211d2d556fd..32c29061172325 100644 --- a/arch/mips/configs/nlm_xlp_defconfig +++ b/arch/mips/configs/nlm_xlp_defconfig @@ -549,7 +549,6 @@ CONFIG_PRINTK_TIME=y CONFIG_DEBUG_INFO=y # CONFIG_ENABLE_MUST_CHECK is not set CONFIG_FRAME_WARN=1024 -CONFIG_UNUSED_SYMBOLS=y CONFIG_DEBUG_MEMORY_INIT=y CONFIG_DETECT_HUNG_TASK=y CONFIG_SCHEDSTATS=y diff --git a/arch/mips/configs/nlm_xlr_defconfig b/arch/mips/configs/nlm_xlr_defconfig index 4ecb157e56d427..bf9b9244929ecd 100644 --- a/arch/mips/configs/nlm_xlr_defconfig +++ b/arch/mips/configs/nlm_xlr_defconfig @@ -500,7 +500,6 @@ CONFIG_CRC7=m CONFIG_PRINTK_TIME=y CONFIG_DEBUG_INFO=y # CONFIG_ENABLE_MUST_CHECK is not set -CONFIG_UNUSED_SYMBOLS=y CONFIG_DEBUG_MEMORY_INIT=y CONFIG_DETECT_HUNG_TASK=y CONFIG_SCHEDSTATS=y diff --git a/arch/parisc/configs/generic-32bit_defconfig b/arch/parisc/configs/generic-32bit_defconfig index 3cbcfad5f7249d..7611d48c599e01 100644 --- a/arch/parisc/configs/generic-32bit_defconfig +++ b/arch/parisc/configs/generic-32bit_defconfig @@ -22,7 +22,6 @@ CONFIG_PCI_LBA=y CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y -CONFIG_UNUSED_SYMBOLS=y # CONFIG_BLK_DEV_BSG is not set # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set CONFIG_BINFMT_MISC=m diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig index 8f81fcbf04c413..53054b81461a10 100644 --- a/arch/parisc/configs/generic-64bit_defconfig +++ b/arch/parisc/configs/generic-64bit_defconfig @@ -31,7 +31,6 @@ CONFIG_MODULE_FORCE_LOAD=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y -CONFIG_UNUSED_SYMBOLS=y CONFIG_BLK_DEV_INTEGRITY=y CONFIG_BINFMT_MISC=m # CONFIG_COMPACTION is not set diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig index ef09f3cce1fa85..34c3859040f9f7 100644 --- a/arch/powerpc/configs/ppc6xx_defconfig +++ b/arch/powerpc/configs/ppc6xx_defconfig @@ -1072,7 +1072,6 @@ CONFIG_NLS_ISO8859_15=m CONFIG_NLS_KOI8_R=m CONFIG_NLS_KOI8_U=m CONFIG_DEBUG_INFO=y -CONFIG_UNUSED_SYMBOLS=y CONFIG_HEADERS_INSTALL=y CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_KERNEL=y diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig index c4f6ff98a612cd..58e54d17e3154b 100644 --- a/arch/s390/configs/debug_defconfig +++ b/arch/s390/configs/debug_defconfig @@ -71,7 +71,6 @@ CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y CONFIG_MODULE_SRCVERSION_ALL=y CONFIG_MODULE_SIG_SHA256=y -CONFIG_UNUSED_SYMBOLS=y
[PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE
As far as I can tell this has never been used at all, and certainly
not any time recently.
Signed-off-by: Christoph Hellwig
---
arch/x86/tools/relocs.c | 4 ++--
include/asm-generic/vmlinux.lds.h | 14 --
include/linux/export.h| 1 -
include/linux/module.h| 5 -
kernel/module.c | 29 ++---
scripts/mod/modpost.c | 13 +
scripts/mod/modpost.h | 1 -
scripts/module.lds.S | 2 --
tools/include/linux/export.h | 1 -
9 files changed, 5 insertions(+), 65 deletions(-)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae58a..0d210d0e83e241 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -61,8 +61,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"(__iommu_table|__apicdrivers|__smp_locks)(|_end)|"
"__(start|end)_pci_.*|"
"__(start|end)_builtin_fw|"
- "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
- "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
+ "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl)|"
+ "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl)|"
"__(start|stop)___param|"
"__(start|stop)___modver|"
"__(start|stop)___bug_table|"
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index b2b3d81b1535a5..83243506e68b00 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -495,13 +495,6 @@
__stop___ksymtab_unused_gpl = .;\
} \
\
- /* Kernel symbol table: GPL-future-only symbols */ \
- __ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \
- __start___ksymtab_gpl_future = .; \
- KEEP(*(SORT(___ksymtab_gpl_future+*))) \
- __stop___ksymtab_gpl_future = .;\
- } \
- \
/* Kernel symbol table: Normal symbols */ \
__kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
__start___kcrctab = .; \
@@ -530,13 +523,6 @@
__stop___kcrctab_unused_gpl = .;\
} \
\
- /* Kernel symbol table: GPL-future-only symbols */ \
- __kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \
- __start___kcrctab_gpl_future = .; \
- KEEP(*(SORT(___kcrctab_gpl_future+*))) \
- __stop___kcrctab_gpl_future = .;\
- } \
- \
/* Kernel symbol table: strings */ \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {
\
*(__ksymtab_strings)\
diff --git a/include/linux/export.h b/include/linux/export.h
index fceb5e85571711..362b64f8d4a7c2 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -157,7 +157,6 @@ struct kernel_symbol {
#define EXPORT_SYMBOL(sym) _EXPORT_SYMBOL(sym, "")
#define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl")
-#define EXPORT_SYMBOL_GPL_FUTURE(sym) _EXPORT_SYMBOL(sym, "_gpl_future")
#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns)
#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns)
diff --git a/include/linux/module.h b/include/linux/module.h
index da0f5966ee80c9..e6e50ee3041238 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -411,11 +411,6 @@ struct module {
bool async_probe_requested;
- /* symbols that will be GPL-only in the near future. */
- const struct kernel_symbol *gpl_future_syms;
- const s32 *gpl_future_crcs;
- unsigned int num_gpl_future_syms;
-
/* Exception table */
unsigned int num_exentries;
struct exception_table_entry *extable;
diff --git a/kernel/module.c b/kernel/module.c
index 576c780e79c799..492b6fa5a662c8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -413,11 +413,8 @@ extern const struct kernel_symbol __start___ksymtab[];
extern const struct kernel_symbol __stop___ksymtab[];
extern const struct kernel_symbol
[PATCH 11/13] module: move struct symsearch to module.c
struct symsearch is only used inside of module.h, so move the definition
out of module.h.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 11 ---
kernel/module.c| 11 +++
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 0f360c48fe92a6..da0f5966ee80c9 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -587,17 +587,6 @@ static inline bool within_module(unsigned long addr, const
struct module *mod)
/* Search for module by name: must be in a RCU-sched critical section. */
struct module *find_module(const char *name);
-struct symsearch {
- const struct kernel_symbol *start, *stop;
- const s32 *crcs;
- enum mod_license {
- NOT_GPL_ONLY,
- GPL_ONLY,
- WILL_BE_GPL_ONLY,
- } license;
- bool unused;
-};
-
/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
symnum out of range. */
int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
diff --git a/kernel/module.c b/kernel/module.c
index f1441d39c015f5..576c780e79c799 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,6 +433,17 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
+struct symsearch {
+ const struct kernel_symbol *start, *stop;
+ const s32 *crcs;
+ enum mod_license {
+ NOT_GPL_ONLY,
+ GPL_ONLY,
+ WILL_BE_GPL_ONLY,
+ } license;
+ bool unused;
+};
+
struct find_symbol_arg {
/* Input */
const char *name;
--
2.29.2
[PATCH 10/13] module: pass struct find_symbol_args to find_symbol
Simplify the calling convention by passing the find_symbol_args structure
to find_symbol instead of initializing it inside the function.
Signed-off-by: Christoph Hellwig
---
kernel/module.c | 113 ++--
1 file changed, 52 insertions(+), 61 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index ff9045cc984b50..f1441d39c015f5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -536,12 +536,7 @@ static bool find_exported_symbol_in_section(const struct
symsearch *syms,
* Find an exported symbol and return it, along with, (optional) crc and
* (optional) module which owns it. Needs preempt disabled or module_mutex.
*/
-static const struct kernel_symbol *find_symbol(const char *name,
- struct module **owner,
- const s32 **crc,
- enum mod_license *license,
- bool gplok,
- bool warn)
+static bool find_symbol(struct find_symbol_arg *fsa)
{
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -561,19 +556,14 @@ static const struct kernel_symbol *find_symbol(const char
*name,
GPL_ONLY, true },
#endif
};
- struct find_symbol_arg fsa = {
- .name = name,
- .gplok = gplok,
- .warn = warn,
- };
struct module *mod;
unsigned int i;
module_assert_mutex_or_preempt();
for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section([i], NULL, ))
- goto found;
+ if (find_exported_symbol_in_section([i], NULL, fsa))
+ return true;
list_for_each_entry_rcu(mod, , list,
lockdep_is_held(_mutex)) {
@@ -603,21 +593,12 @@ static const struct kernel_symbol *find_symbol(const char
*name,
continue;
for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section([i], mod, ))
- goto found;
+ if (find_exported_symbol_in_section([i], mod, fsa))
+ return true;
}
- pr_debug("Failed to find symbol %s\n", name);
- return NULL;
-
-found:
- if (owner)
- *owner = fsa.owner;
- if (crc)
- *crc = fsa.crc;
- if (license)
- *license = fsa.license;
- return fsa.sym;
+ pr_debug("Failed to find symbol %s\n", fsa->name);
+ return false;
}
/*
@@ -1079,12 +1060,15 @@ static inline void print_unload_info(struct seq_file
*m, struct module *mod)
void __symbol_put(const char *symbol)
{
- struct module *owner;
+ struct find_symbol_arg fsa = {
+ .name = symbol,
+ .gplok = true,
+ };
preempt_disable();
- if (!find_symbol(symbol, , NULL, NULL, true, false))
+ if (!find_symbol())
BUG();
- module_put(owner);
+ module_put(fsa.owner);
preempt_enable();
}
EXPORT_SYMBOL(__symbol_put);
@@ -1353,19 +1337,22 @@ static int check_version(const struct load_info *info,
static inline int check_modstruct_version(const struct load_info *info,
struct module *mod)
{
- const s32 *crc;
+ struct find_symbol_arg fsa = {
+ .name = "module_layout",
+ .gplok = true,
+ };
/*
* Since this should be found in kernel (which can't be removed), no
* locking is necessary -- use preempt_disable() to placate lockdep.
*/
preempt_disable();
- if (!find_symbol("module_layout", NULL, , NULL, true, false)) {
+ if (!find_symbol()) {
preempt_enable();
BUG();
}
preempt_enable();
- return check_version(info, "module_layout", mod, crc);
+ return check_version(info, "module_layout", mod, fsa.crc);
}
/* First part is kernel version, which we ignore if module has crcs. */
@@ -1459,10 +1446,11 @@ static const struct kernel_symbol
*resolve_symbol(struct module *mod,
const char *name,
char ownername[])
{
- struct module *owner;
- const struct kernel_symbol *sym;
- const s32 *crc;
- enum mod_license license;
+ struct find_symbol_arg fsa = {
+ .name = name,
+ .gplok = !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)),
+ .warn = true,
+ };
int err;
/*
@@ -1472,42 +1460,40 @@ static const struct kernel_symbol
*resolve_symbol(struct module *mod,
*/
[PATCH 07/13] module: mark module_mutex static
Except for two lockdep asserts module_mutex is only used in module.c.
Remove the two asserts given that the functions they are in are not
exported and just called from the module code, and mark module_mutex
static.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 2 --
kernel/module.c| 2 +-
lib/bug.c | 3 ---
3 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 3ea4ffae608f97..0f360c48fe92a6 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -550,8 +550,6 @@ static inline unsigned long kallsyms_symbol_value(const
Elf_Sym *sym)
}
#endif
-extern struct mutex module_mutex;
-
/* FIXME: It'd be nice to isolate modules during init, too, so they
aren't used before they (may) fail. But presently too much code
(IDE & SCSI) require entry into the module during init.*/
diff --git a/kernel/module.c b/kernel/module.c
index 856df9e17ff3d0..5152fae1fc9406 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -87,7 +87,7 @@
* 3) module_addr_min/module_addr_max.
* (delete and add uses RCU list operations).
*/
-DEFINE_MUTEX(module_mutex);
+static DEFINE_MUTEX(module_mutex);
static LIST_HEAD(modules);
/* Work queue for freeing init sections in success case */
diff --git a/lib/bug.c b/lib/bug.c
index 7103440c0ee1af..8f9d537bfb2a59 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -91,8 +91,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr
*sechdrs,
char *secstrings;
unsigned int i;
- lockdep_assert_held(_mutex);
-
mod->bug_table = NULL;
mod->num_bugs = 0;
@@ -118,7 +116,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const
Elf_Shdr *sechdrs,
void module_bug_cleanup(struct module *mod)
{
- lockdep_assert_held(_mutex);
list_del_rcu(>bug_list);
}
--
2.29.2
[PATCH 09/13] module: merge each_symbol_section into find_symbol
each_symbol_section is only called by find_symbol, so merge the two
functions.
Signed-off-by: Christoph Hellwig
---
kernel/module.c | 148 ++--
1 file changed, 69 insertions(+), 79 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 945fe94b81fd92..ff9045cc984b50 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,73 +433,6 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
-/* Returns true as soon as fn returns true, otherwise false. */
-static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data),
-void *data)
-{
- unsigned int i;
- struct module *mod;
- static const struct symsearch arr[] = {
- { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- NOT_GPL_ONLY, false },
- { __start___ksymtab_gpl, __stop___ksymtab_gpl,
- __start___kcrctab_gpl,
- GPL_ONLY, false },
- { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
- __start___kcrctab_gpl_future,
- WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { __start___ksymtab_unused, __stop___ksymtab_unused,
- __start___kcrctab_unused,
- NOT_GPL_ONLY, true },
- { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
- __start___kcrctab_unused_gpl,
- GPL_ONLY, true },
-#endif
- };
-
- module_assert_mutex_or_preempt();
-
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (fn([i], NULL, data))
- return true;
-
- list_for_each_entry_rcu(mod, , list,
- lockdep_is_held(_mutex)) {
- struct symsearch arr[] = {
- { mod->syms, mod->syms + mod->num_syms, mod->crcs,
- NOT_GPL_ONLY, false },
- { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
- mod->gpl_crcs,
- GPL_ONLY, false },
- { mod->gpl_future_syms,
- mod->gpl_future_syms + mod->num_gpl_future_syms,
- mod->gpl_future_crcs,
- WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
- { mod->unused_syms,
- mod->unused_syms + mod->num_unused_syms,
- mod->unused_crcs,
- NOT_GPL_ONLY, true },
- { mod->unused_gpl_syms,
- mod->unused_gpl_syms + mod->num_unused_gpl_syms,
- mod->unused_gpl_crcs,
- GPL_ONLY, true },
-#endif
- };
-
- if (mod->state == MODULE_STATE_UNFORMED)
- continue;
-
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (fn([i], mod, data))
- return true;
- }
- return false;
-}
-
struct find_symbol_arg {
/* Input */
const char *name;
@@ -610,24 +543,81 @@ static const struct kernel_symbol *find_symbol(const char
*name,
bool gplok,
bool warn)
{
- struct find_symbol_arg fsa;
+ static const struct symsearch arr[] = {
+ { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
+ NOT_GPL_ONLY, false },
+ { __start___ksymtab_gpl, __stop___ksymtab_gpl,
+ __start___kcrctab_gpl,
+ GPL_ONLY, false },
+ { __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
+ __start___kcrctab_gpl_future,
+ WILL_BE_GPL_ONLY, false },
+#ifdef CONFIG_UNUSED_SYMBOLS
+ { __start___ksymtab_unused, __stop___ksymtab_unused,
+ __start___kcrctab_unused,
+ NOT_GPL_ONLY, true },
+ { __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
+ __start___kcrctab_unused_gpl,
+ GPL_ONLY, true },
+#endif
+ };
+ struct find_symbol_arg fsa = {
+ .name = name,
+ .gplok = gplok,
+ .warn = warn,
+ };
+ struct module *mod;
+ unsigned int i;
+
+ module_assert_mutex_or_preempt();
+
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (find_exported_symbol_in_section([i], NULL, ))
+ goto found;
- fsa.name = name;
- fsa.gplok = gplok;
- fsa.warn = warn;
+ list_for_each_entry_rcu(mod, , list,
+ lockdep_is_held(_mutex)) {
[PATCH 08/13] module: remove each_symbol_in_section
each_symbol_in_section just contains a trivial loop over its arguments.
Just open code the loop in the two callers.
Signed-off-by: Christoph Hellwig
---
kernel/module.c | 29 +++--
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 5152fae1fc9406..945fe94b81fd92 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,30 +433,13 @@ extern const s32 __start___kcrctab_unused_gpl[];
#define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
#endif
-static bool each_symbol_in_section(const struct symsearch *arr,
- unsigned int arrsize,
- struct module *owner,
- bool (*fn)(const struct symsearch *syms,
- struct module *owner,
- void *data),
- void *data)
-{
- unsigned int j;
-
- for (j = 0; j < arrsize; j++) {
- if (fn([j], owner, data))
- return true;
- }
-
- return false;
-}
-
/* Returns true as soon as fn returns true, otherwise false. */
static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data),
void *data)
{
+ unsigned int i;
struct module *mod;
static const struct symsearch arr[] = {
{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -479,8 +462,9 @@ static bool each_symbol_section(bool (*fn)(const struct
symsearch *arr,
module_assert_mutex_or_preempt();
- if (each_symbol_in_section(arr, ARRAY_SIZE(arr), NULL, fn, data))
- return true;
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (fn([i], NULL, data))
+ return true;
list_for_each_entry_rcu(mod, , list,
lockdep_is_held(_mutex)) {
@@ -509,8 +493,9 @@ static bool each_symbol_section(bool (*fn)(const struct
symsearch *arr,
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- if (each_symbol_in_section(arr, ARRAY_SIZE(arr), mod, fn, data))
- return true;
+ for (i = 0; i < ARRAY_SIZE(arr); i++)
+ if (fn([i], mod, data))
+ return true;
}
return false;
}
--
2.29.2
[PATCH 06/13] kallsyms: only build {, module_}kallsyms_on_each_symbol when required
kallsyms_on_each_symbol and module_kallsyms_on_each_symbol are only used
by the livepatching code, so don't build them if livepatching is not
enabled.
Signed-off-by: Christoph Hellwig
---
include/linux/kallsyms.h | 17 -
include/linux/module.h | 16
kernel/kallsyms.c| 2 ++
kernel/module.c | 2 ++
4 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 481273f0c72d42..465060acc9816f 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -71,15 +71,14 @@ static inline void *dereference_symbol_descriptor(void *ptr)
return ptr;
}
-#ifdef CONFIG_KALLSYMS
-/* Lookup the address for a symbol. Returns 0 if not found. */
-unsigned long kallsyms_lookup_name(const char *name);
-
-/* Call a function on each kallsyms symbol in the core kernel */
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data);
+#ifdef CONFIG_KALLSYMS
+/* Lookup the address for a symbol. Returns 0 if not found. */
+unsigned long kallsyms_lookup_name(const char *name);
+
extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset);
@@ -108,14 +107,6 @@ static inline unsigned long kallsyms_lookup_name(const
char *name)
return 0;
}
-static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
-{
- return 0;
-}
-
static inline int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset)
diff --git a/include/linux/module.h b/include/linux/module.h
index a64aa84d1b182c..3ea4ffae608f97 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -608,10 +608,6 @@ int module_get_kallsym(unsigned int symnum, unsigned long
*value, char *type,
/* Look for this name: can be of form module:name. */
unsigned long module_kallsyms_lookup_name(const char *name);
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
-struct module *, unsigned long),
- void *data);
-
extern void __noreturn __module_put_and_exit(struct module *mod,
long code);
#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
@@ -795,14 +791,6 @@ static inline unsigned long
module_kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char
*,
- struct module *,
- unsigned long),
-void *data)
-{
- return 0;
-}
-
static inline int register_module_notifier(struct notifier_block *nb)
{
/* no events will happen anyway, so this can always succeed */
@@ -891,4 +879,8 @@ static inline bool module_sig_ok(struct module *module)
}
#endif /* CONFIG_MODULE_SIG */
+int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+struct module *, unsigned long),
+ void *data);
+
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index a0d3f0865916f9..8043a90aa50ed3 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,7 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
+#ifdef CONFIG_LIVEPATCH
/*
* Iterate over all symbols in vmlinux. For symbols from modules use
* module_kallsyms_on_each_symbol instead.
@@ -198,6 +199,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
struct module *,
}
return 0;
}
+#endif /* CONFIG_LIVEPATCH */
static unsigned long get_symbol_pos(unsigned long addr,
unsigned long *symbolsize,
diff --git a/kernel/module.c b/kernel/module.c
index ae9045c5292a78..856df9e17ff3d0 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4371,6 +4371,7 @@ unsigned long module_kallsyms_lookup_name(const char
*name)
return ret;
}
+#ifdef CONFIG_LIVEPATCH
int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data)
@@ -4401,6 +4402,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *,
const char *,
mutex_unlock(_mutex);
return ret;
}
+#endif
[PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol
Require an explicit call to module_kallsyms_on_each_symbol to look
for symbols in modules instead of the call from kallsyms_on_each_symbol,
and acquire module_mutex inside of module_kallsyms_on_each_symbol instead
of leaving that up to the caller.
Signed-off-by: Christoph Hellwig
---
kernel/kallsyms.c | 6 +-
kernel/livepatch/core.c | 6 +-
kernel/module.c | 8
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index fe9de067771c34..a0d3f0865916f9 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,10 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
+/*
+ * Iterate over all symbols in vmlinux. For symbols from modules use
+ * module_kallsyms_on_each_symbol instead.
+ */
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data)
@@ -192,7 +196,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
struct module *,
if (ret != 0)
return ret;
}
- return module_kallsyms_on_each_symbol(fn, data);
+ return 0;
}
static unsigned long get_symbol_pos(unsigned long addr,
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 262cd9b003b9f0..f591dac5e86ef4 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -164,12 +164,8 @@ static int klp_find_object_symbol(const char *objname,
const char *name,
.pos = sympos,
};
- mutex_lock(_mutex);
- if (objname)
+ if (objname || !kallsyms_on_each_symbol(klp_find_callback, ))
module_kallsyms_on_each_symbol(klp_find_callback, );
- else
- kallsyms_on_each_symbol(klp_find_callback, );
- mutex_unlock(_mutex);
/*
* Ensure an address was found. If sympos is 0, ensure symbol is unique;
diff --git a/kernel/module.c b/kernel/module.c
index 6772fb2680eb3e..ae9045c5292a78 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4379,8 +4379,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *,
const char *,
unsigned int i;
int ret;
- module_assert_mutex();
-
+ mutex_lock(_mutex);
list_for_each_entry(mod, , list) {
/* We hold module_mutex: no need for rcu_dereference_sched */
struct mod_kallsyms *kallsyms = mod->kallsyms;
@@ -4396,10 +4395,11 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *,
const char *,
ret = fn(data, kallsyms_symbol_name(kallsyms, i),
mod, kallsyms_symbol_value(sym));
if (ret != 0)
- return ret;
+ break;
}
}
- return 0;
+ mutex_unlock(_mutex);
+ return ret;
}
#endif /* CONFIG_KALLSYMS */
--
2.29.2
[PATCH 03/13] module: unexport find_module and module_mutex
find_module is not used by modular code any more, and random driver code has no business calling it to start with. Signed-off-by: Christoph Hellwig --- kernel/module.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 4bf30e4b3eaaa1..981302f616b411 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -88,7 +88,6 @@ * (delete and add uses RCU list operations). */ DEFINE_MUTEX(module_mutex); -EXPORT_SYMBOL_GPL(module_mutex); static LIST_HEAD(modules); /* Work queue for freeing init sections in success case */ @@ -672,7 +671,6 @@ struct module *find_module(const char *name) module_assert_mutex(); return find_module_all(name, strlen(name), false); } -EXPORT_SYMBOL_GPL(find_module); #ifdef CONFIG_SMP -- 2.29.2
[PATCH 04/13] module: use RCU to synchronize find_module
Allow for a RCU-sched critical section around find_module, following
the lower level find_module_all helper, and switch the two callers
outside of module.c to use such a RCU-sched critical section instead
of module_mutex.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 2 +-
kernel/livepatch/core.c | 5 +++--
kernel/module.c | 1 -
kernel/trace/trace_kprobe.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 7a0bcb5b1ffccd..a64aa84d1b182c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -586,7 +586,7 @@ static inline bool within_module(unsigned long addr, const
struct module *mod)
return within_module_init(addr, mod) || within_module_core(addr, mod);
}
-/* Search for module by name: must hold module_mutex. */
+/* Search for module by name: must be in a RCU-sched critical section. */
struct module *find_module(const char *name);
struct symsearch {
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index f76fdb9255323d..262cd9b003b9f0 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include "core.h"
#include "patch.h"
@@ -57,7 +58,7 @@ static void klp_find_object_module(struct klp_object *obj)
if (!klp_is_module(obj))
return;
- mutex_lock(_mutex);
+ rcu_read_lock_sched();
/*
* We do not want to block removal of patched modules and therefore
* we do not take a reference here. The patches are removed by
@@ -74,7 +75,7 @@ static void klp_find_object_module(struct klp_object *obj)
if (mod && mod->klp_alive)
obj->mod = mod;
- mutex_unlock(_mutex);
+ rcu_read_unlock_sched();
}
static bool klp_initialized(void)
diff --git a/kernel/module.c b/kernel/module.c
index 981302f616b411..6772fb2680eb3e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -668,7 +668,6 @@ static struct module *find_module_all(const char *name,
size_t len,
struct module *find_module(const char *name)
{
- module_assert_mutex();
return find_module_all(name, strlen(name), false);
}
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e6fba1798771b4..3137992baa5e7a 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -124,9 +124,9 @@ static nokprobe_inline bool
trace_kprobe_module_exist(struct trace_kprobe *tk)
if (!p)
return true;
*p = '\0';
- mutex_lock(_mutex);
+ rcu_read_lock_sched();
ret = !!find_module(tk->symbol);
- mutex_unlock(_mutex);
+ rcu_read_unlock_sched();
*p = ':';
return ret;
--
2.29.2
[PATCH 02/13] drm: remove drm_fb_helper_modinit
drm_fb_helper_modinit has a lot of boilerplate for what is not very
simple functionality. Just open code it in the only caller using
IS_ENABLED and IS_MODULE, and skip the find_module check as a
request_module is harmless if the module is already loaded (and not
other caller has this find_module check either).
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/drm_crtc_helper_internal.h | 10 -
drivers/gpu/drm/drm_fb_helper.c| 21 --
drivers/gpu/drm/drm_kms_helper_common.c| 25 +++---
3 files changed, 12 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
b/drivers/gpu/drm/drm_crtc_helper_internal.h
index 25ce42e799952c..61e09f8a8d0ff0 100644
--- a/drivers/gpu/drm/drm_crtc_helper_internal.h
+++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
@@ -32,16 +32,6 @@
#include
#include
-/* drm_fb_helper.c */
-#ifdef CONFIG_DRM_FBDEV_EMULATION
-int drm_fb_helper_modinit(void);
-#else
-static inline int drm_fb_helper_modinit(void)
-{
- return 0;
-}
-#endif
-
/* drm_dp_aux_dev.c */
#ifdef CONFIG_DRM_DP_AUX_CHARDEV
int drm_dp_aux_dev_init(void);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4b81195106875d..0b9f1ae1b7864c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2499,24 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
drm_client_register(_helper->client);
}
EXPORT_SYMBOL(drm_fbdev_generic_setup);
-
-/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
- * but the module doesn't depend on any fb console symbols. At least
- * attempt to load fbcon to avoid leaving the system without a usable console.
- */
-int __init drm_fb_helper_modinit(void)
-{
-#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
- const char name[] = "fbcon";
- struct module *fbcon;
-
- mutex_lock(_mutex);
- fbcon = find_module(name);
- mutex_unlock(_mutex);
-
- if (!fbcon)
- request_module_nowait(name);
-#endif
- return 0;
-}
-EXPORT_SYMBOL(drm_fb_helper_modinit);
diff --git a/drivers/gpu/drm/drm_kms_helper_common.c
b/drivers/gpu/drm/drm_kms_helper_common.c
index 221a8528c9937a..f933da1656eb52 100644
--- a/drivers/gpu/drm/drm_kms_helper_common.c
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -64,19 +64,18 @@ MODULE_PARM_DESC(edid_firmware,
static int __init drm_kms_helper_init(void)
{
- int ret;
-
- /* Call init functions from specific kms helpers here */
- ret = drm_fb_helper_modinit();
- if (ret < 0)
- goto out;
-
- ret = drm_dp_aux_dev_init();
- if (ret < 0)
- goto out;
-
-out:
- return ret;
+ /*
+* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
+* but the module doesn't depend on any fb console symbols. At least
+* attempt to load fbcon to avoid leaving the system without a usable
+* console.
+*/
+ if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
+ IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
+ !IS_ENABLED(CONFIG_EXPERT))
+ request_module_nowait("fbcon");
+
+ return drm_dp_aux_dev_init();
}
static void __exit drm_kms_helper_exit(void)
--
2.29.2
module loader dead code removal and cleanups v2
Hi all, this series removes support for long term unused export types and cleans up various loose ends in the module loader. Changes since v1: - move struct symsearch to module.c - rework drm to not call find_module at all - llow RCU-sched locking for find_module - keep find_module as a public API instead of module_loaded - update a few comments and commit logs Diffstat: arch/arm/configs/bcm2835_defconfig |1 arch/arm/configs/mxs_defconfig |1 arch/mips/configs/nlm_xlp_defconfig |1 arch/mips/configs/nlm_xlr_defconfig |1 arch/parisc/configs/generic-32bit_defconfig |1 arch/parisc/configs/generic-64bit_defconfig |1 arch/powerpc/configs/ppc6xx_defconfig |1 arch/powerpc/platforms/powernv/pci-cxl.c| 22 - arch/s390/configs/debug_defconfig |1 arch/s390/configs/defconfig |1 arch/sh/configs/edosk7760_defconfig |1 arch/sh/configs/sdk7780_defconfig |1 arch/x86/configs/i386_defconfig |1 arch/x86/configs/x86_64_defconfig |1 arch/x86/tools/relocs.c |4 drivers/gpu/drm/drm_crtc_helper_internal.h | 10 drivers/gpu/drm/drm_fb_helper.c | 21 - drivers/gpu/drm/drm_kms_helper_common.c | 25 +- include/asm-generic/vmlinux.lds.h | 42 --- include/linux/export.h |9 include/linux/kallsyms.h| 17 - include/linux/module.h | 48 init/Kconfig| 17 - kernel/kallsyms.c |8 kernel/livepatch/core.c | 11 kernel/module.c | 310 +--- kernel/trace/trace_kprobe.c |4 lib/bug.c |3 scripts/checkpatch.pl |6 scripts/mod/modpost.c | 50 scripts/mod/modpost.h |3 scripts/module.lds.S|6 tools/include/linux/export.h|3 33 files changed, 142 insertions(+), 490 deletions(-)
[PATCH 01/13] powerpc/powernv: remove get_cxl_module
The static inline get_cxl_module function is entirely unused since commit
8bf6b91a5125a ("Revert "powerpc/powernv: Add support for the cxl kernel
api on the real phb"), so remove it.
Signed-off-by: Christoph Hellwig
Reviewed-by: Andrew Donnellan
---
arch/powerpc/platforms/powernv/pci-cxl.c | 22 --
1 file changed, 22 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-cxl.c
b/arch/powerpc/platforms/powernv/pci-cxl.c
index 8c739c94ed28d6..53172862d23bd3 100644
--- a/arch/powerpc/platforms/powernv/pci-cxl.c
+++ b/arch/powerpc/platforms/powernv/pci-cxl.c
@@ -150,25 +150,3 @@ int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned
int hwirq,
return 0;
}
EXPORT_SYMBOL(pnv_cxl_ioda_msi_setup);
-
-#if IS_MODULE(CONFIG_CXL)
-static inline int get_cxl_module(void)
-{
- struct module *cxl_module;
-
- mutex_lock(_mutex);
-
- cxl_module = find_module("cxl");
- if (cxl_module)
- __module_get(cxl_module);
-
- mutex_unlock(_mutex);
-
- if (!cxl_module)
- return -ENODEV;
-
- return 0;
-}
-#else
-static inline int get_cxl_module(void) { return 0; }
-#endif
--
2.29.2
Re: [PATCH] powerpc/sstep: Fix array out of bound warning
On 2021/01/15 11:46AM, Ravi Bangoria wrote:
> Compiling kernel with -Warray-bounds throws below warning:
>
> In function 'emulate_vsx_store':
> warning: array subscript is above array bounds [-Warray-bounds]
> buf.d[2] = byterev_8(reg->d[1]);
> ~^~~
> buf.d[3] = byterev_8(reg->d[0]);
> ~^~~
>
> Fix it by converting local variable 'union vsx_reg buf' into an array.
> Also consider function argument 'union vsx_reg *reg' as array instead
> of pointer because callers are actually passing an array to it.
I think you should change the function prototype to reflect this.
However, while I agree with this change in principle, it looks to be a
lot of code churn for a fairly narrow use. Perhaps we should just
address the specific bug. Something like the below (not tested)?
@@ -818,13 +818,15 @@ void emulate_vsx_store(struct instruction_op *op, const
union vsx_reg *reg,
break;
if (rev) {
/* reverse 32 bytes */
- buf.d[0] = byterev_8(reg->d[3]);
- buf.d[1] = byterev_8(reg->d[2]);
- buf.d[2] = byterev_8(reg->d[1]);
- buf.d[3] = byterev_8(reg->d[0]);
- reg =
+ union vsx_reg buf32[2];
+ buf32[0].d[0] = byterev_8(reg[1].d[1]);
+ buf32[0].d[1] = byterev_8(reg[1].d[0]);
+ buf32[1].d[0] = byterev_8(reg[0].d[1]);
+ buf32[1].d[1] = byterev_8(reg[0].d[0]);
+ memcpy(mem, buf32, size);
+ } else {
+ memcpy(mem, reg, size);
}
- memcpy(mem, reg, size);
break;
case 16:
/* stxv, stxvx, stxvl, stxvll */
- Naveen
Re: [PATCH 03/13] livepatch: refactor klp_init_object
On Thu, Jan 28, 2021 at 05:22:40PM +0100, Christoph Hellwig wrote: > > We need to either update the function description or keep this check. > > > > I prefer to keep the check. The function does the right thing also > > for the object "vmlinux". Also the livepatch code includes many > > similar paranoid checks that makes the code less error prone > > against any further changes. > > Well, the check is in the caller now where we have a conditional for > it. So I'd be tempted to either update the comment, or just drop the > patch. Also even without the check I think it will do the right thing when called for vmlinux given that it simplify won't find a module called vmlinux..
Re: [PATCH 03/13] livepatch: refactor klp_init_object
On Wed, Jan 27, 2021 at 01:58:21PM +0100, Petr Mladek wrote:
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -54,9 +54,6 @@ static void klp_find_object_module(struct klp_object *obj)
> > {
> > struct module *mod;
> >
> > - if (!klp_is_module(obj))
> > - return;
> > -
>
> We need to either update the function description or keep this check.
>
> I prefer to keep the check. The function does the right thing also
> for the object "vmlinux". Also the livepatch code includes many
> similar paranoid checks that makes the code less error prone
> against any further changes.
Well, the check is in the caller now where we have a conditional for
it. So I'd be tempted to either update the comment, or just drop the
patch.
Re: [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*
On Wed, Jan 27, 2021 at 02:49:38PM +0100, Jessica Yu wrote:
>> #ifdef CONFIG_MODULE_SIG
>> /* Signature was verified. */
>> bool sig_ok;
>> @@ -592,7 +580,6 @@ struct symsearch {
>> GPL_ONLY,
>> WILL_BE_GPL_ONLY,
>> } license;
>> -bool unused;
>> };
> Thanks for the cleanups. While we're here, I noticed that struct
> symsearch is only used internally in kernel/module.c, so I don't think
> it actually needs to be in include/linux/module.h. I don't see it used
> anywhere else. We could move maybe that to kernel/module-internal.h.
I've added a patch to just move it directly into module.c.
Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
On Thu, Jan 28, 2021 at 03:44:21PM +0100, Christophe Leroy wrote: > > > Le 28/01/2021 à 15:42, Jens Axboe a écrit : > > On 1/28/21 6:52 AM, Zorro Lang wrote: > > > On Wed, Jan 27, 2021 at 08:06:37PM -0700, Jens Axboe wrote: > > > > On 1/27/21 8:13 PM, Zorro Lang wrote: > > > > > On Thu, Jan 28, 2021 at 10:18:07AM +1000, Nicholas Piggin wrote: > > > > > > Excerpts from Jens Axboe's message of January 28, 2021 5:29 am: > > > > > > > On 1/27/21 9:38 AM, Christophe Leroy wrote: > > > > > > > > > > > > > > > > > > > > > > > > Le 27/01/2021 à 15:56, Zorro Lang a écrit : > > > > > > > > > On powerpc, io_uring test hit below KUAP fault on > > > > > > > > > __do_page_fault. > > > > > > > > > The fail source line is: > > > > > > > > > > > > > > > > > > if (unlikely(!is_user && bad_kernel_fault(regs, > > > > > > > > > error_code, address, is_write))) > > > > > > > > > return SIGSEGV; > > > > > > > > > > > > > > > > > > The is_user() is based on user_mod(regs) only. This's not > > > > > > > > > suit for > > > > > > > > > io_uring, where the helper thread can assume the user app > > > > > > > > > identity > > > > > > > > > and could perform this fault just fine. So turn to use mm to > > > > > > > > > decide > > > > > > > > > if this is valid or not. > > > > > > > > > > > > > > > > I don't understand why testing is_user would be an issue. KUAP > > > > > > > > purpose > > > > > > > > it to block any unallowed access from kernel to user memory > > > > > > > > (Equivalent to SMAP on x86). So it really must be based on > > > > > > > > MSR_PR bit, > > > > > > > > that is what is_user provides. > > > > > > > > > > > > > > > > If the kernel access is legitimate, kernel should have opened > > > > > > > > userspace access then you shouldn't get this "Bug: Read fault > > > > > > > > blocked > > > > > > > > by KUAP!". > > > > > > > > > > > > > > > > As far as I understand, the fault occurs in > > > > > > > > iov_iter_fault_in_readable() which calls > > > > > > > > fault_in_pages_readable() And > > > > > > > > fault_in_pages_readable() uses __get_user() so it is a > > > > > > > > legitimate > > > > > > > > access and you really should get a KUAP fault. > > > > > > > > > > > > > > > > So the problem is somewhere else, I think you proposed patch > > > > > > > > just > > > > > > > > hides the problem, it doesn't fix it. > > > > > > > > > > > > > > If we do kthread_use_mm(), can we agree that the user access is > > > > > > > valid? > > > > > > > > > > > > Yeah the io uring code is fine, provided it uses the uaccess > > > > > > primitives > > > > > > like any other kernel code. It's looking more like a an > > > > > > arch/powerpc bug. > > > > > > > > > > > > > We should be able to copy to/from user space, and including > > > > > > > faults, if > > > > > > > that's been done and the new mm assigned. Because it really > > > > > > > should be. > > > > > > > If SMAP was a problem on x86, we would have seen it long ago. > > > > > > > > > > > > > > I'm assuming this may be breakage related to the recent uaccess > > > > > > > changes > > > > > > > related to set_fs and friends? Or maybe recent changes on the > > > > > > > powerpc > > > > > > > side? > > > > > > > > > > > > > > Zorro, did 5.10 work? > > > > > > > > > > > > Would be interesting to know. > > > > > > > > > > Sure Nick and Jens, which 5.10 rc? version do you want to know ? Or > > > > > any git > > > > > commit(be the HEAD) in 5.10 phase? > > > > > > > > I forget which versions had what series of this, but 5.10 final - and if > > > > that fails, then 5.9 final. IIRC, 5.9 was pre any of these changes, and > > > > 5.10 definitely has them. > > > > > > I justed built linux v5.10 with same .config file, and gave it same test. > > > v5.10 (HEAD=2c85ebc57b Linux 5.10) can't reproduce this bug: > > > > > > # ./check generic/013 generic/051 > > > FSTYP -- xfs (non-debug) > > > PLATFORM -- Linux/ppc64le ibm-p9z-xxx- 5.10.0 #3 SMP Thu Jan 28 > > > 04:12:14 EST 2021 > > > MKFS_OPTIONS -- -f -m > > > crc=1,finobt=1,reflink=1,rmapbt=1,bigtime=1,inobtcount=1 /dev/sda3 > > > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 > > > /mnt/xfstests/scratch > > > > > > generic/013 138s ... 77s > > > generic/051 103s ... 143s > > > Ran: generic/013 generic/051 > > > Passed all 2 tests > > > > Thanks for testing that, so I think it's safe to conclude that there's a > > regression in powerpc fault handling for kthreads that use > > kthread_use_mm in this release. A bisect would definitely find it, but > > might be pointless if Christophe or Nick already have an idea of what it > > is. > > > > I don't have any idea yet, but I'd be curious to see the vmlinux binary > matching the reported Oops. OK, I don't have the vmlinux matching that bug report now, I can help to prepare a new one, but I need lots of time (about 10+ hours). Thanks, Zorro > > Christophe >
Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
Le 28/01/2021 à 15:42, Jens Axboe a écrit : On 1/28/21 6:52 AM, Zorro Lang wrote: On Wed, Jan 27, 2021 at 08:06:37PM -0700, Jens Axboe wrote: On 1/27/21 8:13 PM, Zorro Lang wrote: On Thu, Jan 28, 2021 at 10:18:07AM +1000, Nicholas Piggin wrote: Excerpts from Jens Axboe's message of January 28, 2021 5:29 am: On 1/27/21 9:38 AM, Christophe Leroy wrote: Le 27/01/2021 à 15:56, Zorro Lang a écrit : On powerpc, io_uring test hit below KUAP fault on __do_page_fault. The fail source line is: if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write))) return SIGSEGV; The is_user() is based on user_mod(regs) only. This's not suit for io_uring, where the helper thread can assume the user app identity and could perform this fault just fine. So turn to use mm to decide if this is valid or not. I don't understand why testing is_user would be an issue. KUAP purpose it to block any unallowed access from kernel to user memory (Equivalent to SMAP on x86). So it really must be based on MSR_PR bit, that is what is_user provides. If the kernel access is legitimate, kernel should have opened userspace access then you shouldn't get this "Bug: Read fault blocked by KUAP!". As far as I understand, the fault occurs in iov_iter_fault_in_readable() which calls fault_in_pages_readable() And fault_in_pages_readable() uses __get_user() so it is a legitimate access and you really should get a KUAP fault. So the problem is somewhere else, I think you proposed patch just hides the problem, it doesn't fix it. If we do kthread_use_mm(), can we agree that the user access is valid? Yeah the io uring code is fine, provided it uses the uaccess primitives like any other kernel code. It's looking more like a an arch/powerpc bug. We should be able to copy to/from user space, and including faults, if that's been done and the new mm assigned. Because it really should be. If SMAP was a problem on x86, we would have seen it long ago. I'm assuming this may be breakage related to the recent uaccess changes related to set_fs and friends? Or maybe recent changes on the powerpc side? Zorro, did 5.10 work? Would be interesting to know. Sure Nick and Jens, which 5.10 rc? version do you want to know ? Or any git commit(be the HEAD) in 5.10 phase? I forget which versions had what series of this, but 5.10 final - and if that fails, then 5.9 final. IIRC, 5.9 was pre any of these changes, and 5.10 definitely has them. I justed built linux v5.10 with same .config file, and gave it same test. v5.10 (HEAD=2c85ebc57b Linux 5.10) can't reproduce this bug: # ./check generic/013 generic/051 FSTYP -- xfs (non-debug) PLATFORM -- Linux/ppc64le ibm-p9z-xxx- 5.10.0 #3 SMP Thu Jan 28 04:12:14 EST 2021 MKFS_OPTIONS -- -f -m crc=1,finobt=1,reflink=1,rmapbt=1,bigtime=1,inobtcount=1 /dev/sda3 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch generic/013 138s ... 77s generic/051 103s ... 143s Ran: generic/013 generic/051 Passed all 2 tests Thanks for testing that, so I think it's safe to conclude that there's a regression in powerpc fault handling for kthreads that use kthread_use_mm in this release. A bisect would definitely find it, but might be pointless if Christophe or Nick already have an idea of what it is. I don't have any idea yet, but I'd be curious to see the vmlinux binary matching the reported Oops. Christophe
Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
On 1/28/21 6:52 AM, Zorro Lang wrote: > On Wed, Jan 27, 2021 at 08:06:37PM -0700, Jens Axboe wrote: >> On 1/27/21 8:13 PM, Zorro Lang wrote: >>> On Thu, Jan 28, 2021 at 10:18:07AM +1000, Nicholas Piggin wrote: Excerpts from Jens Axboe's message of January 28, 2021 5:29 am: > On 1/27/21 9:38 AM, Christophe Leroy wrote: >> >> >> Le 27/01/2021 à 15:56, Zorro Lang a écrit : >>> On powerpc, io_uring test hit below KUAP fault on __do_page_fault. >>> The fail source line is: >>> >>>if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, >>> is_write))) >>>return SIGSEGV; >>> >>> The is_user() is based on user_mod(regs) only. This's not suit for >>> io_uring, where the helper thread can assume the user app identity >>> and could perform this fault just fine. So turn to use mm to decide >>> if this is valid or not. >> >> I don't understand why testing is_user would be an issue. KUAP purpose >> it to block any unallowed access from kernel to user memory >> (Equivalent to SMAP on x86). So it really must be based on MSR_PR bit, >> that is what is_user provides. >> >> If the kernel access is legitimate, kernel should have opened >> userspace access then you shouldn't get this "Bug: Read fault blocked >> by KUAP!". >> >> As far as I understand, the fault occurs in >> iov_iter_fault_in_readable() which calls fault_in_pages_readable() And >> fault_in_pages_readable() uses __get_user() so it is a legitimate >> access and you really should get a KUAP fault. >> >> So the problem is somewhere else, I think you proposed patch just >> hides the problem, it doesn't fix it. > > If we do kthread_use_mm(), can we agree that the user access is valid? Yeah the io uring code is fine, provided it uses the uaccess primitives like any other kernel code. It's looking more like a an arch/powerpc bug. > We should be able to copy to/from user space, and including faults, if > that's been done and the new mm assigned. Because it really should be. > If SMAP was a problem on x86, we would have seen it long ago. > > I'm assuming this may be breakage related to the recent uaccess changes > related to set_fs and friends? Or maybe recent changes on the powerpc > side? > > Zorro, did 5.10 work? Would be interesting to know. >>> >>> Sure Nick and Jens, which 5.10 rc? version do you want to know ? Or any git >>> commit(be the HEAD) in 5.10 phase? >> >> I forget which versions had what series of this, but 5.10 final - and if >> that fails, then 5.9 final. IIRC, 5.9 was pre any of these changes, and >> 5.10 definitely has them. > > I justed built linux v5.10 with same .config file, and gave it same test. > v5.10 (HEAD=2c85ebc57b Linux 5.10) can't reproduce this bug: > > # ./check generic/013 generic/051 > FSTYP -- xfs (non-debug) > PLATFORM -- Linux/ppc64le ibm-p9z-xxx- 5.10.0 #3 SMP Thu Jan 28 > 04:12:14 EST 2021 > MKFS_OPTIONS -- -f -m > crc=1,finobt=1,reflink=1,rmapbt=1,bigtime=1,inobtcount=1 /dev/sda3 > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 > /mnt/xfstests/scratch > > generic/013 138s ... 77s > generic/051 103s ... 143s > Ran: generic/013 generic/051 > Passed all 2 tests Thanks for testing that, so I think it's safe to conclude that there's a regression in powerpc fault handling for kthreads that use kthread_use_mm in this release. A bisect would definitely find it, but might be pointless if Christophe or Nick already have an idea of what it is. -- Jens Axboe
Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
On Wed, Jan 27, 2021 at 08:06:37PM -0700, Jens Axboe wrote: > On 1/27/21 8:13 PM, Zorro Lang wrote: > > On Thu, Jan 28, 2021 at 10:18:07AM +1000, Nicholas Piggin wrote: > >> Excerpts from Jens Axboe's message of January 28, 2021 5:29 am: > >>> On 1/27/21 9:38 AM, Christophe Leroy wrote: > > > Le 27/01/2021 à 15:56, Zorro Lang a écrit : > > On powerpc, io_uring test hit below KUAP fault on __do_page_fault. > > The fail source line is: > > > >if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, > > is_write))) > >return SIGSEGV; > > > > The is_user() is based on user_mod(regs) only. This's not suit for > > io_uring, where the helper thread can assume the user app identity > > and could perform this fault just fine. So turn to use mm to decide > > if this is valid or not. > > I don't understand why testing is_user would be an issue. KUAP purpose > it to block any unallowed access from kernel to user memory > (Equivalent to SMAP on x86). So it really must be based on MSR_PR bit, > that is what is_user provides. > > If the kernel access is legitimate, kernel should have opened > userspace access then you shouldn't get this "Bug: Read fault blocked > by KUAP!". > > As far as I understand, the fault occurs in > iov_iter_fault_in_readable() which calls fault_in_pages_readable() And > fault_in_pages_readable() uses __get_user() so it is a legitimate > access and you really should get a KUAP fault. > > So the problem is somewhere else, I think you proposed patch just > hides the problem, it doesn't fix it. > >>> > >>> If we do kthread_use_mm(), can we agree that the user access is valid? > >> > >> Yeah the io uring code is fine, provided it uses the uaccess primitives > >> like any other kernel code. It's looking more like a an arch/powerpc bug. > >> > >>> We should be able to copy to/from user space, and including faults, if > >>> that's been done and the new mm assigned. Because it really should be. > >>> If SMAP was a problem on x86, we would have seen it long ago. > >>> > >>> I'm assuming this may be breakage related to the recent uaccess changes > >>> related to set_fs and friends? Or maybe recent changes on the powerpc > >>> side? > >>> > >>> Zorro, did 5.10 work? > >> > >> Would be interesting to know. > > > > Sure Nick and Jens, which 5.10 rc? version do you want to know ? Or any git > > commit(be the HEAD) in 5.10 phase? > > I forget which versions had what series of this, but 5.10 final - and if > that fails, then 5.9 final. IIRC, 5.9 was pre any of these changes, and > 5.10 definitely has them. I justed built linux v5.10 with same .config file, and gave it same test. v5.10 (HEAD=2c85ebc57b Linux 5.10) can't reproduce this bug: # ./check generic/013 generic/051 FSTYP -- xfs (non-debug) PLATFORM -- Linux/ppc64le ibm-p9z-xxx- 5.10.0 #3 SMP Thu Jan 28 04:12:14 EST 2021 MKFS_OPTIONS -- -f -m crc=1,finobt=1,reflink=1,rmapbt=1,bigtime=1,inobtcount=1 /dev/sda3 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch generic/013 138s ... 77s generic/051 103s ... 143s Ran: generic/013 generic/051 Passed all 2 tests > > -- > Jens Axboe >
[PATCH 1/3] sched: Remove MAX_USER_RT_PRIO
Commit d46523ea32a7 ("[PATCH] fix MAX_USER_RT_PRIO and MAX_RT_PRIO")
was introduced due to a a small time period in which the realtime patch
set was using different values for MAX_USER_RT_PRIO and MAX_RT_PRIO.
This is no longer true, i.e. now MAX_RT_PRIO == MAX_USER_RT_PRIO.
Get rid of MAX_USER_RT_PRIO and make everything use MAX_RT_PRIO
instead.
Signed-off-by: Dietmar Eggemann
---
include/linux/sched/prio.h | 9 +
kernel/sched/core.c| 7 +++
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
index 7d64feafc408..d111f2fd77ea 100644
--- a/include/linux/sched/prio.h
+++ b/include/linux/sched/prio.h
@@ -11,16 +11,9 @@
* priority is 0..MAX_RT_PRIO-1, and SCHED_NORMAL/SCHED_BATCH
* tasks are in the range MAX_RT_PRIO..MAX_PRIO-1. Priority
* values are inverted: lower p->prio value means higher priority.
- *
- * The MAX_USER_RT_PRIO value allows the actual maximum
- * RT priority to be separate from the value exported to
- * user-space. This allows kernel threads to set their
- * priority to a value higher than any user task. Note:
- * MAX_RT_PRIO must not be smaller than MAX_USER_RT_PRIO.
*/
-#define MAX_USER_RT_PRIO 100
-#define MAX_RT_PRIOMAX_USER_RT_PRIO
+#define MAX_RT_PRIO100
#define MAX_PRIO (MAX_RT_PRIO + NICE_WIDTH)
#define DEFAULT_PRIO (MAX_RT_PRIO + NICE_WIDTH / 2)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 06b449942adf..625ec1e12064 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5897,11 +5897,10 @@ static int __sched_setscheduler(struct task_struct *p,
/*
* Valid priorities for SCHED_FIFO and SCHED_RR are
-* 1..MAX_USER_RT_PRIO-1, valid priority for SCHED_NORMAL,
+* 1..MAX_RT_PRIO-1, valid priority for SCHED_NORMAL,
* SCHED_BATCH and SCHED_IDLE is 0.
*/
- if ((p->mm && attr->sched_priority > MAX_USER_RT_PRIO-1) ||
- (!p->mm && attr->sched_priority > MAX_RT_PRIO-1))
+ if (attr->sched_priority > MAX_RT_PRIO-1)
return -EINVAL;
if ((dl_policy(policy) && !__checkparam_dl(attr)) ||
(rt_policy(policy) != (attr->sched_priority != 0)))
@@ -6969,7 +6968,7 @@ SYSCALL_DEFINE1(sched_get_priority_max, int, policy)
switch (policy) {
case SCHED_FIFO:
case SCHED_RR:
- ret = MAX_USER_RT_PRIO-1;
+ ret = MAX_RT_PRIO-1;
break;
case SCHED_DEADLINE:
case SCHED_NORMAL:
--
2.25.1
[PATCH 0/3] sched: Task priority related cleanups
(1) Removing MAX_USER_RT_PRIO was already discussed here in April 2020:
https://lkml.kernel.org/r/20200423094403.6f1d2...@gandalf.local.home
(2) USER_PRIO() and related macros are not used anymore except in one
case for powerpc where MAX_USER_PRIO can be replaced by NICE_WIDTH.
Set_load_weight(), task_prio(), cpu_weight_nice_write_s64(),
__update_max_tr() don't use USER_PRIO() but priority - MAX_RT_PRIO.
(3) The function header of task_prio() needs an update. It looks
ancient since it mentions a prio space [-16 ... 15] for mormal
tasks. I can't figure out why this range is mentioned here? Maybe
the influence of the 'sleep-bonus interactivity' feature which was
removed by commit f3479f10c5d6 ("sched: remove the sleep-bonus
interactivity code")?
Dietmar Eggemann (3):
sched: Remove MAX_USER_RT_PRIO
sched: Remove USER_PRIO, TASK_USER_PRIO and MAX_USER_PRIO
sched/core: Update task_prio() function header
arch/powerpc/platforms/cell/spufs/sched.c | 2 +-
include/linux/sched/prio.h| 18 +-
kernel/sched/core.c | 15 +--
kernel/sched/sched.h | 2 +-
4 files changed, 12 insertions(+), 25 deletions(-)
--
2.25.1
[PATCH 2/3] sched: Remove USER_PRIO, TASK_USER_PRIO and MAX_USER_PRIO
The only remaining use of MAX_USER_PRIO (and USER_PRIO) is the
SCALE_PRIO() definition in the PowerPC Cell architecture's Synergistic
Processor Unit (SPU) scheduler. TASK_USER_PRIO isn't used anymore.
Commit fe443ef2ac42 ("[POWERPC] spusched: Dynamic timeslicing for
SCHED_OTHER") copied SCALE_PRIO() from the task scheduler in v2.6.23.
Commit a4ec24b48dde ("sched: tidy up SCHED_RR") removed it from the task
scheduler in v2.6.24.
Commit 3ee237dddcd8 ("sched/prio: Add 3 macros of MAX_NICE, MIN_NICE and
NICE_WIDTH in prio.h") introduced NICE_WIDTH much later.
With:
MAX_USER_PRIO = USER_PRIO(MAX_PRIO)
= MAX_PRIO - MAX_RT_PRIO
MAX_PRIO = MAX_RT_PRIO + NICE_WIDTH
MAX_USER_PRIO = MAX_RT_PRIO + NICE_WIDTH - MAX_RT_PRIO
MAX_USER_PRIO = NICE_WIDTH
MAX_USER_PRIO can be replaced by NICE_WIDTH to be able to remove all the
{*_}USER_PRIO defines.
Signed-off-by: Dietmar Eggemann
---
arch/powerpc/platforms/cell/spufs/sched.c | 2 +-
include/linux/sched/prio.h| 9 -
kernel/sched/sched.h | 2 +-
3 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/sched.c
b/arch/powerpc/platforms/cell/spufs/sched.c
index f18d5067cd0f..aeb7f3922106 100644
--- a/arch/powerpc/platforms/cell/spufs/sched.c
+++ b/arch/powerpc/platforms/cell/spufs/sched.c
@@ -72,7 +72,7 @@ static struct timer_list spuloadavg_timer;
#define DEF_SPU_TIMESLICE (100 * HZ / (1000 * SPUSCHED_TICK))
#define SCALE_PRIO(x, prio) \
- max(x * (MAX_PRIO - prio) / (MAX_USER_PRIO / 2), MIN_SPU_TIMESLICE)
+ max(x * (MAX_PRIO - prio) / (NICE_WIDTH / 2), MIN_SPU_TIMESLICE)
/*
* scale user-nice values [ -20 ... 0 ... 19 ] to time slice values:
diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h
index d111f2fd77ea..ab83d85e1183 100644
--- a/include/linux/sched/prio.h
+++ b/include/linux/sched/prio.h
@@ -26,15 +26,6 @@
#define NICE_TO_PRIO(nice) ((nice) + DEFAULT_PRIO)
#define PRIO_TO_NICE(prio) ((prio) - DEFAULT_PRIO)
-/*
- * 'User priority' is the nice value converted to something we
- * can work with better when scaling various scheduler parameters,
- * it's a [ 0 ... 39 ] range.
- */
-#define USER_PRIO(p) ((p)-MAX_RT_PRIO)
-#define TASK_USER_PRIO(p) USER_PRIO((p)->static_prio)
-#define MAX_USER_PRIO (USER_PRIO(MAX_PRIO))
-
/*
* Convert nice value [19,-20] to rlimit style value [1,40].
*/
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 045b01064c1e..6edc67df3554 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -140,7 +140,7 @@ extern void call_trace_sched_update_nr_running(struct rq
*rq, int count);
* scale_load() and scale_load_down(w) to convert between them. The
* following must be true:
*
- * scale_load(sched_prio_to_weight[USER_PRIO(NICE_TO_PRIO(0))]) == NICE_0_LOAD
+ * scale_load(sched_prio_to_weight[NICE_TO_PRIO(0)-MAX_RT_PRIO]) ==
NICE_0_LOAD
*
*/
#define NICE_0_LOAD(1L << NICE_0_LOAD_SHIFT)
--
2.25.1
[PATCH 3/3] sched/core: Update task_prio() function header
The description of the RT offset and the values for 'normal' tasks needs
update. Moreover there are DL tasks now.
task_prio() has to stay like it is to guarantee compatibility with the
/proc//stat priority field:
# cat /proc//stat | awk '{ print $18; }'
Signed-off-by: Dietmar Eggemann
---
kernel/sched/core.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 625ec1e12064..be3a956c2d23 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5602,8 +5602,12 @@ SYSCALL_DEFINE1(nice, int, increment)
* @p: the task in question.
*
* Return: The priority value as seen by users in /proc.
- * RT tasks are offset by -200. Normal tasks are centered
- * around 0, value goes from -16 to +15.
+ *
+ * sched policy return value kernel priouser prio/nice
+ *
+ * normal, batch, idle [0 ... 39] [100 ... 139] 0/[-20 ... 19]
+ * fifo, rr [-2 ... -100] [98 ... 0] [1 ... 99]
+ * deadline -101 -1 0
*/
int task_prio(const struct task_struct *p)
{
--
2.25.1
Re: [PATCH v4 02/10] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user()
Le 28/01/2021 à 11:38, David Laight a écrit :
From: Christopher M. Riedl
Sent: 28 January 2021 04:04
Reuse the "safe" implementation from signal.c except for calling
unsafe_copy_from_user() to copy into a local buffer.
Signed-off-by: Christopher M. Riedl
---
arch/powerpc/kernel/signal.h | 33 +
1 file changed, 33 insertions(+)
diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 2559a681536e..c18402d625f1 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task,
void __user *from);
[i], label);\
} while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do {\
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NFPREG];\
How big is that buffer?
#define ELF_NFPREG 33
So that's 264 bytes.
That's a bit big but still reasonable I think.
Christophe
Isn't is likely to be reasonably large compared to a reasonable
kernel stack frame.
Especially since this isn't even a leaf function.
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double),\
That really ought to be sizeof(buf).
David
+ label); \
+ for (i = 0; i < ELF_NFPREG - 1; i++) \
+ __t->thread.TS_FPR(i) = buf[i]; \
+ __t->thread.fp_state.fpscr = buf[i]; \
+} while (0)
+
+#define unsafe_copy_vsx_from_user(task, from, label) do {\
+ struct task_struct *__t = task; \
+ u64 __user *__f = (u64 __user *)from; \
+ u64 buf[ELF_NVSRHALFREG]; \
+ int i; \
+ \
+ unsafe_copy_from_user(buf, __f, \
+ ELF_NVSRHALFREG * sizeof(double), \
+ label); \
+ for (i = 0; i < ELF_NVSRHALFREG ; i++) \
+ __t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i]; \
+} while (0)
+
+
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
#define unsafe_copy_ckfpr_to_user(to, task, label)do {\
struct task_struct *__t = task; \
@@ -80,6 +107,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct
*task, void __user *from);
unsafe_copy_to_user(to, (task)->thread.fp_state.fpr, \
ELF_NFPREG * sizeof(double), label)
+#define unsafe_copy_fpr_from_user(task, from, label) \
+ unsafe_copy_from_user((task)->thread.fp_state.fpr, from, \
+ ELF_NFPREG * sizeof(double), label)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
@@ -115,6 +146,8 @@ copy_ckfpr_from_user(struct task_struct *task, void __user
*from)
#else
#define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
+#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
+
static inline unsigned long
copy_fpr_to_user(void __user *to, struct task_struct *task)
{
--
2.26.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH] ASoC: fsl_spdif: Utilize the defined parameter to clear code
Utilize the defined parameter 'dev' to make the code cleaner.
Signed-off-by: Tang Bin
---
sound/soc/fsl/fsl_spdif.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 455f96908..b6d5563df 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1215,7 +1215,7 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv
*spdif_priv,
for (i = 0; i < STC_TXCLK_SRC_MAX; i++) {
sprintf(tmp, "rxtx%d", i);
- clk = devm_clk_get(>dev, tmp);
+ clk = devm_clk_get(dev, tmp);
if (IS_ERR(clk)) {
dev_err(dev, "no rxtx%d clock in devicetree\n", i);
return PTR_ERR(clk);
@@ -1237,14 +1237,14 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv
*spdif_priv,
break;
}
- dev_dbg(>dev, "use rxtx%d as tx clock source for %dHz sample
rate\n",
+ dev_dbg(dev, "use rxtx%d as tx clock source for %dHz sample rate\n",
spdif_priv->txclk_src[index], rate[index]);
- dev_dbg(>dev, "use txclk df %d for %dHz sample rate\n",
+ dev_dbg(dev, "use txclk df %d for %dHz sample rate\n",
spdif_priv->txclk_df[index], rate[index]);
if (clk_is_match(spdif_priv->txclk[index], spdif_priv->sysclk))
- dev_dbg(>dev, "use sysclk df %d for %dHz sample rate\n",
+ dev_dbg(dev, "use sysclk df %d for %dHz sample rate\n",
spdif_priv->sysclk_df[index], rate[index]);
- dev_dbg(>dev, "the best rate for %dHz sample rate is %dHz\n",
+ dev_dbg(dev, "the best rate for %dHz sample rate is %dHz\n",
rate[index], spdif_priv->txrate[index]);
return 0;
--
2.20.1.windows.1
[PATCH v5 2/2] powerpc/mce: Remove per cpu variables from MCE handlers
Access to per-cpu variables requires translation to be enabled on
pseries machine running in hash mmu mode, Since part of MCE handler
runs in realmode and part of MCE handling code is shared between ppc
architectures pseries and powernv, it becomes difficult to manage
these variables differently on different architectures, So have
these variables in paca instead of having them as per-cpu variables
to avoid complications.
Signed-off-by: Ganesh Goudar
---
v2: Dynamically allocate memory for machine check event info.
v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid
to allocate memory.
v4: Spliting the patch into two.
v5: Fix build error for PPC32.
---
arch/powerpc/include/asm/mce.h | 18 +++
arch/powerpc/include/asm/paca.h| 4 ++
arch/powerpc/kernel/mce.c | 79 ++
arch/powerpc/kernel/setup-common.c | 2 +
4 files changed, 71 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 7d8b6679ec68..331d944280b8 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -206,6 +206,17 @@ struct mce_error_info {
#define MAX_MC_EVT 10
+struct mce_info {
+ int mce_nest_count;
+ struct machine_check_event mce_event[MAX_MC_EVT];
+ /* Queue for delayed MCE events. */
+ int mce_queue_count;
+ struct machine_check_event mce_event_queue[MAX_MC_EVT];
+ /* Queue for delayed MCE UE events. */
+ int mce_ue_count;
+ struct machine_check_event mce_ue_event_queue[MAX_MC_EVT];
+};
+
/* Release flags for get_mce_event() */
#define MCE_EVENT_RELEASE true
#define MCE_EVENT_DONTRELEASE false
@@ -234,4 +245,11 @@ long __machine_check_early_realmode_p8(struct pt_regs
*regs);
long __machine_check_early_realmode_p9(struct pt_regs *regs);
long __machine_check_early_realmode_p10(struct pt_regs *regs);
#endif /* CONFIG_PPC_BOOK3S_64 */
+
+#ifdef CONFIG_PPC_BOOK3S_64
+void mce_init(void);
+#else
+static inline void mce_init(void) { };
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
#endif /* __ASM_PPC64_MCE_H__ */
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 9454d29ff4b4..38e0c55e845d 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
@@ -273,6 +274,9 @@ struct paca_struct {
#ifdef CONFIG_MMIOWB
struct mmiowb_state mmiowb_state;
#endif
+#ifdef CONFIG_PPC_BOOK3S_64
+ struct mce_info *mce_info;
+#endif /* CONFIG_PPC_BOOK3S_64 */
} cacheline_aligned;
extern void copy_mm_to_paca(struct mm_struct *mm);
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 9f3e133b57b7..6ec5c68997ed 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -17,22 +17,13 @@
#include
#include
#include
+#include
#include
#include
#include
-static DEFINE_PER_CPU(int, mce_nest_count);
-static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT], mce_event);
-
-/* Queue for delayed MCE events. */
-static DEFINE_PER_CPU(int, mce_queue_count);
-static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT], mce_event_queue);
-
-/* Queue for delayed MCE UE events. */
-static DEFINE_PER_CPU(int, mce_ue_count);
-static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT],
- mce_ue_event_queue);
+#include "setup.h"
static void machine_check_process_queued_event(struct irq_work *work);
static void machine_check_ue_irq_work(struct irq_work *work);
@@ -103,9 +94,10 @@ void save_mce_event(struct pt_regs *regs, long handled,
struct mce_error_info *mce_err,
uint64_t nip, uint64_t addr, uint64_t phys_addr)
{
- int index = __this_cpu_inc_return(mce_nest_count) - 1;
- struct machine_check_event *mce = this_cpu_ptr(_event[index]);
+ int index = local_paca->mce_info->mce_nest_count++;
+ struct machine_check_event *mce;
+ mce = _paca->mce_info->mce_event[index];
/*
* Return if we don't have enough space to log mce event.
* mce_nest_count may go beyond MAX_MC_EVT but that's ok,
@@ -191,7 +183,7 @@ void save_mce_event(struct pt_regs *regs, long handled,
*/
int get_mce_event(struct machine_check_event *mce, bool release)
{
- int index = __this_cpu_read(mce_nest_count) - 1;
+ int index = local_paca->mce_info->mce_nest_count - 1;
struct machine_check_event *mc_evt;
int ret = 0;
@@ -201,7 +193,7 @@ int get_mce_event(struct machine_check_event *mce, bool
release)
/* Check if we have MCE info to process. */
if (index < MAX_MC_EVT) {
- mc_evt = this_cpu_ptr(_event[index]);
+ mc_evt = _paca->mce_info->mce_event[index];
/* Copy the event structure and release the original */
if (mce)
*mce =
[PATCH v5 1/2] powerpc/mce: Reduce the size of event arrays
Maximum recursive depth of MCE is 4, Considering the maximum depth
allowed reduce the size of event to 10 from 100. This saves us ~19kB
of memory and has no fatal consequences.
Signed-off-by: Ganesh Goudar
---
v4: This patch is a fragment of the orignal patch which is
split into two.
v5: No changes.
---
arch/powerpc/include/asm/mce.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index e6c27ae843dc..7d8b6679ec68 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -204,7 +204,7 @@ struct mce_error_info {
boolignore_event;
};
-#define MAX_MC_EVT 100
+#define MAX_MC_EVT 10
/* Release flags for get_mce_event() */
#define MCE_EVENT_RELEASE true
--
2.26.2
RE: [PATCH v4 02/10] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user()
From: Christopher M. Riedl
> Sent: 28 January 2021 04:04
>
> Reuse the "safe" implementation from signal.c except for calling
> unsafe_copy_from_user() to copy into a local buffer.
>
> Signed-off-by: Christopher M. Riedl
> ---
> arch/powerpc/kernel/signal.h | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
> index 2559a681536e..c18402d625f1 100644
> --- a/arch/powerpc/kernel/signal.h
> +++ b/arch/powerpc/kernel/signal.h
> @@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct task_struct
> *task, void __user *from);
> [i], label);\
> } while (0)
>
> +#define unsafe_copy_fpr_from_user(task, from, label) do {\
> + struct task_struct *__t = task; \
> + u64 __user *__f = (u64 __user *)from; \
> + u64 buf[ELF_NFPREG];\
How big is that buffer?
Isn't is likely to be reasonably large compared to a reasonable
kernel stack frame.
Especially since this isn't even a leaf function.
> + int i; \
> + \
> + unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double),\
That really ought to be sizeof(buf).
David
> + label); \
> + for (i = 0; i < ELF_NFPREG - 1; i++)\
> + __t->thread.TS_FPR(i) = buf[i]; \
> + __t->thread.fp_state.fpscr = buf[i];\
> +} while (0)
> +
> +#define unsafe_copy_vsx_from_user(task, from, label) do {\
> + struct task_struct *__t = task; \
> + u64 __user *__f = (u64 __user *)from; \
> + u64 buf[ELF_NVSRHALFREG]; \
> + int i; \
> + \
> + unsafe_copy_from_user(buf, __f, \
> + ELF_NVSRHALFREG * sizeof(double), \
> + label); \
> + for (i = 0; i < ELF_NVSRHALFREG ; i++) \
> + __t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i]; \
> +} while (0)
> +
> +
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> #define unsafe_copy_ckfpr_to_user(to, task, label) do {\
> struct task_struct *__t = task; \
> @@ -80,6 +107,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct
> *task, void __user *from);
> unsafe_copy_to_user(to, (task)->thread.fp_state.fpr,\
> ELF_NFPREG * sizeof(double), label)
>
> +#define unsafe_copy_fpr_from_user(task, from, label) \
> + unsafe_copy_from_user((task)->thread.fp_state.fpr, from,\
> + ELF_NFPREG * sizeof(double), label)
> +
> static inline unsigned long
> copy_fpr_to_user(void __user *to, struct task_struct *task)
> {
> @@ -115,6 +146,8 @@ copy_ckfpr_from_user(struct task_struct *task, void
> __user *from)
> #else
> #define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
>
> +#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
> +
> static inline unsigned long
> copy_fpr_to_user(void __user *to, struct task_struct *task)
> {
> --
> 2.26.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
* Geert Uytterhoeven [210128 09:32]: > It wasn't. The regression is that the driver no longer probes at first > try, because its dependencies are now probed later. The question is: > why are the dependencies now probed later? How to fix that? I'm afraid that may be unfixable.. It depends on things like the bus driver probe that might get also deferred. As suggested, I agree it's best to get rid of builtin_platform_driver_probe where possible at the cost of dropping the __init as needed. To me it seems we can't even add a warning to __platform_driver_probe() if there's drv->driver.of_match_table for example. That warning would show up on all the devices with driver in question built in even if the device has no such hardware. Regards, Tony
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
Hi, * Michael Walle [210125 19:52]: > Although I do have the changes for the builtin_platform_driver_probe() > ready, I don't think it makes much sense to send these unless we agree > on the increased memory footprint. While there are just a few > builtin_platform_driver_probe() and memory increase _might_ be > negligible, there are many more module_platform_driver_probe(). I just noticed this thread today and have pretty much come to the same conclusions. No need to post a patch for pci-dra7xx.c, I already posted a patch for pci-dra7xx.c yesterday as part of genpd related changes. For me probing started breaking as the power-domains property got added. FYI, it's the following patch: [PATCH 01/15] PCI: pci-dra7xx: Prepare for deferred probe with module_platform_driver Regards, Tony
Re: [PATCH v4 2/2] powerpc/mce: Remove per cpu variables from MCE handlers
On 1/25/21 2:54 PM, Christophe Leroy wrote: Le 22/01/2021 à 13:32, Ganesh Goudar a écrit : Access to per-cpu variables requires translation to be enabled on pseries machine running in hash mmu mode, Since part of MCE handler runs in realmode and part of MCE handling code is shared between ppc architectures pseries and powernv, it becomes difficult to manage these variables differently on different architectures, So have these variables in paca instead of having them as per-cpu variables to avoid complications. Signed-off-by: Ganesh Goudar --- v2: Dynamically allocate memory for machine check event info v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid to allocate memory. v4: Spliting the patch into two. --- arch/powerpc/include/asm/mce.h | 18 +++ arch/powerpc/include/asm/paca.h | 4 ++ arch/powerpc/kernel/mce.c | 79 ++ arch/powerpc/kernel/setup-common.c | 2 +- 4 files changed, 70 insertions(+), 33 deletions(-) diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 71f38e9248be..17dc451f0e45 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -916,7 +916,6 @@ void __init setup_arch(char **cmdline_p) /* On BookE, setup per-core TLB data structures. */ setup_tlb_core_data(); #endif - This line removal is really required for this patch ? I will correct it, Thanks for catching. /* Print various info about the machine that has been gathered so far. */ print_system_info(); @@ -938,6 +937,7 @@ void __init setup_arch(char **cmdline_p) exc_lvl_early_init(); emergency_stack_init(); + mce_init(); You have to include mce.h to avoid build failure on PPC32. Sure, thanks smp_release_cpus(); initmem_init();
Re: [PATCH v6 08/39] powerpc: rearrange do_page_fault error case to be inside exception_enter
Le 15/01/2021 à 17:49, Nicholas Piggin a écrit :
This keeps the context tracking over the entire interrupt handler which
helps later with moving context tracking into interrupt wrappers.
Signed-off-by: Nicholas Piggin
---
arch/powerpc/mm/fault.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index e476d7701413..e4121fd9fcf1 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -544,20 +544,24 @@ NOKPROBE_SYMBOL(__do_page_fault);
long do_page_fault(struct pt_regs *regs)
{
- const struct exception_table_entry *entry;
- enum ctx_state prev_state = exception_enter();
- int rc = __do_page_fault(regs, regs->dar, regs->dsisr);
- exception_exit(prev_state);
- if (likely(!rc))
- return 0;
-
- entry = search_exception_tables(regs->nip);
- if (unlikely(!entry))
- return rc;
Could we keep this layout with using a 'goto' to the end of the function, instead of pushing error
handling to the right ?
Because at the end of the series once all context tracking is gone into helpers, the result looks
unfriendly.
It would look cleaner as:
static long __do_page_fault(struct pt_regs *regs)
{
long err;
const struct exception_table_entry *entry;
err = ___do_page_fault(regs, regs->dar, regs->dsisr);
if (likely(!err))
return 0;
entry = search_exception_tables(regs->nip);
if (likely(entry)) {
instruction_pointer_set(regs, extable_fixup(entry));
return 0;
} else if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64)) {
/* 32 and 64e handle this in asm */
return err;
}
__bad_page_fault(regs, err);
return 0;
}
NOKPROBE_SYMBOL(__do_page_fault);
+ enum ctx_state prev_state;
+ long err;
+
+ prev_state = exception_enter();
+ err = __do_page_fault(regs, regs->dar, regs->dsisr);
+ if (unlikely(err)) {
+ const struct exception_table_entry *entry;
+
+ entry = search_exception_tables(regs->nip);
+ if (likely(entry)) {
+ instruction_pointer_set(regs, extable_fixup(entry));
+ err = 0;
+ }
+ }
- instruction_pointer_set(regs, extable_fixup(entry));
+ exception_exit(prev_state);
- return 0;
+ return err;
}
NOKPROBE_SYMBOL(do_page_fault);
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
Hi Saravana, On Wed, Jan 27, 2021 at 6:11 PM Saravana Kannan wrote: > On Wed, Jan 27, 2021 at 8:56 AM Geert Uytterhoeven > wrote: > > On Wed, Jan 27, 2021 at 5:42 PM Saravana Kannan > > wrote: > > > On Tue, Jan 26, 2021 at 11:43 PM Geert Uytterhoeven > > > wrote: > > > > On Wed, Jan 27, 2021 at 1:44 AM Saravana Kannan > > > > wrote: > > > > > On Tue, Jan 26, 2021 at 12:50 AM Geert Uytterhoeven > > > > > wrote: > > > > > > On Mon, Jan 25, 2021 at 11:42 PM Saravana Kannan > > > > > > wrote: > > > > > > > On Mon, Jan 25, 2021 at 11:49 AM Michael Walle > > > > > > > wrote: > > > > > > > > Am 2021-01-21 12:01, schrieb Geert Uytterhoeven: > > > > > > > > > On Thu, Jan 21, 2021 at 1:05 AM Saravana Kannan > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > >> On Wed, Jan 20, 2021 at 3:53 PM Michael Walle > > > > > > > > >> > > > > > > > > >> wrote: > > > > > > > > >> > Am 2021-01-20 20:47, schrieb Saravana Kannan: > > > > > > > > >> > > On Wed, Jan 20, 2021 at 11:28 AM Michael Walle > > > > > > > > >> > > > > > > > > > > >> > > wrote: > > > > > > > > >> > >> > > > > > > > > >> > >> [RESEND, fat-fingered the buttons of my mail client and > > > > > > > > >> > >> converted > > > > > > > > >> > >> all CCs to BCCs :(] > > > > > > > > >> > >> > > > > > > > > >> > >> Am 2021-01-20 20:02, schrieb Saravana Kannan: > > > > > > > > >> > >> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring > > > > > > > > >> > >> > wrote: > > > > > > > > >> > >> >> > > > > > > > > >> > >> >> On Wed, Jan 20, 2021 at 4:53 AM Michael Walle > > > > > > > > >> > >> >> > > > > > > > > >> > >> >> wrote: > > > > > > > > >> > >> >> > > > > > > > > > >> > >> >> > fw_devlink will defer the probe until all > > > > > > > > >> > >> >> > suppliers are ready. We can't > > > > > > > > >> > >> >> > use builtin_platform_driver_probe() because it > > > > > > > > >> > >> >> > doesn't retry after probe > > > > > > > > >> > >> >> > deferral. Convert it to builtin_platform_driver(). > > > > > > > > >> > >> >> > > > > > > > > >> > >> >> If builtin_platform_driver_probe() doesn't work with > > > > > > > > >> > >> >> fw_devlink, then > > > > > > > > >> > >> >> shouldn't it be fixed or removed? > > > > > > > > >> > >> > > > > > > > > > >> > >> > I was actually thinking about this too. The problem > > > > > > > > >> > >> > with fixing > > > > > > > > >> > >> > builtin_platform_driver_probe() to behave like > > > > > > > > >> > >> > builtin_platform_driver() is that these probe > > > > > > > > >> > >> > functions could be > > > > > > > > >> > >> > marked with __init. But there are also only 20 > > > > > > > > >> > >> > instances of > > > > > > > > >> > >> > builtin_platform_driver_probe() in the kernel: > > > > > > > > >> > >> > $ git grep ^builtin_platform_driver_probe | wc -l > > > > > > > > >> > >> > 20 > > > > > > > > >> > >> > > > > > > > > > >> > >> > So it might be easier to just fix them to not use > > > > > > > > >> > >> > builtin_platform_driver_probe(). > > > > > > > > >> > >> > > > > > > > > > >> > >> > Michael, > > > > > > > > >> > >> > > > > > > > > > >> > >> > Any chance you'd be willing to help me by converting > > > > > > > > >> > >> > all these to > > > > > > > > >> > >> > builtin_platform_driver() and delete > > > > > > > > >> > >> > builtin_platform_driver_probe()? > > > > > > > > >> > >> > > > > > > > > >> > >> If it just moving the probe function to the _driver > > > > > > > > >> > >> struct and > > > > > > > > >> > >> remove the __init annotations. I could look into that. > > > > > > > > >> > > > > > > > > > > >> > > Yup. That's pretty much it AFAICT. > > > > > > > > >> > > > > > > > > > > >> > > builtin_platform_driver_probe() also makes sure the > > > > > > > > >> > > driver doesn't ask > > > > > > > > >> > > for async probe, etc. But I doubt anyone is actually > > > > > > > > >> > > setting async > > > > > > > > >> > > flags and still using builtin_platform_driver_probe(). > > > > > > > > >> > > > > > > > > > >> > Hasn't module_platform_driver_probe() the same problem? > > > > > > > > >> > And there > > > > > > > > >> > are ~80 drivers which uses that. > > > > > > > > >> > > > > > > > > >> Yeah. The biggest problem with all of these is the __init > > > > > > > > >> markers. > > > > > > > > >> Maybe some familiar with coccinelle can help? > > > > > > > > > > > > > > > > > > And dropping them will increase memory usage. > > > > > > > > > > > > > > > > Although I do have the changes for the > > > > > > > > builtin_platform_driver_probe() > > > > > > > > ready, I don't think it makes much sense to send these unless > > > > > > > > we agree > > > > > > > > on the increased memory footprint. While there are just a few > > > > > > > > builtin_platform_driver_probe() and memory increase _might_ be > > > > > > > > negligible, there are many more module_platform_driver_probe(). > > > > > > > > > > > > > > While it's good to drop code that'll not be used past kernel > > > > > > > init, the > > > > > > >
Re: [PATCH 02/27] x86/syscalls: fix -Wmissing-prototypes warnings from COND_SYSCALL()
Hello! On 28.01.2021 3:50, Masahiro Yamada wrote: Building kernel/sys_ni.c with W=1 omits tons of -Wmissing-prototypes Emits? warnings. $ make W=1 kernel/sys_ni.o [ snip ] CC kernel/sys_ni.o In file included from kernel/sys_ni.c:10: ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__x64_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:100:2: note: in expansion of macro '__COND_SYSCALL' 100 | __COND_SYSCALL(x64, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:256:2: note: in expansion of macro '__X64_COND_SYSCALL' 256 | __X64_COND_SYSCALL(name) \ | ^~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__ia32_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:120:2: note: in expansion of macro '__COND_SYSCALL' 120 | __COND_SYSCALL(ia32, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:257:2: note: in expansion of macro '__IA32_COND_SYSCALL' 257 | __IA32_COND_SYSCALL(name) | ^~~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ... __SYS_STUB0() and __SYS_STUBx() defined a few lines above have forward declarations. Let's do likewise for __COND_SYSCALL() to fix the warnings. Signed-off-by: Masahiro Yamada --- arch/x86/include/asm/syscall_wrapper.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h index a84333adeef2..80c08c7d5e72 100644 --- a/arch/x86/include/asm/syscall_wrapper.h +++ b/arch/x86/include/asm/syscall_wrapper.h @@ -80,6 +80,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs); } #define __COND_SYSCALL(abi, name) \ + __weak long __##abi##_##name(const struct pt_regs *__unused); \ __weak long __##abi##_##name(const struct pt_regs *__unused)\ Aren't these two lines identical? [...] MBR, Sergei
Re: [PATCH 02/27] x86/syscalls: fix -Wmissing-prototypes warnings from COND_SYSCALL()
On 28.01.2021 10:59, Sergei Shtylyov wrote: [...] Building kernel/sys_ni.c with W=1 omits tons of -Wmissing-prototypes Emits? warnings. $ make W=1 kernel/sys_ni.o [ snip ] CC kernel/sys_ni.o In file included from kernel/sys_ni.c:10: ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__x64_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:100:2: note: in expansion of macro '__COND_SYSCALL' 100 | __COND_SYSCALL(x64, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:256:2: note: in expansion of macro '__X64_COND_SYSCALL' 256 | __X64_COND_SYSCALL(name) \ | ^~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ./arch/x86/include/asm/syscall_wrapper.h:83:14: warning: no previous prototype for '__ia32_sys_io_setup' [-Wmissing-prototypes] 83 | __weak long __##abi##_##name(const struct pt_regs *__unused) \ | ^~ ./arch/x86/include/asm/syscall_wrapper.h:120:2: note: in expansion of macro '__COND_SYSCALL' 120 | __COND_SYSCALL(ia32, sys_##name) | ^~ ./arch/x86/include/asm/syscall_wrapper.h:257:2: note: in expansion of macro '__IA32_COND_SYSCALL' 257 | __IA32_COND_SYSCALL(name) | ^~~ kernel/sys_ni.c:39:1: note: in expansion of macro 'COND_SYSCALL' 39 | COND_SYSCALL(io_setup); | ^~~~ ... __SYS_STUB0() and __SYS_STUBx() defined a few lines above have forward declarations. Let's do likewise for __COND_SYSCALL() to fix the warnings. Signed-off-by: Masahiro Yamada --- arch/x86/include/asm/syscall_wrapper.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h index a84333adeef2..80c08c7d5e72 100644 --- a/arch/x86/include/asm/syscall_wrapper.h +++ b/arch/x86/include/asm/syscall_wrapper.h @@ -80,6 +80,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs); } #define __COND_SYSCALL(abi, name) \ + __weak long __##abi##_##name(const struct pt_regs *__unused); \ __weak long __##abi##_##name(const struct pt_regs *__unused) \ Aren't these two lines identical? Ah, got it! :-) [...] MBR, Sergei
Re: [PATCH 12/27] m68k: syscalls: switch to generic syscalltbl.sh
Hi Yamada-san,
On Thu, Jan 28, 2021 at 1:54 AM Masahiro Yamada wrote:
> As of v5.11-rc1, 12 architectures duplicate similar shell scripts in
> order to generate syscall table headers. My goal is to unify them into
> the single scripts/syscalltbl.sh.
>
> This commit converts m68k to use scripts/syscalltbl.sh.
>
> Signed-off-by: Masahiro Yamada
Thanks a lot!
Tested-by: Geert Uytterhoeven
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
