Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()
On Thu, Jan 09, 2025 at 01:40:34PM -0800, Roman Kisel wrote: > > > On 1/9/2025 12:18 PM, Wei Liu wrote: > > On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote: > > [...] > > > Roman Kisel (5): > > >hyperv: Define struct hv_output_get_vp_registers > > >hyperv: Fix pointer type in get_vtl(void) > > >hyperv: Enable the hypercall output page for the VTL mode > > >hyperv: Do not overlap the hvcall IO areas in get_vtl() > > >hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id() > > > > The patches have been pushed to hyperv-next. Roman and Nuno, please > > check the tree for correctness. > > This > > ```c > union hv_arm64_pending_synthetic_exception_event { > u64 as_uint64[2]; > struct { > u8 event_pending : 1; > u8 event_type : 3; > u8 reserved : 4; > u8 rsvd[3]; > u64 context; > } __packed; > }; > ``` > > needs to have the `u32 exception_type;` field: > > ```c > union hv_arm64_pending_synthetic_exception_event { > u64 as_uint64[2]; > struct { > u8 event_pending : 1; > u8 event_type : 3; > u8 reserved : 4; > u8 rsvd[3]; > u32 exception_type; > u64 context; > } __packed; > }; > ``` > as otherwise the struct won't cover the array. > Testing the VMs currently with the latest hyperv-next. Fixed. I c&p'ed the code then deleted the right version of the struct. Thanks for checking. Wei.
Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()
On 1/9/2025 12:28 PM, Nuno Das Neves wrote: On 1/9/2025 12:18 PM, Wei Liu wrote: On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote: [...] Roman Kisel (5): hyperv: Define struct hv_output_get_vp_registers hyperv: Fix pointer type in get_vtl(void) hyperv: Enable the hypercall output page for the VTL mode hyperv: Do not overlap the hvcall IO areas in get_vtl() hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id() The patches have been pushed to hyperv-next. Roman and Nuno, please check the tree for correctness. Thanks, Wei. I checked, looks like the first two patches of the series are missing? IIUC, they were to be rolled up into your earlier patches Nuno -- Thank you, Roman
Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()
On 1/9/2025 1:56 PM, Wei Liu wrote: On Thu, Jan 09, 2025 at 01:40:34PM -0800, Roman Kisel wrote: [...] needs to have the `u32 exception_type;` field: ```c union hv_arm64_pending_synthetic_exception_event { u64 as_uint64[2]; struct { u8 event_pending : 1; u8 event_type : 3; u8 reserved : 4; u8 rsvd[3]; u32 exception_type; u64 context; } __packed; }; ``` as otherwise the struct won't cover the array. Testing the VMs currently with the latest hyperv-next. Fixed. I c&p'ed the code then deleted the right version of the struct. Thanks for checking. Happy to help :D Validated with the VMs, and with the latest hyperv-next, the issue is fixed!! Appreciate your help and guidance; thank you, Easwar, Michael, Nuno, Stanislav, Tianyu and Wei for the suggestions that have let make this patchset so much better :) Borislav, I apologize for sending the patchset versions too often. I'm sorry for causing you trouble due to that. I have read up the kernel documentation, and will be a better citizen of the LKML. Wei. -- Thank you, Roman
Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()
On 1/9/2025 12:18 PM, Wei Liu wrote: On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote: [...] Roman Kisel (5): hyperv: Define struct hv_output_get_vp_registers hyperv: Fix pointer type in get_vtl(void) hyperv: Enable the hypercall output page for the VTL mode hyperv: Do not overlap the hvcall IO areas in get_vtl() hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id() The patches have been pushed to hyperv-next. Roman and Nuno, please check the tree for correctness. This ```c union hv_arm64_pending_synthetic_exception_event { u64 as_uint64[2]; struct { u8 event_pending : 1; u8 event_type : 3; u8 reserved : 4; u8 rsvd[3]; u64 context; } __packed; }; ``` needs to have the `u32 exception_type;` field: ```c union hv_arm64_pending_synthetic_exception_event { u64 as_uint64[2]; struct { u8 event_pending : 1; u8 event_type : 3; u8 reserved : 4; u8 rsvd[3]; u32 exception_type; u64 context; } __packed; }; ``` as otherwise the struct won't cover the array. Testing the VMs currently with the latest hyperv-next. Thanks, Wei. -- Thank you, Roman
[PATCH] treewide: const qualify ctl_tables where applicable
Add the const qualifier to all the ctl_tables in the tree except the ones in ./net dir. The "net" sysctl code is special as it modifies the arrays before passing it on to the registration function. Constifying ctl_table structs will prevent the modification of proc_handler function pointers as the arrays would reside in .rodata. This is made possible after commit 78eb4ea25cd5 ("sysctl: treewide: constify the ctl_table argument of proc_handlers") constified all the proc_handlers. Created this by running an spatch followed by a sed command: Spatch: virtual patch @ depends on !(file in "net") disable optional_qualifier @ identifier table_name; @@ + const struct ctl_table table_name [] = { ... }; sed: sed --in-place \ -e "s/struct ctl_table .table = &uts_kern/const struct ctl_table *table = \&uts_kern/" \ kernel/utsname_sysctl.c Signed-off-by: Joel Granados --- This treewide commit builds upon the work Thomas began a few releases ago [1], where he laid the groundwork for constifying ctl_tables. We implement constification throughout the tree, with the exception of the ctl_tables in the "net" directory. Those are special in that they treat the ctl_table as non-const but we can take them at a later point. Upstreaming: === It is late in the release cycle, but I'm hopeful that we can get this in for the upcoming merge window and this is why: 1. We don't use linux-next: As with previous treewide changes similar to this one [1], we avoid using linux-next in order to avoid unwanted merge conflicts 2. This is a non-functional change: which lowers the probability of unforeseen errors or regressions. 3. It will have at least 2 weeks to be tested/reviewed: The PULL should be sent at the end of the merge window, giving it at least 2 weeks. And if there are more release candidates after rc6, there will be more time. Testing: 1. Currently being tested in 0-day 2. sysctl self-tests/kunit-tests Reduced To/Cc: == b4 originally gave me 200 ppl that this should go out to (which seems a bit overkill from my point of view). So I left the mailing lists and reduced the To: the ppl previously involved in the effort and sysctl maintainers. Please tell me if I missed someone important to the constification effort. Comments are greatly appreciated. Specially interested in any specifics from @Thomas and @kees Best [1] https://lore.kernel.org/20240724210014.mc6nima6cekgi...@joels2.panther.com -- --- arch/arm/kernel/isa.c | 2 +- arch/arm64/kernel/fpsimd.c| 4 ++-- arch/arm64/kernel/process.c | 2 +- arch/powerpc/kernel/idle.c| 2 +- arch/powerpc/platforms/pseries/mobility.c | 2 +- arch/riscv/kernel/process.c | 2 +- arch/riscv/kernel/vector.c| 2 +- arch/s390/appldata/appldata_base.c| 2 +- arch/s390/kernel/debug.c | 2 +- arch/s390/kernel/hiperdispatch.c | 2 +- arch/s390/kernel/topology.c | 2 +- arch/s390/mm/cmm.c| 2 +- arch/s390/mm/pgalloc.c| 2 +- arch/x86/entry/vdso/vdso32-setup.c| 2 +- arch/x86/kernel/cpu/bus_lock.c| 2 +- arch/x86/kernel/itmt.c| 2 +- crypto/fips.c | 2 +- drivers/base/firmware_loader/fallback_table.c | 2 +- drivers/cdrom/cdrom.c | 2 +- drivers/char/hpet.c | 2 +- drivers/char/ipmi/ipmi_poweroff.c | 2 +- drivers/char/random.c | 2 +- drivers/gpu/drm/i915/i915_perf.c | 2 +- drivers/gpu/drm/xe/xe_observation.c | 2 +- drivers/hv/hv_common.c| 2 +- drivers/infiniband/core/iwcm.c| 2 +- drivers/infiniband/core/ucma.c| 2 +- drivers/macintosh/mac_hid.c | 2 +- drivers/md/md.c | 2 +- drivers/misc/sgi-xp/xpc_main.c| 4 ++-- drivers/perf/arm_pmuv3.c | 2 +- drivers/perf/riscv_pmu_sbi.c | 2 +- drivers/scsi/scsi_sysctl.c| 2 +- drivers/scsi/sg.c | 2 +- drivers/tty/tty_io.c | 2 +- drivers/xen/balloon.c | 2 +- fs/aio.c | 2 +- fs/cachefiles/error_inject.c | 2 +- fs/coda/sysctl.c | 2 +- fs/coredump.c | 2 +- fs/dcache.c | 2 +- fs/devpts/inode.c | 2 +- fs/eventpoll.c| 2 +- fs/exec.c | 2 +- fs/file_table.c | 2 +- fs/fuse/sysctl.c
Re: [PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers
On 1/8/2025 9:50 PM, Wei Liu wrote: On Wed, Jan 08, 2025 at 03:25:22PM -0800, Nuno Das Neves wrote: On 1/8/2025 2:21 PM, Roman Kisel wrote: [...] I can fix this when I commit the change . This patch will be folded into your old one anyway. Nuno, thank you very much for spotting that! Wei, appreciate that! Didn't mean to create more work for you, sorry about that. Thanks, Wei. -- Thank you, Roman
Re: [PATCH] treewide: const qualify ctl_tables where applicable
On Thu, Jan 9, 2025 at 5:16 AM Joel Granados wrote: > [...] > drivers/base/firmware_loader/fallback_table.c | 2 +- > drivers/cdrom/cdrom.c | 2 +- > drivers/char/hpet.c | 2 +- > drivers/char/ipmi/ipmi_poweroff.c | 2 +- > drivers/char/random.c | 2 +- > drivers/gpu/drm/i915/i915_perf.c | 2 +- > drivers/gpu/drm/xe/xe_observation.c | 2 +- > drivers/hv/hv_common.c| 2 +- > drivers/infiniband/core/iwcm.c| 2 +- > drivers/infiniband/core/ucma.c| 2 +- > drivers/macintosh/mac_hid.c | 2 +- > drivers/md/md.c | 2 +- For md bits: Reviewed-by: Song Liu Thanks, Song [...]
Re: [PATCH] treewide: const qualify ctl_tables where applicable
On Thu, 09 Jan 2025 14:16:39 +0100 Joel Granados wrote: > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 2e113f8b13a2..489cbab3d64c 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -8786,7 +8786,7 @@ ftrace_enable_sysctl(const struct ctl_table *table, int > write, > return ret; > } > > -static struct ctl_table ftrace_sysctls[] = { > +static const struct ctl_table ftrace_sysctls[] = { > { > .procname = "ftrace_enabled", > .data = &ftrace_enabled, > diff --git a/kernel/trace/trace_events_user.c > b/kernel/trace/trace_events_user.c > index 17bcad8f79de..97325fbd6283 100644 > --- a/kernel/trace/trace_events_user.c > +++ b/kernel/trace/trace_events_user.c > @@ -2899,7 +2899,7 @@ static int set_max_user_events_sysctl(const struct > ctl_table *table, int write, > return ret; > } > > -static struct ctl_table user_event_sysctls[] = { > +static const struct ctl_table user_event_sysctls[] = { > { > .procname = "user_events_max", > .data = &max_user_events, Acked-by: Steven Rostedt (Google) # for kernel/trace/ -- Steve
Re: [PATCH] treewide: const qualify ctl_tables where applicable
On Thu, 09 Jan 2025, Joel Granados wrote: > diff --git a/drivers/gpu/drm/i915/i915_perf.c > b/drivers/gpu/drm/i915/i915_perf.c > index 2406cda75b7b..5384d1bb4923 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -4802,7 +4802,7 @@ int i915_perf_remove_config_ioctl(struct drm_device > *dev, void *data, > return ret; > } > > -static struct ctl_table oa_table[] = { > +static const struct ctl_table oa_table[] = { > { >.procname = "perf_stream_paranoid", >.data = &i915_perf_stream_paranoid, For i915, Acked-by: Jani Nikula -- Jani Nikula, Intel
Re: [PATCH] treewide: const qualify ctl_tables where applicable
Joel, > Add the const qualifier to all the ctl_tables in the tree except the > ones in ./net dir. The "net" sysctl code is special as it modifies the > arrays before passing it on to the registration function. Reviewed-by: Martin K. Petersen # SCSI -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] treewide: const qualify ctl_tables where applicable
On Thu, Jan 09, 2025 at 02:16:39PM +0100, Joel Granados wrote: > Add the const qualifier to all the ctl_tables in the tree except the > ones in ./net dir. The "net" sysctl code is special as it modifies the > arrays before passing it on to the registration function. > ... > diff --git a/drivers/char/ipmi/ipmi_poweroff.c > b/drivers/char/ipmi/ipmi_poweroff.c > index 941d2dcc8c9d..de84f59468a9 100644 > --- a/drivers/char/ipmi/ipmi_poweroff.c > +++ b/drivers/char/ipmi/ipmi_poweroff.c > @@ -650,7 +650,7 @@ static struct ipmi_smi_watcher smi_watcher = { > #ifdef CONFIG_PROC_FS > #include > > -static struct ctl_table ipmi_table[] = { > +static const struct ctl_table ipmi_table[] = { > { .procname = "poweroff_powercycle", > .data = &poweroff_powercycle, > .maxlen = sizeof(poweroff_powercycle), For the IPMI portion: Acked-by: Corey Minyard
Re: [PATCH] treewide: const qualify ctl_tables where applicable
On Thu, Jan 09, 2025 at 02:16:39PM +0100, Joel Granados wrote: > Add the const qualifier to all the ctl_tables in the tree except the > ones in ./net dir. The "net" sysctl code is special as it modifies the > arrays before passing it on to the registration function. > > Constifying ctl_table structs will prevent the modification of > proc_handler function pointers as the arrays would reside in .rodata. > This is made possible after commit 78eb4ea25cd5 ("sysctl: treewide: > constify the ctl_table argument of proc_handlers") constified all the > proc_handlers. Sounds like a good idea, Reviewed-by: "Darrick J. Wong" # xfs --D
Re: [PATCH] rhashtable: Fix potential deadlock by moving schedule_work outside lock
Hello Michael, On Thu, Jan 09, 2025 at 03:16:03AM +, Michael Kelley wrote: > From: Breno Leitao Sent: Thursday, January 2, 2025 2:16 AM > > > > On Sat, Dec 21, 2024 at 05:06:55PM +0800, Herbert Xu wrote: > > > On Thu, Dec 12, 2024 at 08:33:31PM +0800, Herbert Xu wrote: > > > > > > > > The growth check should stay with the atomic_inc. Something like > > > > this should work: > > > > > > OK I've applied your patch with the atomic_inc move. > > > > Sorry, I was on vacation, and I am back now. Let me know if you need > > anything further. > > > > Thanks for fixing it, > > --breno > > Breno and Herbert -- > > This patch seems to break things in linux-next. I'm testing with > linux-next20250108 in a VM in the Azure public cloud. The Mellanox mlx5 > ethernet NIC in the VM is failing to get setup. Thanks for reporting the issue. I started rolling this patch to Meta's fleet, and we started seeing a similar problem. Altough not fully understood yet. I would suggest we revert this patch until we investigate further. I'll prepare and send a revert patch shortly. Sorry for the noise, --breno
Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()
On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote: [...] > Roman Kisel (5): > hyperv: Define struct hv_output_get_vp_registers > hyperv: Fix pointer type in get_vtl(void) > hyperv: Enable the hypercall output page for the VTL mode > hyperv: Do not overlap the hvcall IO areas in get_vtl() > hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id() The patches have been pushed to hyperv-next. Roman and Nuno, please check the tree for correctness. Thanks, Wei.
Re: [PATCH v6 0/5] hyperv: Fixes for get_vtl(), hv_vtl_apicid_to_vp_id()
On 1/9/2025 12:18 PM, Wei Liu wrote: > On Wed, Jan 08, 2025 at 02:21:33PM -0800, Roman Kisel wrote: > [...] >> Roman Kisel (5): >> hyperv: Define struct hv_output_get_vp_registers >> hyperv: Fix pointer type in get_vtl(void) >> hyperv: Enable the hypercall output page for the VTL mode >> hyperv: Do not overlap the hvcall IO areas in get_vtl() >> hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id() > > The patches have been pushed to hyperv-next. Roman and Nuno, please > check the tree for correctness. > > Thanks, > Wei. I checked, looks like the first two patches of the series are missing? Nuno
Re: [PATCH v6 1/5] hyperv: Define struct hv_output_get_vp_registers
On Thu, Jan 09, 2025 at 09:25:58AM -0800, Roman Kisel wrote: > > > On 1/8/2025 9:50 PM, Wei Liu wrote: > > On Wed, Jan 08, 2025 at 03:25:22PM -0800, Nuno Das Neves wrote: > > > On 1/8/2025 2:21 PM, Roman Kisel wrote: > > [...] > > > > > > > > I can fix this when I commit the change . This patch will be folded into > > your old one anyway. > > > Nuno, thank you very much for spotting that! Wei, appreciate that! > Didn't mean to create more work for you, sorry about that. No problem at all. I'm happy to help. Wei. > > > Thanks, > > Wei. > > -- > Thank you, > Roman > >