Hi Eric,
On Fri, Mar 29, 2024 at 02:21:28PM +0100, Eric Dumazet wrote:
> On Fri, Mar 29, 2024 at 11:57 AM Dawei Li <dawei.li@xxxxxxxxxxxx> wrote:
> >
> > For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
> > variable on stack is not recommended since it can cause potential stack
> > overflow.
> >
> > Instead, kernel code should always use *cpumask_var API(s) to allocate
> > cpumask var in config-neutral way, leaving allocation strategy to
> > CONFIG_CPUMASK_OFFSTACK.
> >
> > Use *cpumask_var API(s) to address it.
> >
> > Signed-off-by: Dawei Li <dawei.li@xxxxxxxxxxxx>
> > ---
> > net/iucv/iucv.c | 37 ++++++++++++++++++++++++++-----------
> > 1 file changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c
> > index a4ab615ca3e3..b51f46ec32f9 100644
> > --- a/net/iucv/iucv.c
> > +++ b/net/iucv/iucv.c
> > @@ -520,14 +520,19 @@ static void iucv_setmask_mp(void)
> > */
> > static void iucv_setmask_up(void)
> > {
> > - cpumask_t cpumask;
> > + cpumask_var_t cpumask;
> > int cpu;
> >
> > + if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
> > + return;
>
> This can not be right. iucv_setmask_up() is not supposed to fail.
>
> Since iucv_setmask_up() is only called with iucv_register_mutex held,
> you could simply add a 'static' for @cpumask variable.
Correct, iucv_register_mutex is a global lock and can serialize access
on static cpumask var.
I will respin V2 as you suggested.
Thanks,
Dawei
>
>
>
> > +
> > /* Disable all cpu but the first in cpu_irq_cpumask. */
> > - cpumask_copy(&cpumask, &iucv_irq_cpumask);
> > - cpumask_clear_cpu(cpumask_first(&iucv_irq_cpumask), &cpumask);
> > - for_each_cpu(cpu, &cpumask)
> > + cpumask_copy(cpumask, &iucv_irq_cpumask);
> > + cpumask_clear_cpu(cpumask_first(&iucv_irq_cpumask), cpumask);
> > + for_each_cpu(cpu, cpumask)
> > smp_call_function_single(cpu, iucv_block_cpu, NULL, 1);
> > +
> > + free_cpumask_var(cpumask);
> > }
>
