Re: [PATCH v2] NUMA: Early use of cpu_to_node() returns 0 instead of the correct node id
On Wed, Jan 24, 2024 at 09:19:00AM -0800, Lameter, Christopher wrote: > On Tue, 23 Jan 2024, Huang Shijie wrote: > > > During the kernel booting, the generic cpu_to_node() is called too early in > > arm64, powerpc and riscv when CONFIG_NUMA is enabled. > > > > For arm64/powerpc/riscv, there are at least four places in the common code > > where the generic cpu_to_node() is called before it is initialized: > >1.) early_trace_init() in kernel/trace/trace.c > >2.) sched_init() in kernel/sched/core.c > >3.) init_sched_fair_class()in kernel/sched/fair.c > >4.) workqueue_init_early() in kernel/workqueue.c > > > > In order to fix the bug, the patch changes generic cpu_to_node to > > function pointer, and export it for kernel modules. > > Introduce smp_prepare_boot_cpu_start() to wrap the original > > smp_prepare_boot_cpu(), and set cpu_to_node with early_cpu_to_node. > > Introduce smp_prepare_cpus_done() to wrap the original smp_prepare_cpus(), > > and set the cpu_to_node to formal _cpu_to_node(). > > Would you please fix this cleanly without a function pointer? > > What I think needs to be done is a patch series. > > 1. Instrument cpu_to_node so that some warning is issued if it is used too > early. Preloading the array with NUMA_NO_NODE would allow us to do that. > > 2. Implement early_cpu_to_node on platforms that currently do not have it. > > 3. A series of patches that fix each place where cpu_to_node is used too > early. I think step 3 can be simplified with a generic function that sets per_cpu(numa_node) using early_cpu_to_node(). It can be called right after setup_per_cpu_areas(). -- Sincerely yours, Mike.
Re: ps3_gelic_net.c issue (linux kernel 6.8-rc1)
Hi,
On 1/25/24 15:46, Christophe Leroy wrote:
> Hi,
>
> Le 24/01/2024 à 09:41, sambat goson a écrit :
>>
>> Hi,
>> I've just test it and find below code not proper in function
>> "gelic_descr_prepare_rx", line 398.
>> it causes error as my attached file.
>>
>> descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size);
>> if (!descr->skb) {
>> descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
>> return -ENOMEM;
>> }
>> descr->hw_regs.dmac_cmd_status = 0;
>> descr->hw_regs.result_size = 0;
>> descr->hw_regs.valid_size = 0;
>> descr->hw_regs.data_error = 0;
>> descr->hw_regs.payload.dev_addr = 0;
>> descr->hw_regs.payload.size = 0;
>> descr->skb = NULL; >line 398
>>
>
> Looks like a copy/paste error from gelic_descr_release_tx() in commit
> 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures"), the
> whole block is wrong I guess, not only the descr->skb = NULL;
>
> Geoff, can you fix it ?
Thanks for reporting the problem. I'll look into fixing it.
-Geoff
Re: [PATCH] powerpc/iommu: Code cleanup for cell/iommu.c
Le 25/01/2024 à 03:46, Kunwu Chan a écrit :
> This part was commented from commit 165785e5c0be ("[POWERPC] Cell
> iommu support") in about 17 years before.
>
> If there are no plans to enable this part code in the future,
> we can remove this dead code.
>
> Signed-off-by: Kunwu Chan
> ---
> arch/powerpc/platforms/cell/iommu.c | 16
> 1 file changed, 16 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/iommu.c
> b/arch/powerpc/platforms/cell/iommu.c
> index 1202a69b0a20..afce9e64a443 100644
> --- a/arch/powerpc/platforms/cell/iommu.c
> +++ b/arch/powerpc/platforms/cell/iommu.c
> @@ -424,22 +424,6 @@ static void __init cell_iommu_setup_hardware(struct
> cbe_iommu *iommu,
> cell_iommu_enable_hardware(iommu);
> }
>
> -#if 0/* Unused for now */
> -static struct iommu_window *find_window(struct cbe_iommu *iommu,
> - unsigned long offset, unsigned long size)
> -{
> - struct iommu_window *window;
> -
> - /* todo: check for overlapping (but not equal) windows) */
> -
> - list_for_each_entry(window, &(iommu->windows), list) {
> - if (window->offset == offset && window->size == size)
> - return window;
> - }
> -
> - return NULL;
> -}
> -#endif
Same as the other one, please remove the second blank line, don't leave
two blank lines between the remaining functions.
>
> static inline u32 cell_iommu_get_ioid(struct device_node *np)
> {
Re: ps3_gelic_net.c issue (linux kernel 6.8-rc1)
Hi,
Le 24/01/2024 à 09:41, sambat goson a écrit :
>
> Hi,
> I've just test it and find below code not proper in function
> "gelic_descr_prepare_rx", line 398.
> it causes error as my attached file.
>
> descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size);
> if (!descr->skb) {
> descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
> return -ENOMEM;
> }
> descr->hw_regs.dmac_cmd_status = 0;
> descr->hw_regs.result_size = 0;
> descr->hw_regs.valid_size = 0;
> descr->hw_regs.data_error = 0;
> descr->hw_regs.payload.dev_addr = 0;
> descr->hw_regs.payload.size = 0;
> descr->skb = NULL; >line 398
>
Looks like a copy/paste error from gelic_descr_release_tx() in commit
3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures"), the
whole block is wrong I guess, not only the descr->skb = NULL;
Geoff, can you fix it ?
Christophe
RE: [PATCH linux-next v3 06/14] x86, crash: wrap crash dumping code into crash related ifdefs
From: Baoquan He Sent: Wednesday, January 24, 2024 8:10 PM
>
> On 01/24/24 at 11:02pm, Michael Kelley wrote:
> > > diff --git a/arch/x86/kernel/cpu/mshyperv.c
> > > b/arch/x86/kernel/cpu/mshyperv.c
> > > index 01fa06dd06b6..f8163a59026b 100644
> > > --- a/arch/x86/kernel/cpu/mshyperv.c
> > > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > > @@ -210,6 +210,7 @@ static void hv_machine_shutdown(void)
> > > hyperv_cleanup();
> > > }
> > >
> > > +#ifdef CONFIG_CRASH_DUMP
> > > static void hv_machine_crash_shutdown(struct pt_regs *regs)
> > > {
> > > if (hv_crash_handler)
> > > @@ -221,6 +222,7 @@ static void hv_machine_crash_shutdown(struct pt_regs
> > > *regs)
> > > /* Disable the hypercall page when there is only 1 active CPU. */
> > > hyperv_cleanup();
> > > }
> > > +#endif
> > > #endif /* CONFIG_KEXEC_CORE */
> >
> > Note that the #ifdef CONFIG_CRASH_DUMP is nested inside
> > #ifdef CONFIG_KEXEC_CODE here, and in the other Hyper-V code
> > just below. It's also nested in xen_hvm_guest_init() at the bottom
> > of this patch. But the KVM case of setting crash_shutdown is
> > not nested -- you changed #ifdef CONFIG_KEXEC_CORE to #ifdef
> > CONFIG_CRASH_DUMP.
> >
> > I think both approaches work because CONFIG_CRASH_DUMP implies
> > CONFIG_KEXEC_CORE, but I wonder if it would be better to *not* nest
> > in all cases. I'd like to see the cases be consistent so in the future
> > someone doesn't wonder why there's a difference (unless there's
> > a reason for the difference that I missed).
>
> I agree with you, it's a great suggestion. Thanks.
>
> Do you think below draft patch includes all changes you are concerned
> about?
Yes, these changes look good as a delta to your original patch.
But also look at xen_hvm_guest_init(). It looks like your original patch
does nesting there as well, and it could probably be "un-nested".
Michael
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> index f8163a59026b..2e8cd5a4ae85 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -209,6 +209,7 @@ static void hv_machine_shutdown(void)
> if (kexec_in_progress)
> hyperv_cleanup();
> }
> +#endif /* CONFIG_KEXEC_CORE */
>
> #ifdef CONFIG_CRASH_DUMP
> static void hv_machine_crash_shutdown(struct pt_regs *regs)
> @@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct
> pt_regs *regs)
> /* Disable the hypercall page when there is only 1 active CPU. */
> hyperv_cleanup();
> }
> -#endif
> -#endif /* CONFIG_KEXEC_CORE */
> +#endif /* CONFIG_CRASH_DUMP */
> #endif /* CONFIG_HYPERV */
>
> static uint32_t __init ms_hyperv_platform(void)
> @@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void)
> no_timer_check = 1;
> #endif
>
> -#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE)
> +#if IS_ENABLED(CONFIG_HYPERV)
> +#if defined(CONFIG_KEXEC_CORE)
> machine_ops.shutdown = hv_machine_shutdown;
> -#ifdef CONFIG_CRASH_DUMP
> +#endif
> +#if defined(CONFIG_CRASH_DUMP)
> machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> #endif
> #endif
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 1287b0d5962f..f3130f762784 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -826,7 +826,7 @@ void machine_halt(void)
> machine_ops.halt();
> }
>
> -#ifdef CONFIG_KEXEC_CORE
> +#ifdef CONFIG_CRASH_DUMP
> void machine_crash_shutdown(struct pt_regs *regs)
> {
> machine_ops.crash_shutdown(regs);
Re: [PATCH linux-next v3 06/14] x86, crash: wrap crash dumping code into crash related ifdefs
On 01/24/24 at 11:02pm, Michael Kelley wrote:
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c
> > b/arch/x86/kernel/cpu/mshyperv.c
> > index 01fa06dd06b6..f8163a59026b 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -210,6 +210,7 @@ static void hv_machine_shutdown(void)
> > hyperv_cleanup();
> > }
> >
> > +#ifdef CONFIG_CRASH_DUMP
> > static void hv_machine_crash_shutdown(struct pt_regs *regs)
> > {
> > if (hv_crash_handler)
> > @@ -221,6 +222,7 @@ static void hv_machine_crash_shutdown(struct
> > pt_regs *regs)
> > /* Disable the hypercall page when there is only 1 active CPU. */
> > hyperv_cleanup();
> > }
> > +#endif
> > #endif /* CONFIG_KEXEC_CORE */
>
> Note that the #ifdef CONFIG_CRASH_DUMP is nested inside
> #ifdef CONFIG_KEXEC_CODE here, and in the other Hyper-V code
> just below. It's also nested in xen_hvm_guest_init() at the bottom
> of this patch. But the KVM case of setting crash_shutdown is
> not nested -- you changed #ifdef CONFIG_KEXEC_CORE to #ifdef
> CONFIG_CRASH_DUMP.
>
> I think both approaches work because CONFIG_CRASH_DUMP implies
> CONFIG_KEXEC_CORE, but I wonder if it would be better to *not* nest
> in all cases. I'd like to see the cases be consistent so in the future
> someone doesn't wonder why there's a difference (unless there's
> a reason for the difference that I missed).
I agree with you, it's a great suggestion. Thanks.
Do you think below draft patch includes all changes you are concerned
about?
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index f8163a59026b..2e8cd5a4ae85 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -209,6 +209,7 @@ static void hv_machine_shutdown(void)
if (kexec_in_progress)
hyperv_cleanup();
}
+#endif /* CONFIG_KEXEC_CORE */
#ifdef CONFIG_CRASH_DUMP
static void hv_machine_crash_shutdown(struct pt_regs *regs)
@@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs)
/* Disable the hypercall page when there is only 1 active CPU. */
hyperv_cleanup();
}
-#endif
-#endif /* CONFIG_KEXEC_CORE */
+#endif /* CONFIG_CRASH_DUMP */
#endif /* CONFIG_HYPERV */
static uint32_t __init ms_hyperv_platform(void)
@@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void)
no_timer_check = 1;
#endif
-#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE)
+#if IS_ENABLED(CONFIG_HYPERV)
+#if defined(CONFIG_KEXEC_CORE)
machine_ops.shutdown = hv_machine_shutdown;
-#ifdef CONFIG_CRASH_DUMP
+#endif
+#if defined(CONFIG_CRASH_DUMP)
machine_ops.crash_shutdown = hv_machine_crash_shutdown;
#endif
#endif
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1287b0d5962f..f3130f762784 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -826,7 +826,7 @@ void machine_halt(void)
machine_ops.halt();
}
-#ifdef CONFIG_KEXEC_CORE
+#ifdef CONFIG_CRASH_DUMP
void machine_crash_shutdown(struct pt_regs *regs)
{
machine_ops.crash_shutdown(regs);
[PATCH] powerpc/iommu: Code cleanup for cell/iommu.c
This part was commented from commit 165785e5c0be ("[POWERPC] Cell
iommu support") in about 17 years before.
If there are no plans to enable this part code in the future,
we can remove this dead code.
Signed-off-by: Kunwu Chan
---
arch/powerpc/platforms/cell/iommu.c | 16
1 file changed, 16 deletions(-)
diff --git a/arch/powerpc/platforms/cell/iommu.c
b/arch/powerpc/platforms/cell/iommu.c
index 1202a69b0a20..afce9e64a443 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -424,22 +424,6 @@ static void __init cell_iommu_setup_hardware(struct
cbe_iommu *iommu,
cell_iommu_enable_hardware(iommu);
}
-#if 0/* Unused for now */
-static struct iommu_window *find_window(struct cbe_iommu *iommu,
- unsigned long offset, unsigned long size)
-{
- struct iommu_window *window;
-
- /* todo: check for overlapping (but not equal) windows) */
-
- list_for_each_entry(window, &(iommu->windows), list) {
- if (window->offset == offset && window->size == size)
- return window;
- }
-
- return NULL;
-}
-#endif
static inline u32 cell_iommu_get_ioid(struct device_node *np)
{
--
2.39.2
Re: [PATCH v2] NUMA: Early use of cpu_to_node() returns 0 instead of the correct node id
在 2024/1/25 1:19, Lameter, Christopher 写道: On Tue, 23 Jan 2024, Huang Shijie wrote: During the kernel booting, the generic cpu_to_node() is called too early in arm64, powerpc and riscv when CONFIG_NUMA is enabled. For arm64/powerpc/riscv, there are at least four places in the common code where the generic cpu_to_node() is called before it is initialized: 1.) early_trace_init() in kernel/trace/trace.c 2.) sched_init() in kernel/sched/core.c 3.) init_sched_fair_class() in kernel/sched/fair.c 4.) workqueue_init_early() in kernel/workqueue.c In order to fix the bug, the patch changes generic cpu_to_node to function pointer, and export it for kernel modules. Introduce smp_prepare_boot_cpu_start() to wrap the original smp_prepare_boot_cpu(), and set cpu_to_node with early_cpu_to_node. Introduce smp_prepare_cpus_done() to wrap the original smp_prepare_cpus(), and set the cpu_to_node to formal _cpu_to_node(). Would you please fix this cleanly without a function pointer? What I think needs to be done is a patch series. 1. Instrument cpu_to_node so that some warning is issued if it is used too early. Preloading the array with NUMA_NO_NODE would allow us to do that. 2. Implement early_cpu_to_node on platforms that currently do not have it. 3. A series of patches that fix each place where cpu_to_node is used too early. okay, I will try to do it. Thanks Huang Shijie
RE: [PATCH linux-next v3 06/14] x86, crash: wrap crash dumping code into crash related ifdefs
From: Baoquan He Sent: Tuesday, January 23, 2024 9:13 PM
>
> Now crash codes under kernel/ folder has been split out from kexec
> code, crash dumping can be separated from kexec reboot in config
> items on x86 with some adjustments.
>
> Here, also change some ifdefs or IS_ENABLED() check to more appropriate
> ones, e,g
> - #ifdef CONFIG_KEXEC_CORE -> #ifdef CONFIG_CRASH_DUMP
> - (!IS_ENABLED(CONFIG_KEXEC_CORE)) - >
> (!IS_ENABLED(CONFIG_CRASH_RESERVE))
>
> Signed-off-by: Baoquan He
> ---
> arch/x86/kernel/Makefile | 4 ++--
> arch/x86/kernel/cpu/mshyperv.c | 4
> arch/x86/kernel/kexec-bzimage64.c | 4
> arch/x86/kernel/kvm.c | 4 ++--
> arch/x86/kernel/machine_kexec_64.c | 3 +++
> arch/x86/kernel/reboot.c | 2 +-
> arch/x86/kernel/setup.c| 2 +-
> arch/x86/kernel/smp.c | 2 +-
> arch/x86/xen/enlighten_hvm.c | 4
> 9 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 913d4022131e..3668b1edef2d 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -100,9 +100,9 @@ obj-$(CONFIG_TRACING) += trace.o
> obj-$(CONFIG_RETHOOK)+= rethook.o
> obj-$(CONFIG_VMCORE_INFO)+= vmcore_info_$(BITS).o
> obj-$(CONFIG_KEXEC_CORE) += machine_kexec_$(BITS).o
> -obj-$(CONFIG_KEXEC_CORE) += relocate_kernel_$(BITS).o crash.o
> +obj-$(CONFIG_KEXEC_CORE) += relocate_kernel_$(BITS).o
> obj-$(CONFIG_KEXEC_FILE) += kexec-bzimage64.o
> -obj-$(CONFIG_CRASH_DUMP) += crash_dump_$(BITS).o
> +obj-$(CONFIG_CRASH_DUMP) += crash_dump_$(BITS).o crash.o
> obj-y+= kprobes/
> obj-$(CONFIG_MODULES)+= module.o
> obj-$(CONFIG_X86_32) += doublefault_32.o
> diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> index 01fa06dd06b6..f8163a59026b 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -210,6 +210,7 @@ static void hv_machine_shutdown(void)
> hyperv_cleanup();
> }
>
> +#ifdef CONFIG_CRASH_DUMP
> static void hv_machine_crash_shutdown(struct pt_regs *regs)
> {
> if (hv_crash_handler)
> @@ -221,6 +222,7 @@ static void hv_machine_crash_shutdown(struct
> pt_regs *regs)
> /* Disable the hypercall page when there is only 1 active CPU. */
> hyperv_cleanup();
> }
> +#endif
> #endif /* CONFIG_KEXEC_CORE */
Note that the #ifdef CONFIG_CRASH_DUMP is nested inside
#ifdef CONFIG_KEXEC_CODE here, and in the other Hyper-V code
just below. It's also nested in xen_hvm_guest_init() at the bottom
of this patch. But the KVM case of setting crash_shutdown is
not nested -- you changed #ifdef CONFIG_KEXEC_CORE to #ifdef
CONFIG_CRASH_DUMP.
I think both approaches work because CONFIG_CRASH_DUMP implies
CONFIG_KEXEC_CORE, but I wonder if it would be better to *not* nest
in all cases. I'd like to see the cases be consistent so in the future
someone doesn't wonder why there's a difference (unless there's
a reason for the difference that I missed).
> #endif /* CONFIG_HYPERV */
>
> @@ -497,7 +499,9 @@ static void __init ms_hyperv_init_platform(void)
>
> #if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE)
> machine_ops.shutdown = hv_machine_shutdown;
> +#ifdef CONFIG_CRASH_DUMP
> machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> +#endif
> #endif
> if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> /*
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-
> bzimage64.c
> index 2a422e00ed4b..b55737b83a84 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -263,11 +263,13 @@ setup_boot_parameters(struct kimage *image,
> struct boot_params *params,
> memset(>hd0_info, 0, sizeof(params->hd0_info));
> memset(>hd1_info, 0, sizeof(params->hd1_info));
>
> +#ifdef CONFIG_CRASH_DUMP
> if (image->type == KEXEC_TYPE_CRASH) {
> ret = crash_setup_memmap_entries(image, params);
> if (ret)
> return ret;
> } else
> +#endif
> setup_e820_entries(params);
>
> nr_e820_entries = params->e820_entries;
> @@ -433,12 +435,14 @@ static void *bzImage64_load(struct kimage *image, char
> *kernel,
> return ERR_PTR(-EINVAL);
> }
>
> +#ifdef CONFIG_CRASH_DUMP
> /* Allocate and load backup region */
> if (image->type == KEXEC_TYPE_CRASH) {
> ret = crash_load_segments(image);
> if (ret)
> return ERR_PTR(ret);
> }
> +#endif
>
> /*
>* Load purgatory. For 64bit entry point, purgatory code can be
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index dfe9945b9bec..acfc2d3183bc 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -769,7 +769,7 @@ static
Re: [PATCH v2] NUMA: Early use of cpu_to_node() returns 0 instead of the correct node id
On Tue, 23 Jan 2024, Huang Shijie wrote: During the kernel booting, the generic cpu_to_node() is called too early in arm64, powerpc and riscv when CONFIG_NUMA is enabled. For arm64/powerpc/riscv, there are at least four places in the common code where the generic cpu_to_node() is called before it is initialized: 1.) early_trace_init() in kernel/trace/trace.c 2.) sched_init() in kernel/sched/core.c 3.) init_sched_fair_class()in kernel/sched/fair.c 4.) workqueue_init_early() in kernel/workqueue.c In order to fix the bug, the patch changes generic cpu_to_node to function pointer, and export it for kernel modules. Introduce smp_prepare_boot_cpu_start() to wrap the original smp_prepare_boot_cpu(), and set cpu_to_node with early_cpu_to_node. Introduce smp_prepare_cpus_done() to wrap the original smp_prepare_cpus(), and set the cpu_to_node to formal _cpu_to_node(). Would you please fix this cleanly without a function pointer? What I think needs to be done is a patch series. 1. Instrument cpu_to_node so that some warning is issued if it is used too early. Preloading the array with NUMA_NO_NODE would allow us to do that. 2. Implement early_cpu_to_node on platforms that currently do not have it. 3. A series of patches that fix each place where cpu_to_node is used too early.
Re: [PATCH 3/4] net: wan: fsl_qmc_hdlc: Add runtime timeslots changes support
On 24/01/2024 15:26, Herve Codina wrote:
Hi Vadim,
On Wed, 24 Jan 2024 10:10:46 +
Vadim Fedorenko wrote:
[...]
+static int qmc_hdlc_xlate_slot_map(struct qmc_hdlc *qmc_hdlc,
+ u32 slot_map, struct qmc_chan_ts_info
*ts_info)
+{
+ u64 ts_mask_avail;
+ unsigned int bit;
+ unsigned int i;
+ u64 ts_mask;
+ u64 map;
+
+ /* Tx and Rx masks must be identical */
+ if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) {
+ dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch
(0x%llx, 0x%llx)\n",
+ ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail);
+ return -EINVAL;
+ }
+
+ ts_mask_avail = ts_info->rx_ts_mask_avail;
+ ts_mask = 0;
+ map = slot_map;
+ bit = 0;
+ for (i = 0; i < 64; i++) {
+ if (ts_mask_avail & BIT_ULL(i)) {
+ if (map & BIT_ULL(bit))
+ ts_mask |= BIT_ULL(i);
+ bit++;
+ }
+ }
+
+ if (hweight64(ts_mask) != hweight64(map)) {
+ dev_err(qmc_hdlc->dev, "Cannot translate timeslots 0x%llx ->
(0x%llx,0x%llx)\n",
+ map, ts_mask_avail, ts_mask);
+ return -EINVAL;
+ }
+
+ ts_info->tx_ts_mask = ts_mask;
+ ts_info->rx_ts_mask = ts_mask;
+ return 0;
+}
+
+static int qmc_hdlc_xlate_ts_info(struct qmc_hdlc *qmc_hdlc,
+ const struct qmc_chan_ts_info *ts_info, u32
*slot_map)
+{
+ u64 ts_mask_avail;
+ unsigned int bit;
+ unsigned int i;
+ u64 ts_mask;
+ u64 map;
+
Starting from here ...
+ /* Tx and Rx masks must be identical */
+ if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) {
+ dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch
(0x%llx, 0x%llx)\n",
+ ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail);
+ return -EINVAL;
+ }
+ if (ts_info->rx_ts_mask != ts_info->tx_ts_mask) {
+ dev_err(qmc_hdlc->dev, "tx and rx timeslots mismatch (0x%llx,
0x%llx)\n",
+ ts_info->rx_ts_mask, ts_info->tx_ts_mask);
+ return -EINVAL;
+ }
+
+ ts_mask_avail = ts_info->rx_ts_mask_avail;
+ ts_mask = ts_info->rx_ts_mask;
+ map = 0;
+ bit = 0;
+ for (i = 0; i < 64; i++) {
+ if (ts_mask_avail & BIT_ULL(i)) {
+ if (ts_mask & BIT_ULL(i))
+ map |= BIT_ULL(bit);
+ bit++;
+ }
+ }
+
+ if (hweight64(ts_mask) != hweight64(map)) {
+ dev_err(qmc_hdlc->dev, "Cannot translate timeslots (0x%llx,0x%llx)
-> 0x%llx\n",
+ ts_mask_avail, ts_mask, map);
+ return -EINVAL;
+ }
+
till here the block looks like copy of the block from previous function.
It worth to make a separate function for it, I think.
+ if (map >= BIT_ULL(32)) {
+ dev_err(qmc_hdlc->dev, "Slot map out of 32bit (0x%llx,0x%llx) ->
0x%llx\n",
+ ts_mask_avail, ts_mask, map);
+ return -EINVAL;
+ }
+
+ *slot_map = map;
+ return 0;
+}
+
[...]
I am not so sure. There are slighty differences between the two functions.
The error messages and, in particular, the loop in qmc_hdlc_xlate_slot_map() is:
--- 8< ---
ts_mask_avail = ts_info->rx_ts_mask_avail;
ts_mask = 0;
map = slot_map;
bit = 0;
for (i = 0; i < 64; i++) {
if (ts_mask_avail & BIT_ULL(i)) {
if (map & BIT_ULL(bit))
ts_mask |= BIT_ULL(i);
bit++;
}
}
--- 8< ---
whereas it is the following in qmc_hdlc_xlate_ts_info():
--- 8< ---
ts_mask_avail = ts_info->rx_ts_mask_avail;
ts_mask = ts_info->rx_ts_mask;
map = 0;
bit = 0;
for (i = 0; i < 64; i++) {
if (ts_mask_avail & BIT_ULL(i)) {
if (ts_mask & BIT_ULL(i))
map |= BIT_ULL(bit);
bit++;
}
}
--- 8< ---
ts_map and map initializations are not the same, i and bit are not used for
the same purpose and the computed value is not computed based on the same
information.
With that pointed, I am not sure that having some common code for both
function will be relevant. Your opinion ?
I see. I'm just thinking if it's possible to use helpers from bitops.h
and bitmap.h here to avoid open-coding common parts of the code.
Best regards,
Hervé
Re: [PATCH 3/4] net: wan: fsl_qmc_hdlc: Add runtime timeslots changes support
On 23/01/2024 16:49, Herve Codina wrote:
QMC channels support runtime timeslots changes but nothing is done at
the QMC HDLC driver to handle these changes.
Use existing IFACE ioctl in order to configure the timeslots to use.
Signed-off-by: Herve Codina
Reviewed-by: Christophe Leroy
Acked-by: Jakub Kicinski
---
drivers/net/wan/fsl_qmc_hdlc.c | 169 -
1 file changed, 168 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c
index 31b637ec8390..82019cd96365 100644
--- a/drivers/net/wan/fsl_qmc_hdlc.c
+++ b/drivers/net/wan/fsl_qmc_hdlc.c
@@ -32,6 +32,7 @@ struct qmc_hdlc {
struct qmc_hdlc_desc tx_descs[8];
unsigned int tx_out;
struct qmc_hdlc_desc rx_descs[4];
+ u32 slot_map;
};
static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev)
@@ -202,6 +203,162 @@ static netdev_tx_t qmc_hdlc_xmit(struct sk_buff *skb,
struct net_device *netdev)
return NETDEV_TX_OK;
}
+static int qmc_hdlc_xlate_slot_map(struct qmc_hdlc *qmc_hdlc,
+ u32 slot_map, struct qmc_chan_ts_info
*ts_info)
+{
+ u64 ts_mask_avail;
+ unsigned int bit;
+ unsigned int i;
+ u64 ts_mask;
+ u64 map;
+
+ /* Tx and Rx masks must be identical */
+ if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) {
+ dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch
(0x%llx, 0x%llx)\n",
+ ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail);
+ return -EINVAL;
+ }
+
+ ts_mask_avail = ts_info->rx_ts_mask_avail;
+ ts_mask = 0;
+ map = slot_map;
+ bit = 0;
+ for (i = 0; i < 64; i++) {
+ if (ts_mask_avail & BIT_ULL(i)) {
+ if (map & BIT_ULL(bit))
+ ts_mask |= BIT_ULL(i);
+ bit++;
+ }
+ }
+
+ if (hweight64(ts_mask) != hweight64(map)) {
+ dev_err(qmc_hdlc->dev, "Cannot translate timeslots 0x%llx ->
(0x%llx,0x%llx)\n",
+ map, ts_mask_avail, ts_mask);
+ return -EINVAL;
+ }
+
+ ts_info->tx_ts_mask = ts_mask;
+ ts_info->rx_ts_mask = ts_mask;
+ return 0;
+}
+
+static int qmc_hdlc_xlate_ts_info(struct qmc_hdlc *qmc_hdlc,
+ const struct qmc_chan_ts_info *ts_info, u32
*slot_map)
+{
+ u64 ts_mask_avail;
+ unsigned int bit;
+ unsigned int i;
+ u64 ts_mask;
+ u64 map;
+
Starting from here ...
+ /* Tx and Rx masks must be identical */
+ if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) {
+ dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch
(0x%llx, 0x%llx)\n",
+ ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail);
+ return -EINVAL;
+ }
+ if (ts_info->rx_ts_mask != ts_info->tx_ts_mask) {
+ dev_err(qmc_hdlc->dev, "tx and rx timeslots mismatch (0x%llx,
0x%llx)\n",
+ ts_info->rx_ts_mask, ts_info->tx_ts_mask);
+ return -EINVAL;
+ }
+
+ ts_mask_avail = ts_info->rx_ts_mask_avail;
+ ts_mask = ts_info->rx_ts_mask;
+ map = 0;
+ bit = 0;
+ for (i = 0; i < 64; i++) {
+ if (ts_mask_avail & BIT_ULL(i)) {
+ if (ts_mask & BIT_ULL(i))
+ map |= BIT_ULL(bit);
+ bit++;
+ }
+ }
+
+ if (hweight64(ts_mask) != hweight64(map)) {
+ dev_err(qmc_hdlc->dev, "Cannot translate timeslots (0x%llx,0x%llx)
-> 0x%llx\n",
+ ts_mask_avail, ts_mask, map);
+ return -EINVAL;
+ }
+
till here the block looks like copy of the block from previous function.
It worth to make a separate function for it, I think.
+ if (map >= BIT_ULL(32)) {
+ dev_err(qmc_hdlc->dev, "Slot map out of 32bit (0x%llx,0x%llx) ->
0x%llx\n",
+ ts_mask_avail, ts_mask, map);
+ return -EINVAL;
+ }
+
+ *slot_map = map;
+ return 0;
+}
+
+static int qmc_hdlc_set_iface(struct qmc_hdlc *qmc_hdlc, int if_iface, const
te1_settings *te1)
+{
+ struct qmc_chan_ts_info ts_info;
+ int ret;
+
+ ret = qmc_chan_get_ts_info(qmc_hdlc->qmc_chan, _info);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "get QMC channel ts info failed %d\n",
ret);
+ return ret;
+ }
+ ret = qmc_hdlc_xlate_slot_map(qmc_hdlc, te1->slot_map, _info);
+ if (ret)
+ return ret;
+
+ ret = qmc_chan_set_ts_info(qmc_hdlc->qmc_chan, _info);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "set QMC channel ts info failed %d\n",
ret);
+ return ret;
+ }
+
+ qmc_hdlc->slot_map =
Re: [PATCH 1/4] net: wan: Add support for QMC HDLC
On 23/01/2024 16:49, Herve Codina wrote:
The QMC HDLC driver provides support for HDLC using the QMC (QUICC
Multichannel Controller) to transfer the HDLC data.
Signed-off-by: Herve Codina
Reviewed-by: Christophe Leroy
Acked-by: Jakub Kicinski
---
drivers/net/wan/Kconfig| 12 +
drivers/net/wan/Makefile | 1 +
drivers/net/wan/fsl_qmc_hdlc.c | 422 +
3 files changed, 435 insertions(+)
create mode 100644 drivers/net/wan/fsl_qmc_hdlc.c
diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig
index 7dda87756d3f..31ab2136cdf1 100644
--- a/drivers/net/wan/Kconfig
+++ b/drivers/net/wan/Kconfig
@@ -197,6 +197,18 @@ config FARSYNC
To compile this driver as a module, choose M here: the
module will be called farsync.
+config FSL_QMC_HDLC
+ tristate "Freescale QMC HDLC support"
+ depends on HDLC
+ depends on CPM_QMC
+ help
+ HDLC support using the Freescale QUICC Multichannel Controller (QMC).
+
+ To compile this driver as a module, choose M here: the
+ module will be called fsl_qmc_hdlc.
+
+ If unsure, say N.
+
config FSL_UCC_HDLC
tristate "Freescale QUICC Engine HDLC support"
depends on HDLC
diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile
index 8119b49d1da9..00e9b7ee1e01 100644
--- a/drivers/net/wan/Makefile
+++ b/drivers/net/wan/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_WANXL) += wanxl.o
obj-$(CONFIG_PCI200SYN) += pci200syn.o
obj-$(CONFIG_PC300TOO)+= pc300too.o
obj-$(CONFIG_IXP4XX_HSS) += ixp4xx_hss.o
+obj-$(CONFIG_FSL_QMC_HDLC) += fsl_qmc_hdlc.o
obj-$(CONFIG_FSL_UCC_HDLC)+= fsl_ucc_hdlc.o
obj-$(CONFIG_SLIC_DS26522)+= slic_ds26522.o
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c
new file mode 100644
index ..31b637ec8390
--- /dev/null
+++ b/drivers/net/wan/fsl_qmc_hdlc.c
@@ -0,0 +1,422 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Freescale QMC HDLC Device Driver
+ *
+ * Copyright 2023 CS GROUP France
+ *
+ * Author: Herve Codina
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct qmc_hdlc_desc {
+ struct net_device *netdev;
+ struct sk_buff *skb; /* NULL if the descriptor is not in use */
+ dma_addr_t dma_addr;
+ size_t dma_size;
+};
+
+struct qmc_hdlc {
+ struct device *dev;
+ struct qmc_chan *qmc_chan;
+ struct net_device *netdev;
+ bool is_crc32;
+ spinlock_t tx_lock; /* Protect tx descriptors */
+ struct qmc_hdlc_desc tx_descs[8];
+ unsigned int tx_out;
+ struct qmc_hdlc_desc rx_descs[4];
+};
+
+static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev)
+{
+ return dev_to_hdlc(netdev)->priv;
+}
+
+static int qmc_hdlc_recv_queue(struct qmc_hdlc *qmc_hdlc, struct qmc_hdlc_desc
*desc, size_t size);
+
+#define QMC_HDLC_RX_ERROR_FLAGS (QMC_RX_FLAG_HDLC_OVF | \
+QMC_RX_FLAG_HDLC_UNA | \
+QMC_RX_FLAG_HDLC_ABORT | \
+QMC_RX_FLAG_HDLC_CRC)
+
+static void qmc_hcld_recv_complete(void *context, size_t length, unsigned int
flags)
+{
+ struct qmc_hdlc_desc *desc = context;
+ struct net_device *netdev = desc->netdev;
+ struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(desc->netdev);
a line above desc->netdev was stored in netdev. better to reuse it and
make declaration part consistent with qmc_hcld_xmit_complete
+ int ret;
+
+ dma_unmap_single(qmc_hdlc->dev, desc->dma_addr, desc->dma_size,
DMA_FROM_DEVICE);
+
+ if (flags & QMC_HDLC_RX_ERROR_FLAGS) {
+ netdev->stats.rx_errors++;
+ if (flags & QMC_RX_FLAG_HDLC_OVF) /* Data overflow */
+ netdev->stats.rx_over_errors++;
+ if (flags & QMC_RX_FLAG_HDLC_UNA) /* bits received not multiple
of 8 */
+ netdev->stats.rx_frame_errors++;
+ if (flags & QMC_RX_FLAG_HDLC_ABORT) /* Received an abort
sequence */
+ netdev->stats.rx_frame_errors++;
+ if (flags & QMC_RX_FLAG_HDLC_CRC) /* CRC error */
+ netdev->stats.rx_crc_errors++;
+ kfree_skb(desc->skb);
+ } else {
+ netdev->stats.rx_packets++;
+ netdev->stats.rx_bytes += length;
+
+ skb_put(desc->skb, length);
+ desc->skb->protocol = hdlc_type_trans(desc->skb, netdev);
+ netif_rx(desc->skb);
+ }
+
+ /* Re-queue a transfer using the same descriptor */
+ ret = qmc_hdlc_recv_queue(qmc_hdlc, desc, desc->dma_size);
+ if (ret) {
+ dev_err(qmc_hdlc->dev, "queue recv desc failed (%d)\n", ret);
+ netdev->stats.rx_errors++;
+ }
+}
+
+static int qmc_hdlc_recv_queue(struct
ps3_gelic_net.c issue (linux kernel 6.8-rc1)
Hi,
I've just test it and find below code not proper in function
"gelic_descr_prepare_rx", line 398.
it causes error as my attached file.
descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size);
if (!descr->skb) {
descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
return -ENOMEM;
}
descr->hw_regs.dmac_cmd_status = 0;
descr->hw_regs.result_size = 0;
descr->hw_regs.valid_size = 0;
descr->hw_regs.data_error = 0;
descr->hw_regs.payload.dev_addr = 0;
descr->hw_regs.payload.size = 0;
descr->skb = NULL; >line 398
Best regards,
Sombat T.
[ 14.851260] BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 NUMA PS3
[ 14.851364] Modules linked in: ps3_gelic(+) ps3fb ps3rom soundcore ps3_lpm
usb_common spufs fuse configfs autofs4
[ 14.851663] CPU: 1 PID: 578 Comm: (udev-worker) Not tainted 6.8.0 #1
[ 14.851779] Hardware name: SonyPS3 Cell Broadband Engine 0x702100 PS3
[ 14.851892] NIP: c0003d07ec40 LR: c0003d07ebfc CTR:
[ 14.852021] REGS: c4f3ef80 TRAP: 0300 Not tainted (6.8.0)
[ 14.852140] MSR: 8200a032 CR: 84222820
XER:
[ 14.852387] DAR: 00c0 DSISR: 4000 IRQMASK: 0
GPR00: c0003d07ebfc c4f3f220 c0003d091100
c23ff400
GPR04: 0908 0002
GPR08: c67c7e80
0002
GPR12: 44222828 c7fffb80
GPR16: 022d9e80 ff9ad614 ff9ad618
022d9e80
GPR20: 000f4240
c67b81c0
GPR24: c1677640 c0003d08ab00
c67bb3a0
GPR28: c67b8000
c67bb3a0
[ 14.853813] NIP [c0003d07ec40] .gelic_descr_prepare_rx+0xa0/0x134
[ps3_gelic]
[ 14.853993] LR [c0003d07ebfc] .gelic_descr_prepare_rx+0x5c/0x134
[ps3_gelic]
[ 14.854157] Call Trace:
[ 14.854198] [c4f3f220] [c0003d07ebfc]
.gelic_descr_prepare_rx+0x5c/0x134 [ps3_gelic] (unreliable)
[ 14.858642] [c4f3f2b0] [c0003d080244]
.ps3_gelic_driver_probe+0x690/0x7ac [ps3_gelic]
[ 14.863113] [c4f3f3c0] [c004371c]
.ps3_system_bus_probe+0x5c/0x6c
[ 14.867749] [c4f3f440] [c03f7f28] .really_probe+0x18c/0x330
[ 14.872196] [c4f3f4d0] [c03f81f8]
.driver_probe_device+0x34/0xac
[ 14.876869] [c4f3f560] [c03f8464]
.__driver_attach+0x110/0x120
[ 14.881531] [c4f3f5f0] [c03f5b70] .bus_for_each_dev+0x88/0xc0
[ 14.886180] [c4f3f690] [c03f7620] .driver_attach+0x24/0x38
[ 14.886219] [c4f3f700] [c03f6d88] .bus_add_driver+0xe8/0x248
[ 14.886245] [c4f3f7b0] [c03f8dec] .driver_register+0xec/0x140
[ 14.899065] [c4f3f830] [c0043c00]
.ps3_system_bus_driver_register+0x20/0x34
[ 14.903777] [c4f3f8a0] [c0003d083370]
.ps3_gelic_driver_init+0x18/0x2c [ps3_gelic]
[ 14.907653] [c4f3f910] [c000da88] .do_one_initcall+0xac/0x228
[ 14.911635] [c4f3fa00] [c00d6f04] .do_init_module+0x80/0x268
[ 14.915525] [c4f3faa0] [c00d8e00]
.init_module_from_file+0xa4/0xa8
[ 14.918974] [c4f3fbc0] [c00d8f50]
.idempotent_init_module+0x14c/0x260
[ 14.922427] [c4f3fcf0] [c00d90bc]
.__se_sys_finit_module+0x54/0x78
[ 14.925815] [c4f3fd80] [c001c9ec]
.system_call_exception+0x180/0x1e4
[ 14.929342] [c4f3fe10] [c000b354]
system_call_common+0xf4/0x258
[ 14.933769] --- interrupt: c00 at 0x46bf0c
[ 14.938449] NIP: 0046bf0c LR: 0057b708 CTR:
[ 14.938465] REGS: c4f3fe80 TRAP: 0c00 Not tainted (6.8.0)
[ 14.938477] MSR: c032 CR: 24222488 XER:
[ 14.938527] IRQMASK: 0
GPR00: 0161 ff9ad440 f7e40520
0006
GPR04: 00587354 0006
GPR08:
GPR12: 00aef734
GPR16: 022d9e80 ff9ad614 ff9ad618
022d9e80
GPR20: 000f4240 022bc020
GPR24: 022d9e80 0007 022db0a0
00587354
GPR28: 0002 005a7cb4
022d9e80
[ 14.951597] NIP [0046bf0c] 0x46bf0c
[ 14.951612] LR [0057b708] 0x57b708
[ 14.951625] --- interrupt: c00
[ 14.951635] Code: 3860fff4 4864
Re: [RFC PATCH] mm: z3fold: rename CONFIG_Z3FOLD to CONFIG_Z3FOLD_DEPRECATED
On Mon, Jan 22, 2024 at 12:49 PM Yosry Ahmed wrote: > > On Sun, Jan 21, 2024 at 11:42 PM Christoph Hellwig wrote: > > > > On Tue, Jan 16, 2024 at 12:19:39PM -0800, Yosry Ahmed wrote: > > > Well, better compression ratios for one :) > > > > > > I think a long time ago there were complaints that zsmalloc had higher > > > latency than zbud/z3fold, but since then a lot of things have changed > > > (including nice compaction optimization from Sergey, and compaction > > > was one of the main factors AFAICT). Also, recent experiments that > > > Chris Li conducted showed that (at least in our setup), the > > > decompression is only a small part of the fault latency with zswap > > > (i.e. not the main factor) -- so I am not sure if it actually matters > > > in practice. > > > > > > That said, I have not conducted any experiments personally with z3fold > > > or zbud, which is why I proposed the conservative approach of marking > > > as deprecated first. However, if others believe this is unnecessary I > > > am fine with removal as well. Whatever we agree on is fine by me. > > > > In general deprecated is for code that has active (intentional) users > > and/or would break setups. I does sound to me like that is not the > > case here, but others might understand this better. > > I generally agree. So far we have no knowledge of active users, and if > there are some, I expect most of them to be able to switch to zsmalloc > with no problems. That being said, I was trying to take the > conservative approach. If others agree I can send a removal patch > instead. Andrew, do you have an opinion here? We are not sure that there are active users for z3fold. It seems like we have all sorts of opinions here from "don't deprecate" to "remove directly, no need to deprecate first".
Re: [PATCH v2] NUMA: Early use of cpu_to_node() returns 0 instead of the correct node id
On Wed, Jan 24, 2024 at 09:19:00AM -0800, Lameter, Christopher wrote: > On Tue, 23 Jan 2024, Huang Shijie wrote: > > > During the kernel booting, the generic cpu_to_node() is called too early in > > arm64, powerpc and riscv when CONFIG_NUMA is enabled. > > > > For arm64/powerpc/riscv, there are at least four places in the common code > > where the generic cpu_to_node() is called before it is initialized: > >1.) early_trace_init() in kernel/trace/trace.c > >2.) sched_init() in kernel/sched/core.c > >3.) init_sched_fair_class()in kernel/sched/fair.c > >4.) workqueue_init_early() in kernel/workqueue.c > > > > In order to fix the bug, the patch changes generic cpu_to_node to > > function pointer, and export it for kernel modules. > > Introduce smp_prepare_boot_cpu_start() to wrap the original > > smp_prepare_boot_cpu(), and set cpu_to_node with early_cpu_to_node. > > Introduce smp_prepare_cpus_done() to wrap the original smp_prepare_cpus(), > > and set the cpu_to_node to formal _cpu_to_node(). > > Would you please fix this cleanly without a function pointer? > > What I think needs to be done is a patch series. > > 1. Instrument cpu_to_node so that some warning is issued if it is used too > early. Preloading the array with NUMA_NO_NODE would allow us to do that. By preloading do you mean compile-time initialization? > 2. Implement early_cpu_to_node on platforms that currently do not have it. > > 3. A series of patches that fix each place where cpu_to_node is used too > early. Agree. This is the right way to go. And pretty well all of it was discussed in v1, isn't? Thanks, Yury
Re: [PATCH] KVM: PPC: code cleanup for kvmppc_book3s_irqprio_deliver
> If there are no plans to enable this part code in the future, Will the word combination “code part” become preferred for a subsequent change description? > we can remove this dead code. And omit another blank line accordingly? Regards, Markus
Re: [PATCH 3/4] net: wan: fsl_qmc_hdlc: Add runtime timeslots changes support
Hi Vadim,
On Wed, 24 Jan 2024 10:10:46 +
Vadim Fedorenko wrote:
[...]
> > +static int qmc_hdlc_xlate_slot_map(struct qmc_hdlc *qmc_hdlc,
> > + u32 slot_map, struct qmc_chan_ts_info
> > *ts_info)
> > +{
> > + u64 ts_mask_avail;
> > + unsigned int bit;
> > + unsigned int i;
> > + u64 ts_mask;
> > + u64 map;
> > +
> > + /* Tx and Rx masks must be identical */
> > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) {
> > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch
> > (0x%llx, 0x%llx)\n",
> > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail);
> > + return -EINVAL;
> > + }
> > +
> > + ts_mask_avail = ts_info->rx_ts_mask_avail;
> > + ts_mask = 0;
> > + map = slot_map;
> > + bit = 0;
> > + for (i = 0; i < 64; i++) {
> > + if (ts_mask_avail & BIT_ULL(i)) {
> > + if (map & BIT_ULL(bit))
> > + ts_mask |= BIT_ULL(i);
> > + bit++;
> > + }
> > + }
> > +
> > + if (hweight64(ts_mask) != hweight64(map)) {
> > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots 0x%llx ->
> > (0x%llx,0x%llx)\n",
> > + map, ts_mask_avail, ts_mask);
> > + return -EINVAL;
> > + }
> > +
> > + ts_info->tx_ts_mask = ts_mask;
> > + ts_info->rx_ts_mask = ts_mask;
> > + return 0;
> > +}
> > +
> > +static int qmc_hdlc_xlate_ts_info(struct qmc_hdlc *qmc_hdlc,
> > + const struct qmc_chan_ts_info *ts_info, u32
> > *slot_map)
> > +{
> > + u64 ts_mask_avail;
> > + unsigned int bit;
> > + unsigned int i;
> > + u64 ts_mask;
> > + u64 map;
> > +
>
> Starting from here ...
>
> > + /* Tx and Rx masks must be identical */
> > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) {
> > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch
> > (0x%llx, 0x%llx)\n",
> > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail);
> > + return -EINVAL;
> > + }
> > + if (ts_info->rx_ts_mask != ts_info->tx_ts_mask) {
> > + dev_err(qmc_hdlc->dev, "tx and rx timeslots mismatch (0x%llx,
> > 0x%llx)\n",
> > + ts_info->rx_ts_mask, ts_info->tx_ts_mask);
> > + return -EINVAL;
> > + }
> > +
> > + ts_mask_avail = ts_info->rx_ts_mask_avail;
> > + ts_mask = ts_info->rx_ts_mask;
> > + map = 0;
> > + bit = 0;
> > + for (i = 0; i < 64; i++) {
> > + if (ts_mask_avail & BIT_ULL(i)) {
> > + if (ts_mask & BIT_ULL(i))
> > + map |= BIT_ULL(bit);
> > + bit++;
> > + }
> > + }
> > +
> > + if (hweight64(ts_mask) != hweight64(map)) {
> > + dev_err(qmc_hdlc->dev, "Cannot translate timeslots
> > (0x%llx,0x%llx) -> 0x%llx\n",
> > + ts_mask_avail, ts_mask, map);
> > + return -EINVAL;
> > + }
> > +
>
> till here the block looks like copy of the block from previous function.
> It worth to make a separate function for it, I think.
>
> > + if (map >= BIT_ULL(32)) {
> > + dev_err(qmc_hdlc->dev, "Slot map out of 32bit (0x%llx,0x%llx)
> > -> 0x%llx\n",
> > + ts_mask_avail, ts_mask, map);
> > + return -EINVAL;
> > + }
> > +
> > + *slot_map = map;
> > + return 0;
> > +}
> > +
[...]
I am not so sure. There are slighty differences between the two functions.
The error messages and, in particular, the loop in qmc_hdlc_xlate_slot_map() is:
--- 8< ---
ts_mask_avail = ts_info->rx_ts_mask_avail;
ts_mask = 0;
map = slot_map;
bit = 0;
for (i = 0; i < 64; i++) {
if (ts_mask_avail & BIT_ULL(i)) {
if (map & BIT_ULL(bit))
ts_mask |= BIT_ULL(i);
bit++;
}
}
--- 8< ---
whereas it is the following in qmc_hdlc_xlate_ts_info():
--- 8< ---
ts_mask_avail = ts_info->rx_ts_mask_avail;
ts_mask = ts_info->rx_ts_mask;
map = 0;
bit = 0;
for (i = 0; i < 64; i++) {
if (ts_mask_avail & BIT_ULL(i)) {
if (ts_mask & BIT_ULL(i))
map |= BIT_ULL(bit);
bit++;
}
}
--- 8< ---
ts_map and map initializations are not the same, i and bit are not used for
the same purpose and the computed value is not computed based on the same
information.
With that pointed, I am not sure that having some common code for both
function will be relevant. Your opinion ?
Best regards,
Hervé
Re: [PATCH 1/4] net: wan: Add support for QMC HDLC
Hi Vadim,
On Wed, 24 Jan 2024 10:03:45 +
Vadim Fedorenko wrote:
[...]
> > +static void qmc_hcld_recv_complete(void *context, size_t length, unsigned
> > int flags)
> > +{
> > + struct qmc_hdlc_desc *desc = context;
> > + struct net_device *netdev = desc->netdev;
> > + struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(desc->netdev);
>
> a line above desc->netdev was stored in netdev. better to reuse it and
> make declaration part consistent with qmc_hcld_xmit_complete
Yes.
Will updated in the next iteration.
[...]
> > +static netdev_tx_t qmc_hdlc_xmit(struct sk_buff *skb, struct net_device
> > *netdev)
> > +{
> > + struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(netdev);
> > + struct qmc_hdlc_desc *desc;
> > + unsigned long flags;
> > + int ret;
> > +
> > + spin_lock_irqsave(_hdlc->tx_lock, flags);
> > + desc = _hdlc->tx_descs[qmc_hdlc->tx_out];
> > + if (desc->skb) {
> > + /* Should never happen.
> > +* Previous xmit should have already stopped the queue.
> > +*/
>
> according to the comment it's better to make if(unlikely(desc->skb)) or
> even WARN_ONCE()
>
Indeed. I will use WARN_ONCE() in the next iteration.
Thanks for your review,
Hervé
Re: [PATCH linux-next v3 10/14] sh, crash: wrap crash dumping code into crash related ifdefs
On 01/24/24 at 09:13am, John Paul Adrian Glaubitz wrote: > Hello Baoquan, > > On Wed, 2024-01-24 at 13:12 +0800, Baoquan He wrote: > > Now crash codes under kernel/ folder has been split out from kexec > > code, crash dumping can be separated from kexec reboot in config > > items on SuperH with some adjustments. > > > > wrap up crash dumping codes with CONFIG_CRASH_DUMP ifdeffery, and > > use IS_ENABLED(CONFIG_CRASH_RESERVE) check to decide if compiling > > in the crashkernel reservation code. > > Comparing this to the patches, it seems you missed the first word > "Here". Either amend that or write the word "wrap" capitalized. > > I would omit "Here" as it's not necessary and just start the > sentence with "Wrap". You are right, thanks for careful checking. I will see if I need repost to include this update. > > -- > .''`. John Paul Adrian Glaubitz > : :' : Debian Developer > `. `' Physicist > `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 > > ___ > kexec mailing list > ke...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec >
Re: [PATCH] KVM: PPC: code cleanup for kvmppc_book3s_irqprio_deliver
Le 24/01/2024 à 10:36, Kunwu Chan a écrit :
> This part was commented from commit 2f4cf5e42d13 ("Add book3s.c")
> in about 14 years before.
> If there are no plans to enable this part code in the future,
> we can remove this dead code.
>
> Signed-off-by: Kunwu Chan
> ---
> arch/powerpc/kvm/book3s.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 8acec144120e..c2f50e04eec8 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -360,9 +360,6 @@ static int kvmppc_book3s_irqprio_deliver(struct kvm_vcpu
> *vcpu,
> break;
> }
>
> -#if 0
> - printk(KERN_INFO "Deliver interrupt 0x%x? %x\n", vec, deliver);
> -#endif
Please also remove one of the two blank lines.
>
> if (deliver)
> kvmppc_inject_interrupt(vcpu, vec, 0);
[PATCH] powerpc: rename SPRN_HID2 define to SPRN_HID2_750FX
This register number is hardware-specific, rename it for clarity. FIXME comments are added in a few places where it seems like the wrong register is used. As I can't test this, only the rename is done with no functional change. Signed-off-by: Matthias Schiffer --- arch/powerpc/include/asm/reg.h | 2 +- arch/powerpc/kernel/cpu_setup_6xx.S | 4 ++-- arch/powerpc/kvm/book3s_emulate.c| 4 ++-- arch/powerpc/platforms/52xx/lite5200_sleep.S | 6 -- arch/powerpc/platforms/83xx/suspend-asm.S| 6 -- drivers/cpufreq/pmac32-cpufreq.c | 8 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 4ae4ab9090a2..994dfefba98b 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -615,7 +615,7 @@ #define HID1_ABE (1<<10) /* 7450 Address Broadcast Enable */ #define HID1_PS(1<<16) /* 750FX PLL selection */ #endif -#define SPRN_HID2 0x3F8 /* Hardware Implementation Register 2 */ +#define SPRN_HID2_750FX0x3F8 /* IBM 750FX HID2 Register */ #define SPRN_HID2_GEKKO0x398 /* Gekko HID2 Register */ #define SPRN_IABR 0x3F2 /* Instruction Address Breakpoint Register */ #define SPRN_IABR2 0x3FA /* 83xx */ diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S b/arch/powerpc/kernel/cpu_setup_6xx.S index f29ce3dd6140..4f4a4ce34861 100644 --- a/arch/powerpc/kernel/cpu_setup_6xx.S +++ b/arch/powerpc/kernel/cpu_setup_6xx.S @@ -382,7 +382,7 @@ _GLOBAL(__save_cpu_setup) andi. r3,r3,0xff00 cmpwi cr0,r3,0x0200 bne 1f - mfspr r4,SPRN_HID2 + mfspr r4,SPRN_HID2_750FX stw r4,CS_HID2(r5) 1: mtcrr7 @@ -477,7 +477,7 @@ _GLOBAL(__restore_cpu_setup) bne 4f lwz r4,CS_HID2(r5) rlwinm r4,r4,0,19,17 - mtspr SPRN_HID2,r4 + mtspr SPRN_HID2_750FX,r4 sync 4: lwz r4,CS_HID1(r5) diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c index 5bbfb2eed127..de126d153328 100644 --- a/arch/powerpc/kvm/book3s_emulate.c +++ b/arch/powerpc/kvm/book3s_emulate.c @@ -714,7 +714,7 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) case SPRN_HID1: to_book3s(vcpu)->hid[1] = spr_val; break; - case SPRN_HID2: + case SPRN_HID2_750FX: to_book3s(vcpu)->hid[2] = spr_val; break; case SPRN_HID2_GEKKO: @@ -900,7 +900,7 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val case SPRN_HID1: *spr_val = to_book3s(vcpu)->hid[1]; break; - case SPRN_HID2: + case SPRN_HID2_750FX: case SPRN_HID2_GEKKO: *spr_val = to_book3s(vcpu)->hid[2]; break; diff --git a/arch/powerpc/platforms/52xx/lite5200_sleep.S b/arch/powerpc/platforms/52xx/lite5200_sleep.S index 0b12647e7b42..0ec2522ee4ad 100644 --- a/arch/powerpc/platforms/52xx/lite5200_sleep.S +++ b/arch/powerpc/platforms/52xx/lite5200_sleep.S @@ -203,7 +203,8 @@ lite5200_wakeup: /* HIDs, MSR */ LOAD_SPRN(HID1, 0x19) - LOAD_SPRN(HID2, 0x1a) + /* FIXME: Should this use HID2_G2_LE? */ + LOAD_SPRN(HID2_750FX, 0x1a) /* address translation is tricky (see turn_on_mmu) */ @@ -283,7 +284,8 @@ SYM_FUNC_START_LOCAL(save_regs) SAVE_SPRN(HID0, 0x18) SAVE_SPRN(HID1, 0x19) - SAVE_SPRN(HID2, 0x1a) + /* FIXME: Should this use HID2_G2_LE? */ + SAVE_SPRN(HID2_750FX, 0x1a) mfmsr r10 stw r10, (4*0x1b)(r4) /*SAVE_SPRN(LR, 0x1c) have to save it before the call */ diff --git a/arch/powerpc/platforms/83xx/suspend-asm.S b/arch/powerpc/platforms/83xx/suspend-asm.S index bc6bd4d0ae96..6a62ed6082c9 100644 --- a/arch/powerpc/platforms/83xx/suspend-asm.S +++ b/arch/powerpc/platforms/83xx/suspend-asm.S @@ -68,7 +68,8 @@ _GLOBAL(mpc83xx_enter_deep_sleep) mfspr r5, SPRN_HID0 mfspr r6, SPRN_HID1 - mfspr r7, SPRN_HID2 + /* FIXME: Should this use SPRN_HID2_G2_LE? */ + mfspr r7, SPRN_HID2_750FX stw r5, SS_HID+0(r3) stw r6, SS_HID+4(r3) @@ -396,7 +397,8 @@ mpc83xx_deep_resume: mtspr SPRN_HID0, r5 mtspr SPRN_HID1, r6 - mtspr SPRN_HID2, r7 + /* FIXME: Should this use SPRN_HID2_G2_LE? */ + mtspr SPRN_HID2_750FX, r7 lwz r4, SS_IABR+0(r3) lwz r5, SS_IABR+4(r3) diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c index df3567c1e93b..6c9f0888a2a7 100644 --- a/drivers/cpufreq/pmac32-cpufreq.c +++ b/drivers/cpufreq/pmac32-cpufreq.c @@ -120,9 +120,9 @@ static int cpu_750fx_cpu_speed(int low_speed) /* tweak
[PATCH v2] powerpc/6xx: set High BAT Enable flag on G2_LE cores
MMU_FTR_USE_HIGH_BATS is set for G2_LE cores and derivatives like e300cX,
but the high BATs need to be enabled in HID2 to work. Add register
definitions and add the needed setup to __setup_cpu_603.
This fixes boot on CPUs like the MPC5200B with STRICT_KERNEL_RWX enabled
on systems where the flag has not been set by the bootloader already.
Fixes: e4d6654ebe6e ("powerpc/mm/32s: rework mmu_mapin_ram()")
Signed-off-by: Matthias Schiffer
---
v2:
- Use the G2_LE name for cores that have this HID2 register
- Extend __setup_cpu_603 instead of introducing a new setup function
arch/powerpc/include/asm/reg.h | 2 ++
arch/powerpc/kernel/cpu_setup_6xx.S | 20 +++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 4ae4ab9090a2..ade5f094dbd2 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -617,6 +617,8 @@
#endif
#define SPRN_HID2 0x3F8 /* Hardware Implementation Register 2 */
#define SPRN_HID2_GEKKO0x398 /* Gekko HID2 Register */
+#define SPRN_HID2_G2_LE0x3F3 /* G2_LE HID2 Register */
+#define HID2_G2_LE_HBE(1<<18) /* High BAT Enable (G2_LE) */
#define SPRN_IABR 0x3F2 /* Instruction Address Breakpoint Register */
#define SPRN_IABR2 0x3FA /* 83xx */
#define SPRN_IBCR 0x135 /* 83xx Insn Breakpoint Control Reg */
diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S
b/arch/powerpc/kernel/cpu_setup_6xx.S
index f29ce3dd6140..bfd3f442e5eb 100644
--- a/arch/powerpc/kernel/cpu_setup_6xx.S
+++ b/arch/powerpc/kernel/cpu_setup_6xx.S
@@ -26,6 +26,15 @@ BEGIN_FTR_SECTION
bl __init_fpu_registers
END_FTR_SECTION_IFCLR(CPU_FTR_FPU_UNAVAILABLE)
bl setup_common_caches
+
+ /*
+* This assumes that all cores using __setup_cpu_603 with
+* MMU_FTR_USE_HIGH_BATS are G2_LE compatible
+*/
+BEGIN_MMU_FTR_SECTION
+ bl setup_g2_le_hid2
+END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
+
mtlrr5
blr
_GLOBAL(__setup_cpu_604)
@@ -115,6 +124,16 @@ SYM_FUNC_START_LOCAL(setup_604_hid0)
blr
SYM_FUNC_END(setup_604_hid0)
+/* Enable high BATs for G2_LE and derivatives like e300cX */
+SYM_FUNC_START_LOCAL(setup_g2_le_hid2)
+ mfspr r11,SPRN_HID2_G2_LE
+ orisr11,r11,HID2_G2_LE_HBE@h
+ mtspr SPRN_HID2_G2_LE,r11
+ sync
+ isync
+ blr
+SYM_FUNC_END(setup_g2_le_hid2)
+
/* 7400 <= rev 2.7 and 7410 rev = 1.0 suffer from some
* erratas we work around here.
* Moto MPC710CE.pdf describes them, those are errata
@@ -495,4 +514,3 @@ _GLOBAL(__restore_cpu_setup)
mtcrr7
blr
_ASM_NOKPROBE_SYMBOL(__restore_cpu_setup)
-
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/
[PATCH] KVM: PPC: code cleanup for kvmppc_book3s_irqprio_deliver
This part was commented from commit 2f4cf5e42d13 ("Add book3s.c")
in about 14 years before.
If there are no plans to enable this part code in the future,
we can remove this dead code.
Signed-off-by: Kunwu Chan
---
arch/powerpc/kvm/book3s.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 8acec144120e..c2f50e04eec8 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -360,9 +360,6 @@ static int kvmppc_book3s_irqprio_deliver(struct kvm_vcpu
*vcpu,
break;
}
-#if 0
- printk(KERN_INFO "Deliver interrupt 0x%x? %x\n", vec, deliver);
-#endif
if (deliver)
kvmppc_inject_interrupt(vcpu, vec, 0);
--
2.39.2
Re: [PATCH linux-next v3 10/14] sh, crash: wrap crash dumping code into crash related ifdefs
Hello Baoquan, On Wed, 2024-01-24 at 13:12 +0800, Baoquan He wrote: > Now crash codes under kernel/ folder has been split out from kexec > code, crash dumping can be separated from kexec reboot in config > items on SuperH with some adjustments. > > wrap up crash dumping codes with CONFIG_CRASH_DUMP ifdeffery, and > use IS_ENABLED(CONFIG_CRASH_RESERVE) check to decide if compiling > in the crashkernel reservation code. Comparing this to the patches, it seems you missed the first word "Here". Either amend that or write the word "wrap" capitalized. I would omit "Here" as it's not necessary and just start the sentence with "Wrap". Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
