workqueue: make alloc_workqueue() take printf fmt and args for name
Tejun Heo
maintainerPATCH 1/1 workqueue: make alloc_workqueue() take printf fmt and args for name
alloc_workqueue() currently expects the passed in @name pointer to remain accessible. This is inconvenient and a bit silly given that the whole wq is being dynamically allocated. This patch updates alloc_workqueue() and friends to take printf format string instead of opaque string and matching varargs at the end. The name is allocated together with the wq and formatted.
alloc_ordered_workqueue() is converted to a macro to unify varargs handling with alloc_workqueue(), and, while at it, add comment to alloc_workqueue().
None of the current in-kernel users pass in string with '%' as constant name and this change shouldn't cause any problem.
---
Christoph, you probably were seeing the rescuer_task->comm in ps output. wq->name currently is currently only used for error reporting.
I don't really mind this patch going through a different tree as I don't have any other wq changes scheduled at this point, so please feel free to route this through xfs or -mm.
Thank you.
include/linux/workqueue.h | 48 ++++++++++++++++++++++++++++++---------------- kernel/workqueue.c | 32 +++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 26 deletions(-)
include/linux/workqueue.h +32 -16
@@ -297,32 +297,51 @@ extern struct workqueue_struct *system_unbound_wq;extern struct workqueue_struct *system_freezable_wq;extern struct workqueue_struct * -__alloc_workqueue_key (const char *name,unsigned int flags,int max_active, -struct lock_class_key *key,const char *lock_name); +__alloc_workqueue_key (const char *fmt,unsigned int flags,int max_active, +struct lock_class_key *key,const char *lock_name, ...) + __attribute__((format (printf,1 ,6 ))); +/** +* alloc_workqueue - allocate a workqueue +* @fmt: printf format for the name of the workqueue +* @flags: WQ_* flags +* @max_active: max in-flight work items, 0 for default +* @args: args for @fmt +* +* Allocate a workqueue with the specified parameters. For detailed +* information on WQ_* flags, please refer to Documentation/workqueue.txt. +* +* The __lock_name macro dance is to guarantee that single lock_class_key +* doesn't end up with different namesm, which isn't allowed by lockdep. +* +* RETURNS: +* Pointer to the allocated workqueue on success, %NULL on failure. +*/ #ifdef CONFIG_LOCKDEP -#define alloc_workqueue (name, flags, max_active) \ +#define alloc_workqueue(fmt, flags, max_active, args...) \ ({ \ static struct lock_class_key __key; \ const char *__lock_name; \ \ - if (__builtin_constant_p(name)) \ - __lock_name = (name); \ + if (__builtin_constant_p(fmt)) \ + __lock_name = (fmt); \ else \ - __lock_name = #name; \ + __lock_name = #fmt; \ \ - __alloc_workqueue_key((name), (flags), (max_active), \ - &__key, __lock_name); \ + __alloc_workqueue_key((fmt), (flags), (max_active), \ + &__key, __lock_name, ##args); \ })#else -#define alloc_workqueue (name, flags, max_active) \ - __alloc_workqueue_key((name), (flags), (max_active), NULL, NULL) +#define alloc_workqueue(fmt, flags, max_active, args...) \ + __alloc_workqueue_key((fmt), (flags), (max_active), \ + NULL, NULL, ##args)#endif /** * alloc_ordered_workqueue - allocate an ordered workqueue -* @name: name of the workqueue +* @fmt: printf format for the name of the workqueue * @flags: WQ_* flags (only WQ_FREEZABLE and WQ_MEM_RECLAIM are meaningful) +* @args: args for @fmt * * Allocate an ordered workqueue. An ordered workqueue executes at * most one work item at any given time in the queued order. They are
@@ -331,11 +350,8 @@ __alloc_workqueue_key(const char *name, unsigned int flags, int max_active,* RETURNS: * Pointer to the allocated workqueue on success, %NULL on failure. */ -static inline struct workqueue_struct * -alloc_ordered_workqueue (const char *name,unsigned int flags) -{ -return alloc_workqueue (name,WQ_UNBOUND | flags,1 ); -} +#define alloc_ordered_workqueue(fmt, flags, args...) \ + alloc_workqueue(fmt, WQ_UNBOUND | (flags), 1, ##args)#define create_workqueue (name) \ alloc_workqueue((name), WQ_MEM_RECLAIM, 1)
kernel/workqueue.c +22 -10
@@ -242,10 +242,10 @@ struct workqueue_struct {int nr_drainers;/* W: drain in progress */ int saved_max_active;/* W: saved cwq max_active */ -const char *name;/* I: workqueue name */ #ifdef CONFIG_LOCKDEP struct lockdep_map lockdep_map;#endif +char name[];/* I: workqueue name */ };struct workqueue_struct *system_wq__read_mostly ;
@@ -2954,14 +2954,29 @@ static int wq_clamp_max_active(int max_active, unsigned int flags,return clamp_val (max_active,1 , lim); } -struct workqueue_struct *__alloc_workqueue_key (const char *name, +struct workqueue_struct *__alloc_workqueue_key (const char *fmt,unsigned int flags,int max_active,struct lock_class_key *key, -const char *lock_name) +const char *lock_name, ...) { +va_list args, args1;struct workqueue_struct *wq;unsigned int cpu; +size_t namelen; + +/* determine namelen, allocate wq and format name */ +va_start (args, lock_name); +va_copy (args1, args); + namelen =vsnprintf (NULL ,0 , fmt, args) +1 ; + + wq =kzalloc (sizeof (*wq) + namelen,GFP_KERNEL ); +if (!wq) + goto err; + +vsnprintf (wq->name, namelen, fmt, args1); +va_end (args); +va_end (args1);/* * Workqueues which may be used during memory reclaim should
@@ -2978,12 +2993,9 @@ struct workqueue_struct *__alloc_workqueue_key(const char *name, flags |=WQ_HIGHPRI ; max_active = max_active ?:WQ_DFL_ACTIVE ; - max_active =wq_clamp_max_active (max_active, flags, name); - - wq =kzalloc (sizeof (*wq),GFP_KERNEL ); -if (!wq) - goto err; + max_active =wq_clamp_max_active (max_active, flags, wq->name); +/* init wq */ wq->flags = flags; wq->saved_max_active = max_active;mutex_init (&wq->flush_mutex);
@@ -2991,7 +3003,6 @@ struct workqueue_struct *__alloc_workqueue_key(const char *name,INIT_LIST_HEAD (&wq->flusher_queue);INIT_LIST_HEAD (&wq->flusher_overflow); - wq->name = name;lockdep_init_map (&wq->lockdep_map, lock_name, key,0 );INIT_LIST_HEAD (&wq->list);
@@ -3020,7 +3031,8 @@ struct workqueue_struct *__alloc_workqueue_key(const char *name,if (!rescuer) goto err; - rescuer->task =kthread_create (rescuer_thread, wq,"%s" , name); + rescuer->task =kthread_create (rescuer_thread, wq,"%s" , + wq->name);if (IS_ERR (rescuer->task)) goto err;
Quoted 7 lines · Tejun Heo
Christoph, you probably were seeing the
rescuer_task->commin ps output.wq->namecurrently is currently only used for error reporting.I don't really mind this patch going through a different tree as I don't have any other wq changes scheduled at this point, so please feel free to route this through xfs or -mm.
Ben pointed the bug out in a patch that I'd like to get into 3.2 and -stable. I'll probably repost it with a local workaround for now, and will switch to the nicer interface later.