Re: [PATCH 01/53] workqueue: Introduce the create*_workqueue2() macros

[Kees Cook]

On Mon, Jul 01, 2024 at 09:42:50AM -0700, Bart Van Assche wrote:
> On 6/30/24 7:51 PM, Lai Jiangshan wrote:
> > On Mon, Jul 1, 2024 at 6:29 AM Bart Van Assche <bvanassche@xxxxxxx> wrote:
> > 
> > > --- a/include/linux/workqueue.h
> > > +++ b/include/linux/workqueue.h
> > > @@ -525,11 +525,20 @@ alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...);
> > > 
> > >   #define create_workqueue(name)                                         \
> > >          alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
> > > +#define create_workqueue2(fmt, args...) \
> > > +       alloc_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM, 1, ##args)
> > >   #define create_freezable_workqueue(name)                               \
> > >          alloc_workqueue("%s", __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND | \
> > >                          WQ_MEM_RECLAIM, 1, (name))
> > 
> > Is there any possible preprocessor hack to avoid the renaming of the functions?
> 
> Thanks Lai for having taken a look. As one can see here the last patch of
> this patch series renames create_workqueue2() back to create_workqueue(): https://lore.kernel.org/linux-kernel/20240630222904.627462-1-bvanassche@xxxxxxx/

This can be done with the preprocessor to detect how many arguments
are being used, so then there is no need to do the renaming passes and
conversions can land via subsystems:


diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index fb3993894536..00420f85b881 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -523,13 +523,24 @@ alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...);
 #define alloc_ordered_workqueue(fmt, flags, args...)			\
 	alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | (flags), 1, ##args)
 
-#define create_workqueue(name)						\
-	alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
-#define create_freezable_workqueue(name)				\
-	alloc_workqueue("%s", __WQ_LEGACY | WQ_FREEZABLE | WQ_UNBOUND |	\
-			WQ_MEM_RECLAIM, 1, (name))
-#define create_singlethread_workqueue(name)				\
-	alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
+#define __has_one_arg(args...)  __is_defined(COUNT_ARGS(args))
+
+#define __create_workqueue1(flags, name) \
+                alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM | (flags), \
+				1, name)
+#define __create_workqueue0(flags, fmt, args...) \
+                alloc_workqueue(fmt, __WQ_LEGACY | WQ_MEM_RECLAIM | (flags), \
+				1, args)
+
+#define create_workqueue(args...) \
+        CONCATENATE(__create_workqueue, __has_one_arg(args))\
+                (0, args)
+#define create_freezeable_workqueue(args...) \
+        CONCATENATE(__create_workqueue, __has_one_arg(args))\
+                (WQ_UNBOUND | WQ_FREEZABLE, args)
+#define create_singlethread_workqueue(args...) \
+        CONCATENATE(__create_workqueue, __has_one_arg(args))\
+                (WQ_UNBOUND | __WQ_ORDERED, args)
 
 #define from_work(var, callback_work, work_fieldname)	\
 	container_of(callback_work, typeof(*var), work_fieldname)



Now conversions are one step, e.g.:

diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index d74e829de51c..f7f27c6f1a15 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -1458,8 +1458,7 @@ static int allocate_caches_and_workqueue(struct smbd_connection *info)
 	if (!info->response_mempool)
 		goto out3;
 
-	scnprintf(name, MAX_NAME_LEN, "smbd_%p", info);
-	info->workqueue = create_workqueue(name);
+	info->workqueue = create_workqueue("smbd_%p", info);
 	if (!info->workqueue)
 		goto out4;
 
-- 
Kees Cook