| Message ID | 20240809222827.3211998-4-matthew.brost@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use user-defined workqueue lockdep map for drm sched | expand |
On 10-08-2024 03:58, Matthew Brost wrote: > Add an interface for a user-defined workqueue lockdep map, which is > helpful when multiple workqueues are created for the same purpose. This > also helps avoid leaking lockdep maps on each workqueue creation. > > v2: > - Add alloc_workqueue_lockdep_map (Tejun) > v3: > - Drop __WQ_USER_OWNED_LOCKDEP (Tejun) > - static inline alloc_ordered_workqueue_lockdep_map (Tejun) > > Cc: Tejun Heo <tj@kernel.org> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > include/linux/workqueue.h | 52 +++++++++++++++++++++++++++++++++++++++ > kernel/workqueue.c | 28 +++++++++++++++++++++ > 2 files changed, 80 insertions(+) > > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h > index 4eb8f9563136..8ccbf510880b 100644 > --- a/include/linux/workqueue.h > +++ b/include/linux/workqueue.h > @@ -507,6 +507,58 @@ void workqueue_softirq_dead(unsigned int cpu); > __printf(1, 4) struct workqueue_struct * > alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...); > > +#ifdef CONFIG_LOCKDEP > +/** > + * alloc_workqueue_lockdep_map - allocate a workqueue with user-defined lockdep_map > + * @fmt: printf format for the name of the workqueue > + * @flags: WQ_* flags > + * @max_active: max in-flight work items, 0 for default > + * @lockdep_map: user-defined lockdep_map > + * @...: args for @fmt > + * > + * Same as alloc_workqueue but with the a user-define lockdep_map. Useful for > + * workqueues created with the same purpose and to avoid leaking a lockdep_map > + * on each workqueue creation. > + * > + * RETURNS: > + * Pointer to the allocated workqueue on success, %NULL on failure. > + */ > +__printf(1, 5) struct workqueue_struct * > +alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags, int max_active, > + struct lockdep_map *lockdep_map, ...); > + > +/** > + * alloc_ordered_workqueue_lockdep_map - allocate an ordered workqueue with > + * user-defined lockdep_map > + * > + * @fmt: printf format for the name of the workqueue > + * @flags: WQ_* flags (only WQ_FREEZABLE and WQ_MEM_RECLAIM are meaningful) > + * @lockdep_map: user-defined lockdep_map > + * @args: args for @fmt > + * > + * Same as alloc_ordered_workqueue but with the a user-define lockdep_map. > + * Useful for workqueues created with the same purpose and to avoid leaking a > + * lockdep_map on each workqueue creation. > + * > + * RETURNS: > + * Pointer to the allocated workqueue on success, %NULL on failure. > + */ > +__printf(1, 4) static inline struct workqueue_struct * > +alloc_ordered_workqueue_lockdep_map(const char *fmt, unsigned int flags, > + struct lockdep_map *lockdep_map, ...) > +{ > + struct workqueue_struct *wq; > + va_list args; > + > + va_start(args, lockdep_map); > + wq = alloc_workqueue_lockdep_map(fmt, WQ_UNBOUND | __WQ_ORDERED | flags, > + 1, lockdep_map, args); > + va_end(args); > + > + return wq; > +} > +#endif > + > /** > * alloc_ordered_workqueue - allocate an ordered workqueue > * @fmt: printf format for the name of the workqueue > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index 24df85589dc0..f4b50a995e99 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -4775,11 +4775,17 @@ static void wq_init_lockdep(struct workqueue_struct *wq) > > static void wq_unregister_lockdep(struct workqueue_struct *wq) > { > + if (wq->lockdep_map != &wq->__lockdep_map) > + return; > + > lockdep_unregister_key(&wq->key); > } > > static void wq_free_lockdep(struct workqueue_struct *wq) > { > + if (wq->lockdep_map != &wq->__lockdep_map) > + return; > + > if (wq->lock_name != wq->name) > kfree(wq->lock_name); > } > @@ -5756,6 +5762,28 @@ struct workqueue_struct *alloc_workqueue(const char *fmt, > } > EXPORT_SYMBOL_GPL(alloc_workqueue); > > +#ifdef CONFIG_LOCKDEP > +__printf(1, 5) > +struct workqueue_struct * > +alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags, > + int max_active, struct lockdep_map *lockdep_map, ...) > +{ > + struct workqueue_struct *wq; > + va_list args; > + > + va_start(args, lockdep_map); > + wq = __alloc_workqueue(fmt, flags, max_active, args); > + va_end(args); > + if (!wq) > + return NULL; > + > + wq->lockdep_map = lockdep_map; How about NULL check and fallback to dynamic allocation, just to be safe ? > + > + return wq; > +} > +EXPORT_SYMBOL_GPL(alloc_workqueue_lockdep_map); > +#endif > + > static bool pwq_busy(struct pool_workqueue *pwq) > { > int i;
On Fri, Aug 09, 2024 at 03:28:25PM -0700, Matthew Brost wrote: > Add an interface for a user-defined workqueue lockdep map, which is > helpful when multiple workqueues are created for the same purpose. This > also helps avoid leaking lockdep maps on each workqueue creation. > > v2: > - Add alloc_workqueue_lockdep_map (Tejun) > v3: > - Drop __WQ_USER_OWNED_LOCKDEP (Tejun) > - static inline alloc_ordered_workqueue_lockdep_map (Tejun) > > Cc: Tejun Heo <tj@kernel.org> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> 1-3 look fine to me. Would applying them to wq/for-6.12 and pulling them from the dri tree work? Thanks.
On Tue, Aug 13, 2024 at 08:52:26AM -1000, Tejun Heo wrote: > On Fri, Aug 09, 2024 at 03:28:25PM -0700, Matthew Brost wrote: > > Add an interface for a user-defined workqueue lockdep map, which is > > helpful when multiple workqueues are created for the same purpose. This > > also helps avoid leaking lockdep maps on each workqueue creation. > > > > v2: > > - Add alloc_workqueue_lockdep_map (Tejun) > > v3: > > - Drop __WQ_USER_OWNED_LOCKDEP (Tejun) > > - static inline alloc_ordered_workqueue_lockdep_map (Tejun) > > > > Cc: Tejun Heo <tj@kernel.org> > > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > 1-3 look fine to me. Would applying them to wq/for-6.12 and pulling them > from the dri tree work? > Yes, I wanted to get this into 6.12 as I believe we are removing forceprobe for our driver and it would be good to have this in for that. Any idea what this is about though [1]? Matt [1] https://patchwork.freedesktop.org/patch/607769/?series=136701&rev=3#comment_1105151 > Thanks. > > -- > tejun
On Tue, Aug 13, 2024 at 06:55:20PM +0000, Matthew Brost wrote: > On Tue, Aug 13, 2024 at 08:52:26AM -1000, Tejun Heo wrote: > > On Fri, Aug 09, 2024 at 03:28:25PM -0700, Matthew Brost wrote: > > > Add an interface for a user-defined workqueue lockdep map, which is > > > helpful when multiple workqueues are created for the same purpose. This > > > also helps avoid leaking lockdep maps on each workqueue creation. > > > > > > v2: > > > - Add alloc_workqueue_lockdep_map (Tejun) > > > v3: > > > - Drop __WQ_USER_OWNED_LOCKDEP (Tejun) > > > - static inline alloc_ordered_workqueue_lockdep_map (Tejun) > > > > > > Cc: Tejun Heo <tj@kernel.org> > > > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > > > 1-3 look fine to me. Would applying them to wq/for-6.12 and pulling them > > from the dri tree work? > > > > Yes, I wanted to get this into 6.12 as I believe we are removing > forceprobe for our driver and it would be good to have this in for that. > > Any idea what this is about though [1]? It looks like misattribution to me. I'll apply 1-3 to wq/for-6.12. Thanks.
On Fri, Aug 09, 2024 at 03:28:25PM -0700, Matthew Brost wrote: > Add an interface for a user-defined workqueue lockdep map, which is > helpful when multiple workqueues are created for the same purpose. This > also helps avoid leaking lockdep maps on each workqueue creation. > > v2: > - Add alloc_workqueue_lockdep_map (Tejun) > v3: > - Drop __WQ_USER_OWNED_LOCKDEP (Tejun) > - static inline alloc_ordered_workqueue_lockdep_map (Tejun) > > Cc: Tejun Heo <tj@kernel.org> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> Applied 1-3 to wq/for-6.12. Thanks.
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 4eb8f9563136..8ccbf510880b 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -507,6 +507,58 @@ void workqueue_softirq_dead(unsigned int cpu); __printf(1, 4) struct workqueue_struct * alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...); +#ifdef CONFIG_LOCKDEP +/** + * alloc_workqueue_lockdep_map - allocate a workqueue with user-defined lockdep_map + * @fmt: printf format for the name of the workqueue + * @flags: WQ_* flags + * @max_active: max in-flight work items, 0 for default + * @lockdep_map: user-defined lockdep_map + * @...: args for @fmt + * + * Same as alloc_workqueue but with the a user-define lockdep_map. Useful for + * workqueues created with the same purpose and to avoid leaking a lockdep_map + * on each workqueue creation. + * + * RETURNS: + * Pointer to the allocated workqueue on success, %NULL on failure. + */ +__printf(1, 5) struct workqueue_struct * +alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags, int max_active, + struct lockdep_map *lockdep_map, ...); + +/** + * alloc_ordered_workqueue_lockdep_map - allocate an ordered workqueue with + * user-defined lockdep_map + * + * @fmt: printf format for the name of the workqueue + * @flags: WQ_* flags (only WQ_FREEZABLE and WQ_MEM_RECLAIM are meaningful) + * @lockdep_map: user-defined lockdep_map + * @args: args for @fmt + * + * Same as alloc_ordered_workqueue but with the a user-define lockdep_map. + * Useful for workqueues created with the same purpose and to avoid leaking a + * lockdep_map on each workqueue creation. + * + * RETURNS: + * Pointer to the allocated workqueue on success, %NULL on failure. + */ +__printf(1, 4) static inline struct workqueue_struct * +alloc_ordered_workqueue_lockdep_map(const char *fmt, unsigned int flags, + struct lockdep_map *lockdep_map, ...) +{ + struct workqueue_struct *wq; + va_list args; + + va_start(args, lockdep_map); + wq = alloc_workqueue_lockdep_map(fmt, WQ_UNBOUND | __WQ_ORDERED | flags, + 1, lockdep_map, args); + va_end(args); + + return wq; +} +#endif + /** * alloc_ordered_workqueue - allocate an ordered workqueue * @fmt: printf format for the name of the workqueue diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 24df85589dc0..f4b50a995e99 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -4775,11 +4775,17 @@ static void wq_init_lockdep(struct workqueue_struct *wq) static void wq_unregister_lockdep(struct workqueue_struct *wq) { + if (wq->lockdep_map != &wq->__lockdep_map) + return; + lockdep_unregister_key(&wq->key); } static void wq_free_lockdep(struct workqueue_struct *wq) { + if (wq->lockdep_map != &wq->__lockdep_map) + return; + if (wq->lock_name != wq->name) kfree(wq->lock_name); } @@ -5756,6 +5762,28 @@ struct workqueue_struct *alloc_workqueue(const char *fmt, } EXPORT_SYMBOL_GPL(alloc_workqueue); +#ifdef CONFIG_LOCKDEP +__printf(1, 5) +struct workqueue_struct * +alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags, + int max_active, struct lockdep_map *lockdep_map, ...) +{ + struct workqueue_struct *wq; + va_list args; + + va_start(args, lockdep_map); + wq = __alloc_workqueue(fmt, flags, max_active, args); + va_end(args); + if (!wq) + return NULL; + + wq->lockdep_map = lockdep_map; + + return wq; +} +EXPORT_SYMBOL_GPL(alloc_workqueue_lockdep_map); +#endif + static bool pwq_busy(struct pool_workqueue *pwq) { int i;
Add an interface for a user-defined workqueue lockdep map, which is helpful when multiple workqueues are created for the same purpose. This also helps avoid leaking lockdep maps on each workqueue creation. v2: - Add alloc_workqueue_lockdep_map (Tejun) v3: - Drop __WQ_USER_OWNED_LOCKDEP (Tejun) - static inline alloc_ordered_workqueue_lockdep_map (Tejun) Cc: Tejun Heo <tj@kernel.org> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Signed-off-by: Matthew Brost <matthew.brost@intel.com> --- include/linux/workqueue.h | 52 +++++++++++++++++++++++++++++++++++++++ kernel/workqueue.c | 28 +++++++++++++++++++++ 2 files changed, 80 insertions(+)