Re: [PATCH 6/6] smp: Cleanup smp_call_function*()

From: Christoph Hellwig
Date: Thu Jun 18 2020 - 02:51:14 EST


On Wed, Jun 17, 2020 at 01:04:01PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 17, 2020 at 01:23:49AM -0700, Christoph Hellwig wrote:
>
> > > @@ -178,9 +178,7 @@ static void zpci_handle_fallback_irq(voi
> > > if (atomic_inc_return(&cpu_data->scheduled) > 1)
> > > continue;
> > >
> > > - cpu_data->csd.func = zpci_handle_remote_irq;
> > > - cpu_data->csd.info = &cpu_data->scheduled;
> > > - cpu_data->csd.flags = 0;
> > > + cpu_data->csd = CSD_INIT(zpci_handle_remote_irq, &cpu_data->scheduled);
> >
> > This looks weird. I'd much rather see an initialization ala INIT_WORK:
> >
> > INIT_CSD(&cpu_data->csd, zpci_handle_remote_irq,
> > &cpu_data->scheduled);
>
>
> like so then?

Much better. Although if we touch all the callers we might as well
pass the csd as the argument to the callback, as with that we can
pretty trivially remove the private data field later.

Btw, it seems the callers that don't have the CSD embedded into the
containing structure seems to be of these two kinds:

- reimplementing on_each_cpumask (mostly because they can be called
from irq context)
- reimplenenting smp_call_function_single because they want
to sleep instead of busy wait

I wonder if those would be useful primitives for smp.c..