Re: [PATCH 2/3] smp: Add a smp_cond_func_t argument to smp_call_function_many()

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Jan 17, 2020 at 10:01:36AM +0100, Sebastian Andrzej Siewior wrote:

> @@ -448,7 +435,8 @@ void smp_call_function_many(const struct cpumask *mask,
>  
>  	/* Fastpath: do that cpu by itself. */
>  	if (next_cpu >= nr_cpu_ids) {
> +		if (!cond_func || (cond_func && cond_func(cpu, info)))
> +			smp_call_function_single(cpu, func, info, wait);

Can't we write that like:

		if (!cond_func || cond_func(cpu, info))

>  		return;
>  	}
>  
> @@ -465,6 +453,9 @@ void smp_call_function_many(const struct cpumask *mask,
>  	for_each_cpu(cpu, cfd->cpumask) {
>  		call_single_data_t *csd = per_cpu_ptr(cfd->csd, cpu);
>  
> +		if (cond_func && !cond_func(cpu, info))
> +			continue;
> +
>  		csd_lock(csd);
>  		if (wait)
>  			csd->flags |= CSD_FLAG_SYNCHRONOUS;
> @@ -486,6 +477,26 @@ void smp_call_function_many(const struct cpumask *mask,
>  		}
>  	}
>  }
> +
> +/**
> + * smp_call_function_many(): Run a function on a set of other CPUs.
> + * @mask: The set of cpus to run on (only runs on online subset).
> + * @func: The function to run. This must be fast and non-blocking.
> + * @info: An arbitrary pointer to pass to the function.
> + * @wait: If true, wait (atomically) until function has completed
> + *        on other CPUs.
> + *
> + * If @wait is true, then returns once @func has returned.
> + *
> + * You must not call this function with disabled interrupts or from a
> + * hardware interrupt handler or from a bottom half handler. Preemption
> + * must be disabled when calling this function.
> + */
> +void smp_call_function_many(const struct cpumask *mask,
> +			    smp_call_func_t func, void *info, bool wait)
> +{
> +	smp_call_function_many_cond(mask, func, info, wait, NULL);
> +}
>  EXPORT_SYMBOL(smp_call_function_many);
>  
>  /**
> @@ -684,33 +695,17 @@ void on_each_cpu_cond_mask(smp_cond_func_t cond_func, smp_call_func_t func,
>  			   void *info, bool wait, gfp_t gfp_flags,
>  			   const struct cpumask *mask)
>  {
> +	int cpu = get_cpu();
>  
> +	smp_call_function_many_cond(mask, func, info, wait, cond_func);
> +	if (cpumask_test_cpu(cpu, mask) && cond_func(cpu, info)) {
> +		unsigned long flags;
>  
> +		local_irq_save(flags);
> +		func(info);
> +		local_irq_restore(flags);
>  	}
> +	put_cpu();
>  }
>  EXPORT_SYMBOL(on_each_cpu_cond_mask);

But yes, over-all this seems like a very nice cleanup.