Re: [PATCH v5 RESEND] arch/powerpc/kvm: Add support for reading VPA counters for pseries guests
On Mon, Apr 22, 2024 at 09:15:02PM +0530, Naveen N Rao wrote:
> On Tue, Apr 02, 2024 at 12:36:54PM +0530, Gautam Menghani wrote:
> > PAPR hypervisor has introduced three new counters in the VPA area of
> > LPAR CPUs for KVM L2 guest (see [1] for terminology) observability - 2
> > for context switches from host to guest and vice versa, and 1 counter
> > for getting the total time spent inside the KVM guest. Add a tracepoint
> > that enables reading the counters for use by ftrace/perf. Note that this
> > tracepoint is only available for nestedv2 API (i.e, KVM on PowerVM).
> >
> > Also maintain an aggregation of the context switch times in vcpu->arch.
> > This will be useful in getting the aggregate times with a pmu driver
> > which will be upstreamed in the near future.
>
> It would be better to add code to maintain aggregate times as part of
> that pmu driver.
>
> >
> > [1] Terminology:
> > a. L1 refers to the VM (LPAR) booted on top of PAPR hypervisor
> > b. L2 refers to the KVM guest booted on top of L1.
> >
> > Signed-off-by: Vaibhav Jain
> > Signed-off-by: Gautam Menghani
> > ---
> > v5 RESEND:
> > 1. Add the changelog
> >
> > v4 -> v5:
> > 1. Define helper functions for getting/setting the accumulation counter
> > in L2's VPA
> >
> > v3 -> v4:
> > 1. After vcpu_run, check the VPA flag instead of checking for tracepoint
> > being enabled for disabling the cs time accumulation.
> >
> > v2 -> v3:
> > 1. Move the counter disabling and zeroing code to a different function.
> > 2. Move the get_lppaca() inside the tracepoint_enabled() branch.
> > 3. Add the aggregation logic to maintain total context switch time.
> >
> > v1 -> v2:
> > 1. Fix the build error due to invalid struct member reference.
> >
> > arch/powerpc/include/asm/kvm_host.h | 5
> > arch/powerpc/include/asm/lppaca.h | 11 +---
> > arch/powerpc/kvm/book3s_hv.c| 40 +
> > arch/powerpc/kvm/trace_hv.h | 25 ++
> > 4 files changed, 78 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_host.h
> > b/arch/powerpc/include/asm/kvm_host.h
> > index 8abac532146e..d953b32dd68a 100644
> > --- a/arch/powerpc/include/asm/kvm_host.h
> > +++ b/arch/powerpc/include/asm/kvm_host.h
> > @@ -847,6 +847,11 @@ struct kvm_vcpu_arch {
> > gpa_t nested_io_gpr;
> > /* For nested APIv2 guests*/
> > struct kvmhv_nestedv2_io nestedv2_io;
> > +
> > + /* Aggregate context switch and guest run time info (in ns) */
> > + u64 l1_to_l2_cs_agg;
> > + u64 l2_to_l1_cs_agg;
> > + u64 l2_runtime_agg;
>
> Can be dropped from this patch.
>
> > #endif
> >
> > #ifdef CONFIG_KVM_BOOK3S_HV_EXIT_TIMING
> > diff --git a/arch/powerpc/include/asm/lppaca.h
> > b/arch/powerpc/include/asm/lppaca.h
> > index 61ec2447dabf..bda6b86b9f13 100644
> > --- a/arch/powerpc/include/asm/lppaca.h
> > +++ b/arch/powerpc/include/asm/lppaca.h
> > @@ -62,7 +62,8 @@ struct lppaca {
> > u8 donate_dedicated_cpu; /* Donate dedicated CPU cycles */
> > u8 fpregs_in_use;
> > u8 pmcregs_in_use;
> > - u8 reserved8[28];
> > + u8 l2_accumul_cntrs_enable; /* Enable usage of counters for KVM
> > guest */
>
> A simpler name - l2_counters_enable or such?
>
> > + u8 reserved8[27];
> > __be64 wait_state_cycles; /* Wait cycles for this proc */
> > u8 reserved9[28];
> > __be16 slb_count; /* # of SLBs to maintain */
> > @@ -92,9 +93,13 @@ struct lppaca {
> > /* cacheline 4-5 */
> >
> > __be32 page_ins; /* CMO Hint - # page ins by OS */
> > - u8 reserved12[148];
> > + u8 reserved12[28];
> > + volatile __be64 l1_to_l2_cs_tb;
> > + volatile __be64 l2_to_l1_cs_tb;
> > + volatile __be64 l2_runtime_tb;
> > + u8 reserved13[96];
> > volatile __be64 dtl_idx;/* Dispatch Trace Log head index */
> > - u8 reserved13[96];
> > + u8 reserved14[96];
> > } cacheline_aligned;
> >
> > #define lppaca_of(cpu) (*paca_ptrs[cpu]->lppaca_ptr)
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 8e86eb577eb8..fea1c1429975 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -4108,6 +4108,37 @@ static void vcpu_vpa_increment_dispatch(struct
> > kvm_vcpu *vcpu)
> > }
> > }
> >
> > +static inline int kvmhv_get_l2_accumul(void)
> > +{
> > + return get_lppaca()->l2_accumul_cntrs_enable;
> > +}
> > +
> > +static inline void kvmhv_set_l2_accumul(int val)
> ^^^
> bool?
>
> > +{
> > + get_lppaca()->l2_accumul_cntrs_enable = val;
> > +}
> > +
> > +static void do_trace_nested_cs_time(struct kvm_vcpu *vcpu)
> > +{
> > + struct lppaca *lp = get_lppaca();
> > + u64 l1_to_l2_ns, l2_to_l1_ns, l2_runtime_ns;
> > +
> > + l1_to_l2_ns = tb_to_ns(be64_to_cpu(lp->l1_to_l2_cs_tb));
> > +
Re: Please add powerpc topic/kdump-hotplug branch to linux-next
Hi Michael, On Tue, 23 Apr 2024 23:56:42 +1000 Michael Ellerman wrote: > \> Can you please add the topic/kdump-hotplug branch of the powerpc tree to > linux-next. It contains a series that touches generic kexec code as well > as x86 and powerpc code. > > The hope is to have to get it merged for v6.10, so it should go along > with the powerpc next branch in the merge order. > > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/log/?h=topic/kdump-hotplug Added from today. Thanks for adding your subsystem tree as a participant of linux-next. As you may know, this is not a judgement of your code. The purpose of linux-next is for integration testing and to lower the impact of conflicts between subsystems in the next merge window. You will need to ensure that the patches/commits in your tree/series have been: * submitted under GPL v2 (or later) and include the Contributor's Signed-off-by, * posted to the relevant mailing list, * reviewed by you (or another maintainer of your subsystem tree), * successfully unit tested, and * destined for the current or next Linux merge window. Basically, this should be just what you would send to Linus (or ask him to fetch). It is allowed to be rebased if you deem it necessary. -- Cheers, Stephen Rothwell s...@canb.auug.org.au -- Cheers, Stephen Rothwell pgpJEy4ogqcZM.pgp Description: OpenPGP digital signature
[linux-next:master] BUILD REGRESSION a59668a9397e7245b26e9be85d23f242ff757ae8
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: a59668a9397e7245b26e9be85d23f242ff757ae8 Add linux-next specific files for 20240423 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202404231839.ohiy9lw8-...@intel.com Error/Warning: (recently discovered and may have been fixed) WARNING: modpost: vmlinux: section mismatch in reference: __io_uring_register+0x4e (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: add_master_key+0xcc (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: address_val+0x172 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: affine_move_task+0x7c (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: bcma_pmu_init+0x11c (section: .text) -> .LBE2162 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: bitmap_list_string.isra.0+0x176 (section: .text) -> .L498 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: bpf_cgroup_link_fill_link_info+0x18 (section: .text) -> .LBE2191 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: bpf_map_meta_alloc+0x68 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: bstr_printf+0x4c (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: cgroup1_reconfigure+0x6a (section: .text) -> .LBE2191 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: cgroup_bpf_replace+0x18 (section: .text) -> .LBE2191 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: class_srcu_destructor.isra.0+0x44 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: cpu_cluster_pm_enter+0x36 (section: .text) -> .LBB2191 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: dma_direct_mmap+0xf2 (section: .text) -> .LVL129 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: drm_atomic_helper_check_plane_state+0x80 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x228 (section: .text) -> .LBE1217 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: drm_of_component_probe+0xec (section: .text) -> .LVL1112 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: drm_test_rect_rotate_inv+0x3c (section: .text) -> .LVL1139 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: dw_hdmi_bridge_atomic_get_output_bus_fmts+0x80 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiod_direction_input.part.0+0x2a (section: .text) -> .LVL1117 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiod_direction_output_raw_commit+0x40 (section: .text) -> .LVL1117 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiod_find_and_request+0x4e (section: .text) -> .LVL1117 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiod_get_direction+0x4e (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiod_to_irq+0x48 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiolib_seq_start+0x32 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: gpiolib_seq_stop+0x4e (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: handle_new_recv_msgs+0x23a (section: .text) -> .LVL1117 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: iio_compute_scan_bytes+0x5e (section: .text) -> .LVL1049 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: iio_dev_attr_in_accel_scale_available+0x14 (section: .data) -> .LVL793 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: ipmi_get_maintenance_mode+0x8a (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: ipmi_get_my_LUN+0x82 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: ipmi_get_smi_info+0x32 (section: .text) -> .LVL1117 (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: ipmi_set_my_address+0x82 (section: .text) -> .L443 (section: .init.text) WARNING: modpost: vmlinux:
Re: [PATCH v5 00/15] mm: jit/text allocator
On Mon, Apr 22, 2024 at 12:44:21PM +0300, Mike Rapoport wrote: > From: "Mike Rapoport (IBM)" > > (something went wrong with the prevois posting, sorry for the noise) > > Hi, > > Since v3 I looked into making execmem more of an utility toolbox, as we > discussed at LPC with Mark Rutland, but it was getting more hairier than > having a struct describing architecture constraints and a type identifying > the consumer of execmem. > > And I do think that having the description of architecture constraints for > allocations of executable memory in a single place is better than having it > spread all over the place. > > The patches available via git: > https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git/log/?h=execmem/v5 Thanks! I've merged and pushed this onto modules-next in its entirety now for wider testing. Luis
Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
On Tue, Apr 23, 2024 at 09:54:35AM +0200, Thomas Weißschuh wrote: > * Patch 1 is a bugfix for the stack_erasing sysctl handler > * Patches 2-10 change various helper functions throughout the kernel to > be able to handle 'const ctl_table'. > * Patch 11 changes the signatures of all proc handlers through the tree. > Some other signatures are also adapted, for details see the commit > message. > > Only patch 1 changes any code at all. > > The series was compile-tested on top of next-20230423 for > i386, x86_64, arm, arm64, riscv, loongarch, s390 and m68k. > > The series was split from my larger series sysctl-const series [0]. > It only focusses on the proc_handlers but is an important step to be > able to move all static definitions of ctl_table into .rodata. > > [0] > https://lore.kernel.org/lkml/20231204-const-sysctl-v2-0-7a5060b11...@weissschuh.net/ > > Signed-off-by: Thomas Weißschuh Cover letters don't need SOBS we only use them for patches. But anyway: Reviewed-by: Luis Chamberlain Luis
Re: [PATCH v5 04/15] sparc: simplify module_alloc()
Hi Mike, On Mon, Apr 22, 2024 at 12:44:25PM +0300, Mike Rapoport wrote: > From: "Mike Rapoport (IBM)" > > Define MODULES_VADDR and MODULES_END as VMALLOC_START and VMALLOC_END > for 32-bit and reduce module_alloc() to > > __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, ...) > > as with the new defines the allocations becames identical for both 32 > and 64 bits. > > While on it, drop unsed include of > > Suggested-by: Sam Ravnborg > Signed-off-by: Mike Rapoport (IBM) Looks good. Reviewed-by: Sam Ravnborg
Re: [PATCH] ibmvnic: Use -EBUSY in __ibmvnic_reset()
On Tue, Apr 23, 2024 at 09:55:57AM -0500, Nick Child wrote:
> > You're right that it doesn't affect the behavior of the driver except
> > for the debug output when we do:
> >
> > netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc);
> >
> > But the - was left off uninitentionally so I think we should apply it.
> >
> > I have been trying to look for similar bugs where the - is left off.
> > It's a bit challenging because there places where we use positive
> > error codes deliberately. But in this case a static checker could
> > easily detect the bug with a low false positive ratio by saying, "We're
> > mixing normal negative error codes with positive EBUSY".
> >
> > regards,
> > dan carpenter
>
> Hello, small clarification, this patch will not effect the debug print
> statement due to the break statement immediately following:
> while () {
> if () {
> rc = -EBUSY;
> break;
> }
> netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc);
> }
> Though this return code can be passed to adapter->reset_done_rc, which is
> only treated as a boolean.
>
> So, the point of the patch not doing any behavioral differences is still
> true.
Ah yes. You're right.
regards,
dan carpenter
Re: [PATCH] ibmvnic: Use -EBUSY in __ibmvnic_reset()
On 4/23/24 06:55, Dan Carpenter wrote:
On Tue, Apr 23, 2024 at 12:54:55PM +0200, Paolo Abeni wrote:
On Fri, 2024-04-19 at 16:08 +0200, Markus Elfring wrote:
From: Markus Elfring
Date: Fri, 19 Apr 2024 15:46:17 +0200
Add a minus sign before the error code “EBUSY”
so that a negative value will be used as in other cases.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5e9a93bdb518..737ae83a836a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3212,7 +3212,7 @@ static void __ibmvnic_reset(struct work_struct *work)
adapter->state == VNIC_REMOVED) {
spin_unlock_irqrestore(>state_lock, flags);
kfree(rwi);
- rc = EBUSY;
+ rc = -EBUSY;
break;
AFAICS the error is always used as bool, so this will not change any
behavior in practice. I tend to think we should not merge this kind of
change outside some larger work in the same area, but I'd love a second
opinion from the driver owners.
I missed the original patch due to my procmail filters...
You're right that it doesn't affect the behavior of the driver except
for the debug output when we do:
netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc);
But the - was left off uninitentionally so I think we should apply it.
I have been trying to look for similar bugs where the - is left off.
It's a bit challenging because there places where we use positive
error codes deliberately. But in this case a static checker could
easily detect the bug with a low false positive ratio by saying, "We're
mixing normal negative error codes with positive EBUSY".
regards,
dan carpenter
Hello, small clarification, this patch will not effect the debug print
statement due to the break statement immediately following:
while () {
if () {
rc = -EBUSY;
break;
}
netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc);
}
Though this return code can be passed to adapter->reset_done_rc, which
is only treated as a boolean.
So, the point of the patch not doing any behavioral differences is still
true.
Personally, I don't have strong opinions on this.
Reviewed-by: Nick Child
Re: [PATCH v2] ASoC: dt-bindings: fsl,ssi: Convert to YAML
On Mon, Apr 22, 2024 at 01:48:14PM +0800, Shengjiu Wang wrote: > Convert the fsl,ssi binding to YAML. > > Add below compatible strings which were not listed > in document: > > fsl,imx50-ssi > fsl,imx53-ssi > fsl,imx25-ssi > fsl,imx27-ssi > fsl,imx6q-ssi > fsl,imx6sl-ssi > fsl,imx6sx-ssi > > Add below fsl,mode strings which were not listed. > > i2s-slave > i2s-master > lj-slave > lj-master > rj-slave > rj-master > > Add 'ac97-gpios' property which were not listed. > Then dtbs_check can pass. > > And remove the 'codec' description which should be > in the 'codec' binding doc. > > Signed-off-by: Shengjiu Wang > --- > changes in v2: > - change fallback string to const. > - add dai-common.yaml > - add ac97-gpios property > > .../devicetree/bindings/sound/fsl,ssi.txt | 87 > .../devicetree/bindings/sound/fsl,ssi.yaml| 192 ++ > 2 files changed, 192 insertions(+), 87 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/sound/fsl,ssi.txt > create mode 100644 Documentation/devicetree/bindings/sound/fsl,ssi.yaml > > diff --git a/Documentation/devicetree/bindings/sound/fsl,ssi.txt > b/Documentation/devicetree/bindings/sound/fsl,ssi.txt > deleted file mode 100644 > index 7e15a85cecd2.. > --- a/Documentation/devicetree/bindings/sound/fsl,ssi.txt > +++ /dev/null > @@ -1,87 +0,0 @@ > -Freescale Synchronous Serial Interface > - > -The SSI is a serial device that communicates with audio codecs. It can > -be programmed in AC97, I2S, left-justified, or right-justified modes. > - > -Required properties: > -- compatible: Compatible list, should contain one of the following > -compatibles: > - fsl,mpc8610-ssi > - fsl,imx51-ssi > - fsl,imx35-ssi > - fsl,imx21-ssi > -- cell-index: The SSI, <0> = SSI1, <1> = SSI2, and so on. > -- reg: Offset and length of the register set for the device. > -- interrupts:where a is the interrupt number and b is a > -field that represents an encoding of the sense and > -level information for the interrupt. This should be > -encoded based on the information in section 2) > -depending on the type of interrupt controller you > -have. > -- fsl,fifo-depth: The number of elements in the transmit and receive FIFOs. > -This number is the maximum allowed value for > SFCSR[TFWM0]. > - - clocks: "ipg" - Required clock for the SSI unit > -"baud" - Required clock for SSI master mode. Otherwise > this > - clock is not used > - > -Required are also ac97 link bindings if ac97 is used. See > -Documentation/devicetree/bindings/sound/soc-ac97link.txt for the necessary > -bindings. > - > -Optional properties: > -- codec-handle: Phandle to a 'codec' node that defines an audio > -codec connected to this SSI. This node is typically > -a child of an I2C or other control node. > -- fsl,fiq-stream-filter: Bool property. Disabled DMA and use FIQ instead to > - filter the codec stream. This is necessary for some boards > - where an incompatible codec is connected to this SSI, e.g. > - on pca100 and pcm043. > -- dmas: Generic dma devicetree binding as described in > - Documentation/devicetree/bindings/dma/dma.txt. > -- dma-names: Two dmas have to be defined, "tx" and "rx", if fsl,imx-fiq > - is not defined. > -- fsl,mode: The operating mode for the AC97 interface only. > -"ac97-slave" - AC97 mode, SSI is clock slave > -"ac97-master" - AC97 mode, SSI is clock master > -- fsl,ssi-asynchronous: > -If specified, the SSI is to be programmed in asynchronous > -mode. In this mode, pins SRCK, STCK, SRFS, and STFS must > -all be connected to valid signals. In synchronous mode, > -SRCK and SRFS are ignored. Asynchronous mode allows > -playback and capture to use different sample sizes and > -sample rates. Some drivers may require that SRCK and > STCK > -be connected together, and SRFS and STFS be connected > -together. This would still allow different sample sizes, > -but not different sample rates. > -- fsl,playback-dma: Phandle to a node for the DMA channel to use for > -playback of audio. This is typically dictated by SOC > -design. See the notes below. > -Only used on Power Architecture. > -- fsl,capture-dma: Phandle to a node for the DMA channel to use for > -capture (recording) of audio. This is
Please add powerpc topic/kdump-hotplug branch to linux-next
Hi Stephen, Can you please add the topic/kdump-hotplug branch of the powerpc tree to linux-next. It contains a series that touches generic kexec code as well as x86 and powerpc code. The hope is to have to get it merged for v6.10, so it should go along with the powerpc next branch in the merge order. https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/log/?h=topic/kdump-hotplug cheers
Re: [PATCH] ibmvnic: Use -EBUSY in __ibmvnic_reset()
On Tue, Apr 23, 2024 at 12:54:55PM +0200, Paolo Abeni wrote:
> On Fri, 2024-04-19 at 16:08 +0200, Markus Elfring wrote:
> > From: Markus Elfring
> > Date: Fri, 19 Apr 2024 15:46:17 +0200
> >
> > Add a minus sign before the error code “EBUSY”
> > so that a negative value will be used as in other cases.
> >
> > This issue was transformed by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring
> > ---
> > drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> > b/drivers/net/ethernet/ibm/ibmvnic.c
> > index 5e9a93bdb518..737ae83a836a 100644
> > --- a/drivers/net/ethernet/ibm/ibmvnic.c
> > +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> > @@ -3212,7 +3212,7 @@ static void __ibmvnic_reset(struct work_struct *work)
> > adapter->state == VNIC_REMOVED) {
> > spin_unlock_irqrestore(>state_lock, flags);
> > kfree(rwi);
> > - rc = EBUSY;
> > + rc = -EBUSY;
> > break;
> >
>
> AFAICS the error is always used as bool, so this will not change any
> behavior in practice. I tend to think we should not merge this kind of
> change outside some larger work in the same area, but I'd love a second
> opinion from the driver owners.
I missed the original patch due to my procmail filters...
You're right that it doesn't affect the behavior of the driver except
for the debug output when we do:
netdev_dbg(adapter->netdev, "Reset failed, rc=%d\n", rc);
But the - was left off uninitentionally so I think we should apply it.
I have been trying to look for similar bugs where the - is left off.
It's a bit challenging because there places where we use positive
error codes deliberately. But in this case a static checker could
easily detect the bug with a low false positive ratio by saying, "We're
mixing normal negative error codes with positive EBUSY".
regards,
dan carpenter
[PATCH AUTOSEL 6.6 05/16] powerpc/crypto/chacha-p10: Fix failure on non Power10
From: Michael Ellerman
[ Upstream commit 69630926011c1f7170a465b7b5c228deb66e9372 ]
The chacha-p10-crypto module provides optimised chacha routines for
Power10. It also selects CRYPTO_ARCH_HAVE_LIB_CHACHA which says it
provides chacha_crypt_arch() to generic code.
Notably the module needs to provide chacha_crypt_arch() regardless of
whether it is loaded on Power10 or an older CPU.
The implementation of chacha_crypt_arch() already has a fallback to
chacha_crypt_generic(), however the module as a whole fails to load on
pre-Power10, because of the use of module_cpu_feature_match().
This breaks for example loading wireguard:
jostaberry-1:~ # modprobe -v wireguard
insmod
/lib/modules/6.8.0-lp155.8.g7e0e887-default/kernel/arch/powerpc/crypto/chacha-p10-crypto.ko.zst
modprobe: ERROR: could not insert 'wireguard': No such device
Fix it by removing module_cpu_feature_match(), and instead check the
CPU feature manually. If the CPU feature is not found, the module
still loads successfully, but doesn't register the Power10 specific
algorithms. That allows chacha_crypt_generic() to remain available for
use, fixing the problem.
[root@fedora ~]# modprobe -v wireguard
insmod /lib/modules/6.8.0-1-g786a790c4d79/kernel/net/ipv4/udp_tunnel.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/net/ipv6/ip6_udp_tunnel.ko
insmod /lib/modules/6.8.0-1-g786a790c4d79/kernel/lib/crypto/libchacha.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/arch/powerpc/crypto/chacha-p10-crypto.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/lib/crypto/libchacha20poly1305.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/drivers/net/wireguard/wireguard.ko
[ 18.910452][ T721] wireguard: allowedips self-tests: pass
[ 18.914999][ T721] wireguard: nonce counter self-tests: pass
[ 19.029066][ T721] wireguard: ratelimiter self-tests: pass
[ 19.029257][ T721] wireguard: WireGuard 1.0.0 loaded. See
www.wireguard.com for information.
[ 19.029361][ T721] wireguard: Copyright (C) 2015-2019 Jason A. Donenfeld
. All Rights Reserved.
Reported-by: Michal Suchánek
Closes: https://lore.kernel.org/all/20240315122005.gg20...@kitsune.suse.cz/
Acked-by: Herbert Xu
Signed-off-by: Michael Ellerman
Link: https://msgid.link/20240328130200.3041687-1-...@ellerman.id.au
Signed-off-by: Sasha Levin
---
arch/powerpc/crypto/chacha-p10-glue.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/crypto/chacha-p10-glue.c
b/arch/powerpc/crypto/chacha-p10-glue.c
index 74fb86b0d2097..7c728755852e1 100644
--- a/arch/powerpc/crypto/chacha-p10-glue.c
+++ b/arch/powerpc/crypto/chacha-p10-glue.c
@@ -197,6 +197,9 @@ static struct skcipher_alg algs[] = {
static int __init chacha_p10_init(void)
{
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return 0;
+
static_branch_enable(_p10);
return crypto_register_skciphers(algs, ARRAY_SIZE(algs));
@@ -204,10 +207,13 @@ static int __init chacha_p10_init(void)
static void __exit chacha_p10_exit(void)
{
+ if (!static_branch_likely(_p10))
+ return;
+
crypto_unregister_skciphers(algs, ARRAY_SIZE(algs));
}
-module_cpu_feature_match(PPC_MODULE_FEATURE_P10, chacha_p10_init);
+module_init(chacha_p10_init);
module_exit(chacha_p10_exit);
MODULE_DESCRIPTION("ChaCha and XChaCha stream ciphers (P10 accelerated)");
--
2.43.0
[PATCH AUTOSEL 6.8 05/18] powerpc/crypto/chacha-p10: Fix failure on non Power10
From: Michael Ellerman
[ Upstream commit 69630926011c1f7170a465b7b5c228deb66e9372 ]
The chacha-p10-crypto module provides optimised chacha routines for
Power10. It also selects CRYPTO_ARCH_HAVE_LIB_CHACHA which says it
provides chacha_crypt_arch() to generic code.
Notably the module needs to provide chacha_crypt_arch() regardless of
whether it is loaded on Power10 or an older CPU.
The implementation of chacha_crypt_arch() already has a fallback to
chacha_crypt_generic(), however the module as a whole fails to load on
pre-Power10, because of the use of module_cpu_feature_match().
This breaks for example loading wireguard:
jostaberry-1:~ # modprobe -v wireguard
insmod
/lib/modules/6.8.0-lp155.8.g7e0e887-default/kernel/arch/powerpc/crypto/chacha-p10-crypto.ko.zst
modprobe: ERROR: could not insert 'wireguard': No such device
Fix it by removing module_cpu_feature_match(), and instead check the
CPU feature manually. If the CPU feature is not found, the module
still loads successfully, but doesn't register the Power10 specific
algorithms. That allows chacha_crypt_generic() to remain available for
use, fixing the problem.
[root@fedora ~]# modprobe -v wireguard
insmod /lib/modules/6.8.0-1-g786a790c4d79/kernel/net/ipv4/udp_tunnel.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/net/ipv6/ip6_udp_tunnel.ko
insmod /lib/modules/6.8.0-1-g786a790c4d79/kernel/lib/crypto/libchacha.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/arch/powerpc/crypto/chacha-p10-crypto.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/lib/crypto/libchacha20poly1305.ko
insmod
/lib/modules/6.8.0-1-g786a790c4d79/kernel/drivers/net/wireguard/wireguard.ko
[ 18.910452][ T721] wireguard: allowedips self-tests: pass
[ 18.914999][ T721] wireguard: nonce counter self-tests: pass
[ 19.029066][ T721] wireguard: ratelimiter self-tests: pass
[ 19.029257][ T721] wireguard: WireGuard 1.0.0 loaded. See
www.wireguard.com for information.
[ 19.029361][ T721] wireguard: Copyright (C) 2015-2019 Jason A. Donenfeld
. All Rights Reserved.
Reported-by: Michal Suchánek
Closes: https://lore.kernel.org/all/20240315122005.gg20...@kitsune.suse.cz/
Acked-by: Herbert Xu
Signed-off-by: Michael Ellerman
Link: https://msgid.link/20240328130200.3041687-1-...@ellerman.id.au
Signed-off-by: Sasha Levin
---
arch/powerpc/crypto/chacha-p10-glue.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/crypto/chacha-p10-glue.c
b/arch/powerpc/crypto/chacha-p10-glue.c
index 74fb86b0d2097..7c728755852e1 100644
--- a/arch/powerpc/crypto/chacha-p10-glue.c
+++ b/arch/powerpc/crypto/chacha-p10-glue.c
@@ -197,6 +197,9 @@ static struct skcipher_alg algs[] = {
static int __init chacha_p10_init(void)
{
+ if (!cpu_has_feature(CPU_FTR_ARCH_31))
+ return 0;
+
static_branch_enable(_p10);
return crypto_register_skciphers(algs, ARRAY_SIZE(algs));
@@ -204,10 +207,13 @@ static int __init chacha_p10_init(void)
static void __exit chacha_p10_exit(void)
{
+ if (!static_branch_likely(_p10))
+ return;
+
crypto_unregister_skciphers(algs, ARRAY_SIZE(algs));
}
-module_cpu_feature_match(PPC_MODULE_FEATURE_P10, chacha_p10_init);
+module_init(chacha_p10_init);
module_exit(chacha_p10_exit);
MODULE_DESCRIPTION("ChaCha and XChaCha stream ciphers (P10 accelerated)");
--
2.43.0
Re: [PATCH] ibmvnic: Use -EBUSY in __ibmvnic_reset()
On Fri, 2024-04-19 at 16:08 +0200, Markus Elfring wrote:
> From: Markus Elfring
> Date: Fri, 19 Apr 2024 15:46:17 +0200
>
> Add a minus sign before the error code “EBUSY”
> so that a negative value will be used as in other cases.
>
> This issue was transformed by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5e9a93bdb518..737ae83a836a 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -3212,7 +3212,7 @@ static void __ibmvnic_reset(struct work_struct *work)
> adapter->state == VNIC_REMOVED) {
> spin_unlock_irqrestore(>state_lock, flags);
> kfree(rwi);
> - rc = EBUSY;
> + rc = -EBUSY;
> break;
>
AFAICS the error is always used as bool, so this will not change any
behavior in practice. I tend to think we should not merge this kind of
change outside some larger work in the same area, but I'd love a second
opinion from the driver owners.
Thanks,
Paolo
> }
Re: [PATCH v3 0/5] Add generic data patching functions
On Mon, Mar 25, 2024 at 04:52:57PM +1100, Benjamin Gray wrote: > Currently patch_instruction() bases the write length on the value being > written. If the value looks like a prefixed instruction it writes 8 bytes, > otherwise it writes 4 bytes. This makes it potentially buggy to use for > writing arbitrary data, as if you want to write 4 bytes but it decides to > write 8 bytes it may clobber the following memory or be unaligned and > trigger an oops if it tries to cross a page boundary. > > To solve this, this series pulls out the size parameter to the 'top' of > the memory patching logic, and propagates it through the various functions. > > The two sizes supported are int and long; this allows for patching > instructions and pointers on both ppc32 and ppc64. On ppc32 these are the > same size, so care is taken to only use the size parameter on static > functions, so the compiler can optimise it out entirely. Unfortunately > GCC trips over its own feet here and won't optimise in a way that is > optimal for strict RWX (mpc85xx_smp_defconfig) and no RWX > (pmac32_defconfig). More details in the v2 cover letter. > > Changes from v2: > * Various changes noted on each patch > * Data patching now enforced to be aligned > * Restore page aligned flushing optimisation > > Changes from v1: > * Addressed the v1 review actions > * Removed noinline (for now) > > v2: > https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20231016050147.115686-1-bg...@linux.ibm.com/ > v1: > https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20230207015643.590684-1-bg...@linux.ibm.com/ > > Benjamin Gray (5): > powerpc/code-patching: Add generic memory patching > powerpc/code-patching: Add data patch alignment check > powerpc/64: Convert patch_instruction() to patch_u32() > powerpc/32: Convert patch_instruction() to patch_uint() > powerpc/code-patching: Add boot selftest for data patching > > arch/powerpc/include/asm/code-patching.h | 37 + > arch/powerpc/kernel/module_64.c | 5 +- > arch/powerpc/kernel/static_call.c| 2 +- > arch/powerpc/lib/code-patching.c | 70 +++- > arch/powerpc/lib/test-code-patching.c| 36 > arch/powerpc/platforms/powermac/smp.c| 2 +- > 6 files changed, 132 insertions(+), 20 deletions(-) Apart from the minor comments, for this series: Acked-by: Naveen N Rao Thanks for working on this. - Naveen
Re: [PATCH v3 3/5] powerpc/64: Convert patch_instruction() to patch_u32()
On Mon, Mar 25, 2024 at 04:53:00PM +1100, Benjamin Gray wrote:
> This use of patch_instruction() is working on 32 bit data, and can fail
> if the data looks like a prefixed instruction and the extra write
> crosses a page boundary. Use patch_u32() to fix the write size.
>
> Fixes: 8734b41b3efe ("powerpc/module_64: Fix livepatching for RO modules")
> Link: https://lore.kernel.org/all/20230203004649.1f59dbd4@yea/
> Signed-off-by: Benjamin Gray
>
> ---
>
> v2: * Added the fixes tag, it seems appropriate even if the subject does
> mention a more robust solution being required.
>
> patch_u64() should be more efficient, but judging from the bug report
> it doesn't seem like the data is doubleword aligned.
Asking again, is that still the case? It looks like at least the first
fix below can be converted to patch_u64().
- Naveen
> ---
> arch/powerpc/kernel/module_64.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 7112adc597a8..e9bab599d0c2 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -651,12 +651,11 @@ static inline int create_stub(const Elf64_Shdr *sechdrs,
> // func_desc_t is 8 bytes if ABIv2, else 16 bytes
> desc = func_desc(addr);
> for (i = 0; i < sizeof(func_desc_t) / sizeof(u32); i++) {
> - if (patch_instruction(((u32 *)>funcdata) + i,
> - ppc_inst(((u32 *)())[i])))
> + if (patch_u32(((u32 *)>funcdata) + i, ((u32 *))[i]))
> return 0;
> }
>
> - if (patch_instruction(>magic, ppc_inst(STUB_MAGIC)))
> + if (patch_u32(>magic, STUB_MAGIC))
> return 0;
>
> return 1;
> --
> 2.44.0
>
Re: [PATCH v3 5/5] powerpc/code-patching: Add boot selftest for data patching
On Mon, Mar 25, 2024 at 04:53:02PM +1100, Benjamin Gray wrote:
> Extend the code patching selftests with some basic coverage of the new
> data patching variants too.
>
> Signed-off-by: Benjamin Gray
>
> ---
>
> v3: * New in v3
> ---
> arch/powerpc/lib/test-code-patching.c | 36 +++
> 1 file changed, 36 insertions(+)
>
> diff --git a/arch/powerpc/lib/test-code-patching.c
> b/arch/powerpc/lib/test-code-patching.c
> index c44823292f73..e96c48fcd4db 100644
> --- a/arch/powerpc/lib/test-code-patching.c
> +++ b/arch/powerpc/lib/test-code-patching.c
> @@ -347,6 +347,41 @@ static void __init test_prefixed_patching(void)
> check(!memcmp(iptr, expected, sizeof(expected)));
> }
>
> +static void __init test_data_patching(void)
> +{
> + void *buf;
> + u32 *addr32;
> +
> + buf = vzalloc(PAGE_SIZE);
> + check(buf);
> + if (!buf)
> + return;
> +
> + addr32 = buf + 128;
> +
> + addr32[1] = 0xA0A1A2A3;
> + addr32[2] = 0xB0B1B2B3;
> +
> + patch_uint([1], 0xC0C1C2C3);
> +
> + check(addr32[0] == 0);
> + check(addr32[1] == 0xC0C1C2C3);
> + check(addr32[2] == 0xB0B1B2B3);
> + check(addr32[3] == 0);
> +
> + patch_ulong([1], 0xD0D1D2D3);
> +
> + check(addr32[0] == 0);
> + *(unsigned long *)([1]) = 0xD0D1D2D3;
Should that have been a check() instead?
- Naveen
> +
> + if (!IS_ENABLED(CONFIG_PPC64))
> + check(addr32[2] == 0xB0B1B2B3);
> +
> + check(addr32[3] == 0);
> +
> + vfree(buf);
> +}
> +
> static int __init test_code_patching(void)
> {
> pr_info("Running code patching self-tests ...\n");
> @@ -356,6 +391,7 @@ static int __init test_code_patching(void)
> test_create_function_call();
> test_translate_branch();
> test_prefixed_patching();
> + test_data_patching();
>
> return 0;
> }
> --
> 2.44.0
>
[PATCH v3 01/11] stackleak: don't modify ctl_table argument
In a future commit the proc_handlers will change to
"const struct ctl_table".
As a preparation for that adapt the logic to work with a temporary
variable, similar to how it is done in other parts of the kernel.
Fixes: 964c9dff0091 ("stackleak: Allow runtime disabling of kernel stack
erasing")
Acked-by: Kees Cook
Signed-off-by: Thomas Weißschuh
---
kernel/stackleak.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/stackleak.c b/kernel/stackleak.c
index d099f3affcf1..558b9d6d28d3 100644
--- a/kernel/stackleak.c
+++ b/kernel/stackleak.c
@@ -27,10 +27,11 @@ static int stack_erasing_sysctl(struct ctl_table *table,
int write,
int ret = 0;
int state = !static_branch_unlikely(_erasing_bypass);
int prev_state = state;
+ struct ctl_table tmp = *table;
- table->data =
- table->maxlen = sizeof(int);
- ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+ tmp.data =
+ tmp.maxlen = sizeof(int);
+ ret = proc_dointvec_minmax(, write, buffer, lenp, ppos);
state = !!state;
if (ret || !write || state == prev_state)
return ret;
--
2.44.0
[PATCH v3 02/11] cgroup: bpf: constify ctl_table arguments and fields
In a future commit the sysctl core will only use
"const struct ctl_table". As a preparation for that adapt the cgroup-bpf
code.
Signed-off-by: Thomas Weißschuh
---
include/linux/filter.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 7a27f19bf44d..4eada55a2df8 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1404,7 +1404,7 @@ struct bpf_sock_ops_kern {
struct bpf_sysctl_kern {
struct ctl_table_header *head;
- struct ctl_table *table;
+ const struct ctl_table *table;
void *cur_val;
size_t cur_len;
void *new_val;
--
2.44.0
[PATCH v3 10/11] sysctl: constify ctl_table arguments of utility function
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helper.
Signed-off-by: Thomas Weißschuh
---
include/linux/sysctl.h | 2 +-
kernel/sysctl.c| 21 +++--
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 09db2f2e6488..54fbec062772 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -237,7 +237,7 @@ extern struct ctl_table_header
*register_sysctl_mount_point(const char *path);
void do_sysctl_args(void);
bool sysctl_is_alias(char *param);
-int do_proc_douintvec(struct ctl_table *table, int write,
+int do_proc_douintvec(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos,
int (*conv)(unsigned long *lvalp,
unsigned int *valp,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e0b917328cf9..62dd27752960 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -205,7 +205,7 @@ static int _proc_do_string(char *data, int maxlen, int
write,
return 0;
}
-static void warn_sysctl_write(struct ctl_table *table)
+static void warn_sysctl_write(const struct ctl_table *table)
{
pr_warn_once("%s wrote to %s when file position was not 0!\n"
"This will not be supported in the future. To silence this\n"
@@ -223,7 +223,7 @@ static void warn_sysctl_write(struct ctl_table *table)
* handlers can ignore the return value.
*/
static bool proc_first_pos_non_zero_ignore(loff_t *ppos,
- struct ctl_table *table)
+ const struct ctl_table *table)
{
if (!*ppos)
return false;
@@ -468,7 +468,7 @@ static int do_proc_douintvec_conv(unsigned long *lvalp,
static const char proc_wspace_sep[] = { ' ', '\t', '\n' };
-static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
+static int __do_proc_dointvec(void *tbl_data, const struct ctl_table *table,
int write, void *buffer,
size_t *lenp, loff_t *ppos,
int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
@@ -541,7 +541,7 @@ static int __do_proc_dointvec(void *tbl_data, struct
ctl_table *table,
return err;
}
-static int do_proc_dointvec(struct ctl_table *table, int write,
+static int do_proc_dointvec(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos,
int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
int write, void *data),
@@ -552,7 +552,7 @@ static int do_proc_dointvec(struct ctl_table *table, int
write,
}
static int do_proc_douintvec_w(unsigned int *tbl_data,
- struct ctl_table *table,
+ const struct ctl_table *table,
void *buffer,
size_t *lenp, loff_t *ppos,
int (*conv)(unsigned long *lvalp,
@@ -639,7 +639,7 @@ static int do_proc_douintvec_r(unsigned int *tbl_data, void
*buffer,
return err;
}
-static int __do_proc_douintvec(void *tbl_data, struct ctl_table *table,
+static int __do_proc_douintvec(void *tbl_data, const struct ctl_table *table,
int write, void *buffer,
size_t *lenp, loff_t *ppos,
int (*conv)(unsigned long *lvalp,
@@ -675,7 +675,7 @@ static int __do_proc_douintvec(void *tbl_data, struct
ctl_table *table,
return do_proc_douintvec_r(i, buffer, lenp, ppos, conv, data);
}
-int do_proc_douintvec(struct ctl_table *table, int write,
+int do_proc_douintvec(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos,
int (*conv)(unsigned long *lvalp,
unsigned int *valp,
@@ -1023,8 +1023,9 @@ static int sysrq_sysctl_handler(struct ctl_table *table,
int write,
}
#endif
-static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table,
- int write, void *buffer, size_t *lenp, loff_t *ppos,
+static int __do_proc_doulongvec_minmax(void *data,
+ const struct ctl_table *table, int write,
+ void *buffer, size_t *lenp, loff_t *ppos,
unsigned long convmul, unsigned long convdiv)
{
unsigned long *i, *min, *max;
@@ -1096,7 +1097,7 @@ static int __do_proc_doulongvec_minmax(void *data, struct
ctl_table *table,
return err;
}
-static int do_proc_doulongvec_minmax(struct ctl_table *table, int write,
+static int do_proc_doulongvec_minmax(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos, unsigned long convmul,
unsigned long convdiv)
{
--
2.44.0
[PATCH v3 08/11] ipv6/ndisc: constify ctl_table arguments of utility function
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helper.
Signed-off-by: Thomas Weißschuh
---
net/ipv6/ndisc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index ae134634c323..945d5f5ca039 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1936,7 +1936,7 @@ static struct notifier_block ndisc_netdev_notifier = {
};
#ifdef CONFIG_SYSCTL
-static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl,
+static void ndisc_warn_deprecated_sysctl(const struct ctl_table *ctl,
const char *func, const char *dev_name)
{
static char warncomm[TASK_COMM_LEN];
--
2.44.0
[PATCH v3 07/11] ipv6/addrconf: constify ctl_table arguments of utility functions
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helpers.
Signed-off-by: Thomas Weißschuh
---
net/ipv6/addrconf.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9aa0900abfa1..96ab349e8ba4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -863,7 +863,7 @@ static void addrconf_forward_change(struct net *net, __s32
newf)
}
}
-static int addrconf_fixup_forwarding(struct ctl_table *table, int *p, int newf)
+static int addrconf_fixup_forwarding(const struct ctl_table *table, int *p,
int newf)
{
struct net *net;
int old;
@@ -931,7 +931,7 @@ static void addrconf_linkdown_change(struct net *net, __s32
newf)
}
}
-static int addrconf_fixup_linkdown(struct ctl_table *table, int *p, int newf)
+static int addrconf_fixup_linkdown(const struct ctl_table *table, int *p, int
newf)
{
struct net *net;
int old;
@@ -6378,7 +6378,7 @@ static void addrconf_disable_change(struct net *net,
__s32 newf)
}
}
-static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int newf)
+static int addrconf_disable_ipv6(const struct ctl_table *table, int *p, int
newf)
{
struct net *net = (struct net *)table->extra2;
int old;
@@ -6669,7 +6669,7 @@ void addrconf_disable_policy_idev(struct inet6_dev *idev,
int val)
}
static
-int addrconf_disable_policy(struct ctl_table *ctl, int *valp, int val)
+int addrconf_disable_policy(const struct ctl_table *ctl, int *valp, int val)
{
struct net *net = (struct net *)ctl->extra2;
struct inet6_dev *idev;
--
2.44.0
[PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
* Patch 1 is a bugfix for the stack_erasing sysctl handler * Patches 2-10 change various helper functions throughout the kernel to be able to handle 'const ctl_table'. * Patch 11 changes the signatures of all proc handlers through the tree. Some other signatures are also adapted, for details see the commit message. Only patch 1 changes any code at all. The series was compile-tested on top of next-20230423 for i386, x86_64, arm, arm64, riscv, loongarch, s390 and m68k. The series was split from my larger series sysctl-const series [0]. It only focusses on the proc_handlers but is an important step to be able to move all static definitions of ctl_table into .rodata. [0] https://lore.kernel.org/lkml/20231204-const-sysctl-v2-0-7a5060b11...@weissschuh.net/ Signed-off-by: Thomas Weißschuh --- Changes in v3: - Rebase on current -next - Cc affected mailing lists again to gather feedback - Link to v2: https://lore.kernel.org/r/20240323-sysctl-const-handler-v2-0-e80b178f1...@weissschuh.net Changes in v2: - Reduce recipient list - Fix source formatting - Rebase onto next-20240322 - Link to v1: https://lore.kernel.org/r/20240315-sysctl-const-handler-v1-0-1322ac7cb...@weissschuh.net --- Thomas Weißschuh (11): stackleak: don't modify ctl_table argument cgroup: bpf: constify ctl_table arguments and fields hugetlb: constify ctl_table arguments of utility functions utsname: constify ctl_table arguments of utility function neighbour: constify ctl_table arguments of utility function ipv4/sysctl: constify ctl_table arguments of utility functions ipv6/addrconf: constify ctl_table arguments of utility functions ipv6/ndisc: constify ctl_table arguments of utility function ipvs: constify ctl_table arguments of utility functions sysctl: constify ctl_table arguments of utility function sysctl: treewide: constify the ctl_table argument of handlers arch/arm64/kernel/armv8_deprecated.c | 2 +- arch/arm64/kernel/fpsimd.c| 2 +- arch/s390/appldata/appldata_base.c| 10 ++-- arch/s390/kernel/debug.c | 2 +- arch/s390/kernel/topology.c | 2 +- arch/s390/mm/cmm.c| 6 +-- arch/x86/kernel/itmt.c| 2 +- drivers/cdrom/cdrom.c | 4 +- drivers/char/random.c | 6 +-- drivers/macintosh/mac_hid.c | 2 +- drivers/net/vrf.c | 2 +- drivers/parport/procfs.c | 12 ++--- drivers/perf/arm_pmuv3.c | 4 +- drivers/perf/riscv_pmu_sbi.c | 2 +- fs/coredump.c | 2 +- fs/dcache.c | 4 +- fs/drop_caches.c | 2 +- fs/exec.c | 4 +- fs/file_table.c | 4 +- fs/fs-writeback.c | 2 +- fs/inode.c| 4 +- fs/pipe.c | 2 +- fs/quota/dquot.c | 2 +- fs/xfs/xfs_sysctl.c | 6 +-- include/linux/filter.h| 2 +- include/linux/ftrace.h| 4 +- include/linux/mm.h| 8 +-- include/linux/perf_event.h| 6 +-- include/linux/security.h | 2 +- include/linux/sysctl.h| 36 ++--- include/linux/vmstat.h| 6 +-- include/linux/writeback.h | 2 +- include/net/ndisc.h | 2 +- include/net/neighbour.h | 6 +-- include/net/netfilter/nf_hooks_lwtunnel.h | 2 +- ipc/ipc_sysctl.c | 8 +-- kernel/bpf/syscall.c | 4 +- kernel/delayacct.c| 4 +- kernel/events/callchain.c | 2 +- kernel/events/core.c | 4 +- kernel/fork.c | 2 +- kernel/hung_task.c| 6 +-- kernel/kexec_core.c | 2 +- kernel/kprobes.c | 2 +- kernel/latencytop.c | 4 +- kernel/pid_namespace.c| 2 +- kernel/pid_sysctl.h | 2 +- kernel/printk/internal.h | 2 +- kernel/printk/printk.c| 2 +- kernel/printk/sysctl.c| 5 +- kernel/sched/core.c | 8 +-- kernel/sched/rt.c | 16 +++--- kernel/sched/topology.c | 2 +- kernel/seccomp.c | 10 ++-- kernel/stackleak.c| 9 ++-- kernel/sysctl.c | 89 --- kernel/time/timer.c | 2 +- kernel/trace/ftrace.c | 2 +- kernel/trace/trace.c
[PATCH v3 06/11] ipv4/sysctl: constify ctl_table arguments of utility functions
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helpers.
Signed-off-by: Thomas Weißschuh
---
net/ipv4/sysctl_net_ipv4.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index ce5d19978a26..fc4c001bf72a 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -130,7 +130,8 @@ static int ipv4_privileged_ports(struct ctl_table *table,
int write,
return ret;
}
-static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t
*low, kgid_t *high)
+static void inet_get_ping_group_range_table(const struct ctl_table *table,
+ kgid_t *low, kgid_t *high)
{
kgid_t *data = table->data;
struct net *net =
@@ -145,7 +146,8 @@ static void inet_get_ping_group_range_table(struct
ctl_table *table, kgid_t *low
}
/* Update system visible IP port range */
-static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t
high)
+static void set_ping_group_range(const struct ctl_table *table,
+kgid_t low, kgid_t high)
{
kgid_t *data = table->data;
struct net *net =
--
2.44.0
[PATCH v3 09/11] ipvs: constify ctl_table arguments of utility functions
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helpers.
Signed-off-by: Thomas Weißschuh
---
net/netfilter/ipvs/ip_vs_ctl.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 143a341bbc0a..689ac521ea2d 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1924,7 +1924,8 @@ proc_do_sync_ports(struct ctl_table *table, int write,
return rc;
}
-static int ipvs_proc_est_cpumask_set(struct ctl_table *table, void *buffer)
+static int ipvs_proc_est_cpumask_set(const struct ctl_table *table,
+void *buffer)
{
struct netns_ipvs *ipvs = table->extra2;
cpumask_var_t *valp = table->data;
@@ -1962,8 +1963,8 @@ static int ipvs_proc_est_cpumask_set(struct ctl_table
*table, void *buffer)
return ret;
}
-static int ipvs_proc_est_cpumask_get(struct ctl_table *table, void *buffer,
-size_t size)
+static int ipvs_proc_est_cpumask_get(const struct ctl_table *table,
+void *buffer, size_t size)
{
struct netns_ipvs *ipvs = table->extra2;
cpumask_var_t *valp = table->data;
--
2.44.0
[PATCH v3 11/11] sysctl: treewide: constify the ctl_table argument of handlers
Adapt the proc_hander function signature to make it clear that handlers
are not supposed to modify their ctl_table argument.
This is a prerequisite to moving the static ctl_table structs into
.rodata.
By migrating all handlers at once a lengthy transition can be avoided.
The patch was mostly generated by coccinelle with the following script:
@@
identifier func, ctl, write, buffer, lenp, ppos;
@@
int func(
- struct ctl_table *ctl,
+ const struct ctl_table *ctl,
int write, void *buffer, size_t *lenp, loff_t *ppos)
{ ... }
In addition to the scripted changes some other changes are done:
* the typedef proc_handler is adapted
* the prototypes of non-static handler are adapted
* kernel/seccomp.c:{read,write}_actions_logged() and
kernel/watchdog.c:proc_watchdog_common() are adapted as they need to
adapted together with the handlers for type-consistency reasons
Signed-off-by: Thomas Weißschuh
---
arch/arm64/kernel/armv8_deprecated.c | 2 +-
arch/arm64/kernel/fpsimd.c| 2 +-
arch/s390/appldata/appldata_base.c| 10 ++---
arch/s390/kernel/debug.c | 2 +-
arch/s390/kernel/topology.c | 2 +-
arch/s390/mm/cmm.c| 6 +--
arch/x86/kernel/itmt.c| 2 +-
drivers/cdrom/cdrom.c | 4 +-
drivers/char/random.c | 6 +--
drivers/macintosh/mac_hid.c | 2 +-
drivers/net/vrf.c | 2 +-
drivers/parport/procfs.c | 12 +++---
drivers/perf/arm_pmuv3.c | 4 +-
drivers/perf/riscv_pmu_sbi.c | 2 +-
fs/coredump.c | 2 +-
fs/dcache.c | 4 +-
fs/drop_caches.c | 2 +-
fs/exec.c | 4 +-
fs/file_table.c | 4 +-
fs/fs-writeback.c | 2 +-
fs/inode.c| 4 +-
fs/pipe.c | 2 +-
fs/quota/dquot.c | 2 +-
fs/xfs/xfs_sysctl.c | 6 +--
include/linux/ftrace.h| 4 +-
include/linux/mm.h| 8 ++--
include/linux/perf_event.h| 6 +--
include/linux/security.h | 2 +-
include/linux/sysctl.h| 34
include/linux/vmstat.h| 6 +--
include/linux/writeback.h | 2 +-
include/net/ndisc.h | 2 +-
include/net/neighbour.h | 6 +--
include/net/netfilter/nf_hooks_lwtunnel.h | 2 +-
ipc/ipc_sysctl.c | 8 ++--
kernel/bpf/syscall.c | 4 +-
kernel/delayacct.c| 4 +-
kernel/events/callchain.c | 2 +-
kernel/events/core.c | 4 +-
kernel/fork.c | 2 +-
kernel/hung_task.c| 6 +--
kernel/kexec_core.c | 2 +-
kernel/kprobes.c | 2 +-
kernel/latencytop.c | 4 +-
kernel/pid_namespace.c| 2 +-
kernel/pid_sysctl.h | 2 +-
kernel/printk/internal.h | 2 +-
kernel/printk/printk.c| 2 +-
kernel/printk/sysctl.c| 5 ++-
kernel/sched/core.c | 8 ++--
kernel/sched/rt.c | 16
kernel/sched/topology.c | 2 +-
kernel/seccomp.c | 10 ++---
kernel/stackleak.c| 2 +-
kernel/sysctl.c | 68 +++
kernel/time/timer.c | 2 +-
kernel/trace/ftrace.c | 2 +-
kernel/trace/trace.c | 2 +-
kernel/trace/trace_events_user.c | 2 +-
kernel/trace/trace_stack.c| 2 +-
kernel/umh.c | 2 +-
kernel/utsname_sysctl.c | 2 +-
kernel/watchdog.c | 12 +++---
mm/compaction.c | 6 +--
mm/hugetlb.c | 8 ++--
mm/page-writeback.c | 18
mm/page_alloc.c | 14 +++
mm/util.c | 12 +++---
mm/vmstat.c | 4 +-
net/bridge/br_netfilter_hooks.c | 2 +-
net/core/neighbour.c | 18
net/core/sysctl_net_core.c| 20 -
net/ipv4/devinet.c| 6 +--
net/ipv4/route.c | 2 +-
net/ipv4/sysctl_net_ipv4.c| 32 +++
net/ipv6/addrconf.c | 19 +
net/ipv6/ndisc.c
[PATCH v3 04/11] utsname: constify ctl_table arguments of utility function
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helper.
Signed-off-by: Thomas Weißschuh
---
kernel/utsname_sysctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/utsname_sysctl.c b/kernel/utsname_sysctl.c
index 76a772072557..04e4513f2985 100644
--- a/kernel/utsname_sysctl.c
+++ b/kernel/utsname_sysctl.c
@@ -15,7 +15,7 @@
#ifdef CONFIG_PROC_SYSCTL
-static void *get_uts(struct ctl_table *table)
+static void *get_uts(const struct ctl_table *table)
{
char *which = table->data;
struct uts_namespace *uts_ns;
--
2.44.0
[PATCH v3 05/11] neighbour: constify ctl_table arguments of utility function
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helper.
Signed-off-by: Thomas Weißschuh
---
net/core/neighbour.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 0805c00c63d4..92a01664a723 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -3578,7 +3578,7 @@ static void neigh_copy_dflt_parms(struct net *net, struct
neigh_parms *p,
rcu_read_unlock();
}
-static void neigh_proc_update(struct ctl_table *ctl, int write)
+static void neigh_proc_update(const struct ctl_table *ctl, int write)
{
struct net_device *dev = ctl->extra1;
struct neigh_parms *p = ctl->extra2;
--
2.44.0
[PATCH v3 03/11] hugetlb: constify ctl_table arguments of utility functions
In a future commit the proc_handlers themselves will change to
"const struct ctl_table". As a preparation for that adapt the internal
helpers.
Signed-off-by: Thomas Weißschuh
---
mm/hugetlb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 3b7d5ddc32ad..8d12ce63a439 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4911,7 +4911,7 @@ static unsigned int allowed_mems_nr(struct hstate *h)
}
#ifdef CONFIG_SYSCTL
-static int proc_hugetlb_doulongvec_minmax(struct ctl_table *table, int write,
+static int proc_hugetlb_doulongvec_minmax(const struct ctl_table *table, int
write,
void *buffer, size_t *length,
loff_t *ppos, unsigned long *out)
{
@@ -4928,7 +4928,7 @@ static int proc_hugetlb_doulongvec_minmax(struct
ctl_table *table, int write,
}
static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
-struct ctl_table *table, int write,
+const struct ctl_table *table, int write,
void *buffer, size_t *length, loff_t *ppos)
{
struct hstate *h = _hstate;
--
2.44.0
