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..