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@intel.com>
---
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; \
+})
void free_kthread_struct(struct task_struct *k);
void kthread_bind(struct task_struct *k, unsigned int cpu);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index f7be976ff88a..e9da0115fb2b 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -559,23 +559,27 @@ void kthread_bind(struct task_struct *p, unsigned int cpu)
EXPORT_SYMBOL(kthread_bind);
/**
- * kthread_create_on_cpu - Create a cpu bound kthread
+ * __kthread_create_on_cpu - Create a cpu bound kthread
* @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: This helper function creates and names a kernel thread
*/
-struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
- void *data, unsigned int cpu,
- const char *namefmt)
+struct task_struct *__kthread_create_on_cpu(int (*threadfn)(void *data),
+ void *data, unsigned int cpu,
+ const char *namefmt, ...)
{
struct task_struct *p;
+ va_list args;
+
+ va_start(args, namefmt);
+ p = __kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
+ args);
+ va_end(args);
- p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
- cpu);
if (IS_ERR(p))
return p;
kthread_bind(p, cpu);
@@ -583,7 +587,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),
to_kthread(p)->cpu = cpu;
return p;
}
-EXPORT_SYMBOL(kthread_create_on_cpu);
+EXPORT_SYMBOL(__kthread_create_on_cpu);
void kthread_set_per_cpu(struct task_struct *k, int cpu)
{
--
2.46.0On 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@intel.com>
> ---
> 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.
From: Stanislav Fomichev <sdf@fomichev.me>
Date: Fri, 30 Aug 2024 15:56:12 -0700
> 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@intel.com>
>> ---
>> 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
It will still be a -Wshadow warning if the caller has a variable with
the same name. I know it's enabled only on W=2, but anyway I feel like
we shouldn't introduce any new warnings when possible.
> 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.
Thanks,
Olek
On 09/03, Alexander Lobakin wrote:
> From: Stanislav Fomichev <sdf@fomichev.me>
> Date: Fri, 30 Aug 2024 15:56:12 -0700
>
> > 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@intel.com>
> >> ---
> >> 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
>
> It will still be a -Wshadow warning if the caller has a variable with
> the same name. I know it's enabled only on W=2, but anyway I feel like
> we shouldn't introduce any new warnings when possible.
Makes sense, thanks! That's why, presumably, kthread_run uses __k name
to avoid the warning..
© 2016 - 2025 Red Hat, Inc.