Hi,
This is v3 of previous series on removal of on-stack cpumask var for
sparc arch.
Change since v2:
- PATCH[1]:
- Rename helper, cpumask_any_but_current -> any_other_mm_cpus. (Sam)
- Add Reviewed-by from Sam.
- PATCH[2-4]:
- Add Reviewed-by from Sam.
- PATCH[5]:
- Remove __initdata annotation and related commit message. (Sam)
- PATCH[6]:
- Change from ":?" to "if else" style. (Sam)
- Core logic _unchanged_.
- PATCH[7]:
- Removed due to potentially conflicted with other pending series. (Sam)
v1:
https://lore.kernel.org/all/[email protected]/
v2:
https://lore.kernel.org/lkml/[email protected]/
Dawei Li (6):
sparc/srmmu: Remove on-stack cpumask var
sparc/irq: Remove on-stack cpumask var
sparc/of: Remove on-stack cpumask var
sparc/pci_msi: Remove on-stack cpumask var
sparc/init: Remove on-stack cpumask var
sparc/leon: Remove on-stack cpumask var
arch/sparc/kernel/irq_64.c | 10 +++-----
arch/sparc/kernel/leon_kernel.c | 7 +++---
arch/sparc/kernel/of_device_64.c | 5 +---
arch/sparc/kernel/pci_msi.c | 5 +---
arch/sparc/mm/init_64.c | 2 +-
arch/sparc/mm/srmmu.c | 40 ++++++++++----------------------
6 files changed, 21 insertions(+), 48 deletions(-)
Thanks,
Dawei
--
2.27.0
In general it's preferable to avoid placing cpumasks on the stack, as
for large values of NR_CPUS these can consume significant amounts of
stack space and make stack overflows more likely.
@cpumask of irq_set_affinity() is read-only and free of change, drop
unneeded cpumask var.
Reviewed-by: Sam Ravnborg <[email protected]>
Signed-off-by: Dawei Li <[email protected]>
---
arch/sparc/kernel/of_device_64.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/sparc/kernel/of_device_64.c b/arch/sparc/kernel/of_device_64.c
index c350c58c7f69..f98c2901f335 100644
--- a/arch/sparc/kernel/of_device_64.c
+++ b/arch/sparc/kernel/of_device_64.c
@@ -624,10 +624,7 @@ static unsigned int __init build_one_device_irq(struct platform_device *op,
out:
nid = of_node_to_nid(dp);
if (nid != -1) {
- cpumask_t numa_mask;
-
- cpumask_copy(&numa_mask, cpumask_of_node(nid));
- irq_set_affinity(irq, &numa_mask);
+ irq_set_affinity(irq, cpumask_of_node(nid));
}
return irq;
--
2.27.0
In general it's preferable to avoid placing cpumasks on the stack, as
for large values of NR_CPUS these can consume significant amounts of
stack space and make stack overflows more likely.
@cpumask of irq_set_affinity() is read-only and free of change, drop
unneeded cpumask var.
Reviewed-by: Sam Ravnborg <[email protected]>
Signed-off-by: Dawei Li <[email protected]>
---
arch/sparc/kernel/pci_msi.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/sparc/kernel/pci_msi.c b/arch/sparc/kernel/pci_msi.c
index fc7402948b7b..acb2f83a1d5c 100644
--- a/arch/sparc/kernel/pci_msi.c
+++ b/arch/sparc/kernel/pci_msi.c
@@ -287,10 +287,7 @@ static int bringup_one_msi_queue(struct pci_pbm_info *pbm,
nid = pbm->numa_node;
if (nid != -1) {
- cpumask_t numa_mask;
-
- cpumask_copy(&numa_mask, cpumask_of_node(nid));
- irq_set_affinity(irq, &numa_mask);
+ irq_set_affinity(irq, cpumask_of_node(nid));
}
err = request_irq(irq, sparc64_msiq_interrupt, 0,
"MSIQ",
--
2.27.0
In general it's preferable to avoid placing cpumasks on the stack, as
for large values of NR_CPUS these can consume significant amounts of
stack space and make stack overflows more likely.
Use cpumask_subset() and cpumask_first_and() to avoid the need for a
temporary cpumask on the stack.
Signed-off-by: Dawei Li <[email protected]>
---
arch/sparc/kernel/leon_kernel.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/sparc/kernel/leon_kernel.c b/arch/sparc/kernel/leon_kernel.c
index 4c61da491fee..a43cf794bb1e 100644
--- a/arch/sparc/kernel/leon_kernel.c
+++ b/arch/sparc/kernel/leon_kernel.c
@@ -106,13 +106,12 @@ unsigned long leon_get_irqmask(unsigned int irq)
#ifdef CONFIG_SMP
static int irq_choose_cpu(const struct cpumask *affinity)
{
- cpumask_t mask;
+ unsigned int cpu = cpumask_first_and(affinity, cpu_online_mask);
- cpumask_and(&mask, cpu_online_mask, affinity);
- if (cpumask_equal(&mask, cpu_online_mask) || cpumask_empty(&mask))
+ if (cpumask_subset(cpu_online_mask, affinity) || cpu >= nr_cpu_ids)
return boot_cpu_id;
else
- return cpumask_first(&mask);
+ return cpu;
}
#else
#define irq_choose_cpu(affinity) boot_cpu_id
--
2.27.0
In general it's preferable to avoid placing cpumasks on the stack, as
for large values of NR_CPUS these can consume significant amounts of
stack space and make stack overflows more likely.
Since the cpumask var resides in __init function, which means it's free
of any concurrenct access, it can be safely marked with static to get
rid of allocation on stack.
Signed-off-by: Dawei Li <[email protected]>
---
arch/sparc/mm/init_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 1ca9054d9b97..9edbf57a2c59 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -1438,7 +1438,7 @@ static int __init numa_attach_mlgroup(struct mdesc_handle *md, u64 grp,
static int __init numa_parse_mdesc_group(struct mdesc_handle *md, u64 grp,
int index)
{
- cpumask_t mask;
+ static cpumask_t mask;
int cpu;
numa_parse_mdesc_group_cpus(md, grp, &mask);
--
2.27.0
Hi Dawei.
On Tue, Apr 23, 2024 at 04:30:42PM +0800, Dawei Li wrote:
> In general it's preferable to avoid placing cpumasks on the stack, as
> for large values of NR_CPUS these can consume significant amounts of
> stack space and make stack overflows more likely.
>
> Since the cpumask var resides in __init function, which means it's free
> of any concurrenct access, it can be safely marked with static to get
> rid of allocation on stack.
>
> Signed-off-by: Dawei Li <[email protected]>
I am not convinced this patch is the right approach, and I am not sure
it is worth trying to fix it.
This patch adds complexity, where the other patches simplified code.
I recommend to drop this, we can re-visit if this turns out to be a real
problem.
Sam
On Tue, Apr 23, 2024 at 04:30:43PM +0800, Dawei Li wrote:
> In general it's preferable to avoid placing cpumasks on the stack, as
> for large values of NR_CPUS these can consume significant amounts of
> stack space and make stack overflows more likely.
>
> Use cpumask_subset() and cpumask_first_and() to avoid the need for a
> temporary cpumask on the stack.
>
> Signed-off-by: Dawei Li <[email protected]>
Looks fine.
Reviewed-by: Sam Ravnborg <[email protected]>