From: Stanislav Fomichev <sdf@fomichev.me>
Date: Fri, 30 Aug 2024 15:56:12 -0700

On 08/30, Alexander Lobakin wrote:

quoted

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