Re: [RFC PATCH 1/3] workqueue: Add an interface to taint workqueue lockdep with reclaim

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Oct 21, 2025 at 03:04:14PM -0700, Matthew Brost wrote:
> On Tue, Oct 21, 2025 at 11:56:30AM -1000, Tejun Heo wrote:
> > Hello,

Missed a comment.

> > 
> > On Tue, Oct 21, 2025 at 02:39:50PM -0700, Matthew Brost wrote:
> > > Drivers often use workqueues that are in the reclaim path (e.g., DRM
> > > scheduler workqueues). It is useful to teach lockdep that memory cannot
> > > be allocated on these workqueues. Add an interface to taint workqueue
> > > lockdep with reclaim.
> > 
> > Given that it's about reclaim, "memory cannot be allocated" may be a bit
> > misleading. Can you make the description more accurate? Also, it'd be great

Can fix the comment. The rule is memory cannot be allocated in the
context of reclaim (e.g., GFP_KERNEL).

> > if you can include an example lockdep splat for reference.

My driver (Xe) doesn't break anything but can hack to trigger a lockdep
warning and include it.

Matt

> > 
> > > 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 | 19 +++++++++++++++++++
> > >  kernel/workqueue.c        |  9 +++++++++
> > >  2 files changed, 28 insertions(+)
> > > 
> > > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> > > index dabc351cc127..954c7eb7e225 100644
> > > --- a/include/linux/workqueue.h
> > > +++ b/include/linux/workqueue.h
> > > @@ -553,6 +553,25 @@ alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags, int max_active,
> > >  						1, lockdep_map, ##args))
> > >  #endif
> > >  
> > > +
> > > +#ifdef CONFIG_LOCKDEP
> > > +/**
> > > + * taint_reclaim_workqueue - taint workqueue lockdep map with reclaim
> > > + * @wq: workqueue to taint with reclaim
> > > + * gfp: gfp taint
> >       ^@
> > 
> > > + *
> > > + * Drivers often use workqueues that are in the reclaim path (e.g., DRM
> > > + * scheduler workqueues). It is useful to teach lockdep that memory cannot be
> > > + * allocated on these workqueues.
> > > + */
> > > +extern void taint_reclaim_workqueue(struct workqueue_struct *wq, gfp_t gfp);
> > > +#else
> > > +static inline void taint_reclaim_workqueue(struct workqueue_struct *wq,
> > > +					   gfp_t gfp)
> > 
> > Would a more direct name work better, maybe something like
> > workqueue_warn_on_reclaim()?
> > 
> 
> Can rename, but perhaps not needed depending on what we land on below.
> 
> > Hmm... would it make sense to tie this to WQ_MEM_RECLAIM - ie. enable it
> > implicitly on workqueues w/ the flag set?
> > 
> 
> I had considered this, and for a while I thought WQ_MEM_RECLAIM already
> did what I'm suggesting—especially since I’ve spotted bugs in drivers
> where I would have expected lockdep to catch them.
> 
> In my opinion, this approach is better, but it has a broader kernel-wide
> scope and could potentially break some things. My subsequent patches
> will likely break one or two DRM drivers, so it might not be a concern
> to fix everything that breaks across the kernel. It's up to you which
> route we want to take here.
> 
> Matt 
> 
> > Thanks.
> > 
> > -- 
> > tejun