> -static DEFINE_PER_CPU(call_single_data_t, backtrace_csd);
> +static DEFINE_PER_CPU(call_single_data_t, backtrace_csd) =
> CSD_INIT(handle_backtrace, NULL);
> static struct cpumask backtrace_csd_busy;
Besides the crazy long line: does assigning to a DEFINE_PER_CPU
really work and initialize all the members?
> @@ -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);
Also for many smp_call_function_* users it would be trivial and actually
lead to nicer code if the data argument went away and we'd just use
container_of to get to the containing structure. For the remaining
ones we can trivially general a container strucuture that has the
extra data pointer.
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -629,9 +629,7 @@ void blk_mq_force_complete_rq(struct req
> shared = cpus_share_cache(cpu, ctx->cpu);
>
> if (cpu != ctx->cpu && !shared && cpu_online(ctx->cpu)) {
> - rq->csd.func = __blk_mq_complete_request_remote;
> - rq->csd.info = rq;
> - rq->csd.flags = 0;
> + rq->csd = CSD_INIT(__blk_mq_complete_request_remote, rq);
> smp_call_function_single_async(ctx->cpu, &rq->csd);
> } else {
> q->mq_ops->complete(rq);
> --- a/block/blk-softirq.c
> +++ b/block/blk-softirq.c
> @@ -57,13 +57,8 @@ static void trigger_softirq(void *data)
> static int raise_blk_irq(int cpu, struct request *rq)
> {
> if (cpu_online(cpu)) {
> - call_single_data_t *data = &rq->csd;
> -
> - data->func = trigger_softirq;
> - data->info = rq;
> - data->flags = 0;
> -
> - smp_call_function_single_async(cpu, data);
> + rq->csd = CSD_INIT(trigger_softirq, rq);
> + smp_call_function_single_async(cpu, &rq->csd);
> return 0;
> }
FYI, I rewrote much of the blk code in this series:
https://lore.kernel.org/linux-block/[email protected]/T/#t
that you also were Cced on.
> struct __call_single_data {
> - union {
> - struct __call_single_node node;
> - struct {
> - struct llist_node llist;
> - unsigned int flags;
> - };
> - };
> + struct __call_single_node node;
> smp_call_func_t func;
> void *info;
> };
Can we rename this to struct call_single_data without the __prefix
and switch all the users you touch anyway away from the typedef?
