On 08/30, Alexander Lobakin wrote:
> Currently, kthread_{create,run}_on_cpu() doesn't support varargs like
> kthread_create{,_on_node}() do, which makes them less convenient to
> use.
> Convert them to take varargs as the last argument. The only difference
> is that they always append the CPU ID at the end and require the format
> string to have an excess '%u' at the end due to that. That's still true;
> meanwhile, the compiler will correctly point out to that if missing.
> One more nice side effect is that you can now use the underscored
> __kthread_create_on_cpu() if you want to override that rule and not
> have CPU ID at the end of the name.
> The current callers are not anyhow affected.
>
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
> ---
> include/linux/kthread.h | 51 ++++++++++++++++++++++++++---------------
> kernel/kthread.c | 22 ++++++++++--------
> 2 files changed, 45 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> index b11f53c1ba2e..27a94e691948 100644
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
> @@ -27,11 +27,21 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
> #define kthread_create(threadfn, data, namefmt, arg...) \
> kthread_create_on_node(threadfn, data, NUMA_NO_NODE, namefmt, ##arg)
>
> -
> -struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
> - void *data,
> - unsigned int cpu,
> - const char *namefmt);
> +__printf(4, 5)
> +struct task_struct *__kthread_create_on_cpu(int (*threadfn)(void *data),
> + void *data, unsigned int cpu,
> + const char *namefmt, ...);
> +
> +#define kthread_create_on_cpu(threadfn, data, cpu, namefmt, ...) \
> + _kthread_create_on_cpu(threadfn, data, cpu, __UNIQUE_ID(cpu_), \
> + namefmt, ##__VA_ARGS__)
> +
> +#define _kthread_create_on_cpu(threadfn, data, cpu, uc, namefmt, ...) ({ \
> + u32 uc = (cpu); \
> + \
> + __kthread_create_on_cpu(threadfn, data, uc, namefmt, \
> + ##__VA_ARGS__, uc); \
> +})
>
> void get_kthread_comm(char *buf, size_t buf_size, struct task_struct *tsk);
> bool set_kthread_struct(struct task_struct *p);
> @@ -62,25 +72,28 @@ bool kthread_is_per_cpu(struct task_struct *k);
> * @threadfn: the function to run until signal_pending(current).
> * @data: data ptr for @threadfn.
> * @cpu: The cpu on which the thread should be bound,
> - * @namefmt: printf-style name for the thread. Format is restricted
> - * to "name.*%u". Code fills in cpu number.
> + * @namefmt: printf-style name for the thread. Must have an excess '%u'
> + * at the end as kthread_create_on_cpu() fills in CPU number.
> *
> * Description: Convenient wrapper for kthread_create_on_cpu()
> * followed by wake_up_process(). Returns the kthread or
> * ERR_PTR(-ENOMEM).
> */
> -static inline struct task_struct *
> -kthread_run_on_cpu(int (*threadfn)(void *data), void *data,
> - unsigned int cpu, const char *namefmt)
> -{
> - struct task_struct *p;
> -
> - p = kthread_create_on_cpu(threadfn, data, cpu, namefmt);
> - if (!IS_ERR(p))
> - wake_up_process(p);
> -
> - return p;
> -}
> +#define kthread_run_on_cpu(threadfn, data, cpu, namefmt, ...) \
> + _kthread_run_on_cpu(threadfn, data, cpu, __UNIQUE_ID(task_), \
> + namefmt, ##__VA_ARGS__)
> +
> +#define _kthread_run_on_cpu(threadfn, data, cpu, ut, namefmt, ...) \
> +({ \
> + struct task_struct *ut; \
> + \
> + ut = kthread_create_on_cpu(threadfn, data, cpu, namefmt, \
> + ##__VA_ARGS__); \
> + if (!IS_ERR(ut)) \
> + wake_up_process(ut); \
> + \
> + ut; \
> +})
Why do you need to use __UNIQUE_ID here? Presumably ({}) in _kthread_run_on_cpu
should be enough to avoid the issue of non unique variable in the parent
scope. (and similar kthread_run isn't using any __UNIQUE_IDs)
The rest of the patches look good.
