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 <[email protected]> > > > Cc: Lai Jiangshan <[email protected]> > > > Signed-off-by: Matthew Brost <[email protected]> > > > --- > > > 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
