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.
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)
+{
+}
+#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 45320e27a16c..fea410c20b71 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5846,6 +5846,15 @@ alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags,
return wq;
}
EXPORT_SYMBOL_GPL(alloc_workqueue_lockdep_map);
+
+void taint_reclaim_workqueue(struct workqueue_struct *wq, gfp_t gfp)
+{
+ fs_reclaim_acquire(gfp);
+ lock_map_acquire(wq->lockdep_map);
+ lock_map_release(wq->lockdep_map);
+ fs_reclaim_release(gfp);
+}
+EXPORT_SYMBOL_GPL(taint_reclaim_workqueue);
#endif
static bool pwq_busy(struct pool_workqueue *pwq)
--
2.34.1On 10/21/25 23:39, 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.
Oh that is so wonderfully evil. I'm absolutely in favor of doing this.
But can't we check for the existing WQ_MEM_RECLAIM flag in the workqueue handling instead?
Additional to that we should also make sure that the same wq is used for timeout and free and that this wq is single threaded *and* has the WQ_MEM_RECLAIM flag set.
Otherwise we run into the same lifetime issue with the job and memory reclaim during device reset as well.
Regards,
Christian.
>
> 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)
> +{
> +}
> +#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 45320e27a16c..fea410c20b71 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -5846,6 +5846,15 @@ alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags,
> return wq;
> }
> EXPORT_SYMBOL_GPL(alloc_workqueue_lockdep_map);
> +
> +void taint_reclaim_workqueue(struct workqueue_struct *wq, gfp_t gfp)
> +{
> + fs_reclaim_acquire(gfp);
> + lock_map_acquire(wq->lockdep_map);
> + lock_map_release(wq->lockdep_map);
> + fs_reclaim_release(gfp);
> +}
> +EXPORT_SYMBOL_GPL(taint_reclaim_workqueue);
> #endif
>
> static bool pwq_busy(struct pool_workqueue *pwq)
On Tue, Oct 28, 2025 at 10:32:54AM +0100, Christian König wrote:
> On 10/21/25 23:39, 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.
>
> Oh that is so wonderfully evil. I'm absolutely in favor of doing this.
>
> But can't we check for the existing WQ_MEM_RECLAIM flag in the workqueue handling instead?
>
Tejun suggested tying the lockdep annotation to WQ_MEM_RECLAIM, but the
entire kernel explodes because many workqueues throughout Linux don’t
adhere to this rule. Here's a link to my latest reply to Tejun [1].
[1] https://patchwork.freedesktop.org/patch/682494/?series=156284&rev=1#comment_1255380
> Additional to that we should also make sure that the same wq is used for timeout and free and that this wq is single threaded *and* has the WQ_MEM_RECLAIM flag set.
>
Currently, free runs on the same work queue as run_job. We could look
into moving it to a separate queue, but that’s a separate issue.
IIRC the workqueue_struct is private and so we can't fish that out in
the DRM scheduler without adding helpers to workqueue layer. Ofc we
could do that too if you think this would be helpful.
> Otherwise we run into the same lifetime issue with the job and memory reclaim during device reset as well.
>
My patches in this series taint the submit_wq and timeout_wq in the DRM
scheduler [2]. I have a solid understanding of reclaim rules, and this
change helped uncover some convoluted cases in Xe—specifically in our
device reset code involving power management and reclaim [3]. So I can
confirm this has been quite helpful.
Matt
[2] https://patchwork.freedesktop.org/patch/682496/?series=156284&rev=1
[3] https://patchwork.freedesktop.org/series/156292/
> Regards,
> Christian.
>
> >
> > 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)
> > +{
> > +}
> > +#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 45320e27a16c..fea410c20b71 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -5846,6 +5846,15 @@ alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags,
> > return wq;
> > }
> > EXPORT_SYMBOL_GPL(alloc_workqueue_lockdep_map);
> > +
> > +void taint_reclaim_workqueue(struct workqueue_struct *wq, gfp_t gfp)
> > +{
> > + fs_reclaim_acquire(gfp);
> > + lock_map_acquire(wq->lockdep_map);
> > + lock_map_release(wq->lockdep_map);
> > + fs_reclaim_release(gfp);
> > +}
> > +EXPORT_SYMBOL_GPL(taint_reclaim_workqueue);
> > #endif
> >
> > static bool pwq_busy(struct pool_workqueue *pwq)
>
Hello, On Tue, Oct 28, 2025 at 01:16:43PM -0700, Matthew Brost wrote: > On Tue, Oct 28, 2025 at 10:32:54AM +0100, Christian König wrote: > > On 10/21/25 23:39, 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. > > > > Oh that is so wonderfully evil. I'm absolutely in favor of doing this. > > > > But can't we check for the existing WQ_MEM_RECLAIM flag in the workqueue handling instead? > > > > Tejun suggested tying the lockdep annotation to WQ_MEM_RECLAIM, but the > entire kernel explodes because many workqueues throughout Linux don’t > adhere to this rule. Here's a link to my latest reply to Tejun [1]. How about making it a WQ flag? Thanks. -- tejun
On Wed, Oct 29, 2025 at 05:06:46AM -1000, Tejun Heo wrote: > Hello, > > On Tue, Oct 28, 2025 at 01:16:43PM -0700, Matthew Brost wrote: > > On Tue, Oct 28, 2025 at 10:32:54AM +0100, Christian König wrote: > > > On 10/21/25 23:39, 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. > > > > > > Oh that is so wonderfully evil. I'm absolutely in favor of doing this. > > > > > > But can't we check for the existing WQ_MEM_RECLAIM flag in the workqueue handling instead? > > > > > > > Tejun suggested tying the lockdep annotation to WQ_MEM_RECLAIM, but the > > entire kernel explodes because many workqueues throughout Linux don’t > > adhere to this rule. Here's a link to my latest reply to Tejun [1]. > > How about making it a WQ flag? > That could work too. We want to enforce rules of drivers actually set these flags setting passing workqueues to the DRM scheduler. Any objection to adding helpers to the workqueue layer to fish the information we'd like to enforce? Matt > Thanks. > > -- > tejun
On Wed, Oct 29, 2025 at 09:46:15AM -0700, Matthew Brost wrote: > On Wed, Oct 29, 2025 at 05:06:46AM -1000, Tejun Heo wrote: > > How about making it a WQ flag? > > That could work too. We want to enforce rules of drivers actually set > these flags setting passing workqueues to the DRM scheduler. Any > objection to adding helpers to the workqueue layer to fish the > information we'd like to enforce? Difficult to tell definitively without looking at the helpers but no objections to the general idea. Thanks. -- tejun
On 10/28/25 21:16, Matthew Brost wrote:
> On Tue, Oct 28, 2025 at 10:32:54AM +0100, Christian König wrote:
>> On 10/21/25 23:39, 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.
>>
>> Oh that is so wonderfully evil. I'm absolutely in favor of doing this.
>>
>> But can't we check for the existing WQ_MEM_RECLAIM flag in the workqueue handling instead?
>>
>
> Tejun suggested tying the lockdep annotation to WQ_MEM_RECLAIM, but the
> entire kernel explodes because many workqueues throughout Linux don’t
> adhere to this rule. Here's a link to my latest reply to Tejun [1].
>
> [1] https://patchwork.freedesktop.org/patch/682494/?series=156284&rev=1#comment_1255380
Sorry my fault, I hadn't read up to the latest discussion when I wrote the mail.
My educated guess is that a lot of wq just set WQ_MEM_RECLAIM to be guaranteed to to start even under memory pressure.
So yeah probably best to keep your approach here for now and somebody from core MM should take a look at cleaning it up later on.
>> Additional to that we should also make sure that the same wq is used for timeout and free and that this wq is single threaded *and* has the WQ_MEM_RECLAIM flag set.
>>
>
> Currently, free runs on the same work queue as run_job. We could look
> into moving it to a separate queue, but that’s a separate issue.
We really need to make sure the free and timeout wq are the same and single threaded.
The hack the scheduler currently does with removing and re-inserting the job on a timeout is something we should really try to fix.
> IIRC the workqueue_struct is private and so we can't fish that out in
> the DRM scheduler without adding helpers to workqueue layer. Ofc we
> could do that too if you think this would be helpful.
I might be wrong, but IIRC there was a helper to get the flags from the wq.
That should be enough to test if it is single threaded or not.
>
>> Otherwise we run into the same lifetime issue with the job and memory reclaim during device reset as well.
>>
>
> My patches in this series taint the submit_wq and timeout_wq in the DRM
> scheduler [2]. I have a solid understanding of reclaim rules, and this
> change helped uncover some convoluted cases in Xe—specifically in our
> device reset code involving power management and reclaim [3]. So I can
> confirm this has been quite helpful.
Yeah, completely agree. We most likely have quite a bunch of issues in our reset code path as well.
Regards,
Christian.
>
> Matt
>
> [2] https://patchwork.freedesktop.org/patch/682496/?series=156284&rev=1
> [3] https://patchwork.freedesktop.org/series/156292/
>
>> Regards,
>> Christian.
>>
>>>
>>> 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)
>>> +{
>>> +}
>>> +#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 45320e27a16c..fea410c20b71 100644
>>> --- a/kernel/workqueue.c
>>> +++ b/kernel/workqueue.c
>>> @@ -5846,6 +5846,15 @@ alloc_workqueue_lockdep_map(const char *fmt, unsigned int flags,
>>> return wq;
>>> }
>>> EXPORT_SYMBOL_GPL(alloc_workqueue_lockdep_map);
>>> +
>>> +void taint_reclaim_workqueue(struct workqueue_struct *wq, gfp_t gfp)
>>> +{
>>> + fs_reclaim_acquire(gfp);
>>> + lock_map_acquire(wq->lockdep_map);
>>> + lock_map_release(wq->lockdep_map);
>>> + fs_reclaim_release(gfp);
>>> +}
>>> +EXPORT_SYMBOL_GPL(taint_reclaim_workqueue);
>>> #endif
>>>
>>> static bool pwq_busy(struct pool_workqueue *pwq)
>>
Hello,
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
if you can include an example lockdep splat for reference.
> 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()?
Hmm... would it make sense to tie this to WQ_MEM_RECLAIM - ie. enable it
implicitly on workqueues w/ the flag set?
Thanks.
--
tejun
On Tue, Oct 21, 2025 at 11:56:30AM -1000, Tejun Heo wrote: > Hello, > > 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 > if you can include an example lockdep splat for reference. > > > 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
Hello, On Tue, Oct 21, 2025 at 03:04:14PM -0700, Matthew Brost wrote: > > 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. Yeah, it is bothersome that WQ_MEM_RECLAIM doesn't currently have a way to ensure compliance. I just didn't know about the lockdep mechanism. Can you please update the patch so that WQ_MEM_RECLAIM implicitly enables the checking? Thanks. -- tejun
On Tue, Oct 21, 2025 at 01:28:31PM -1000, Tejun Heo wrote: > Hello, > > On Tue, Oct 21, 2025 at 03:04:14PM -0700, Matthew Brost wrote: > > > 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. > > Yeah, it is bothersome that WQ_MEM_RECLAIM doesn't currently have a way to > ensure compliance. I just didn't know about the lockdep mechanism. Can you I agree this is the best route to ensure compliance. > please update the patch so that WQ_MEM_RECLAIM implicitly enables the > checking? > Sure, but a bunch of things immediately break—including a convoluted case in my driver. I can fix the kernel to the extent that my CI catches issues, and fix any obvious cases through manual inspection. However, I suspect that if we merge this, we'll be dealing with fallout throughout a kernel RC cycle. Matt > Thanks. > > -- > tejun
On Tue, Oct 21, 2025 at 06:22:19PM -0700, Matthew Brost wrote: > > please update the patch so that WQ_MEM_RECLAIM implicitly enables the > > checking? > > Sure, but a bunch of things immediately break—including a convoluted > case in my driver. I can fix the kernel to the extent that my CI catches > issues, and fix any obvious cases through manual inspection. However, > I suspect that if we merge this, we'll be dealing with fallout > throughout a kernel RC cycle. Sure, we're still early in this cycle and can try to resolve as much as possible and if there's just too much, we can make it optional and so on. Thanks. -- tejun
On Tue, Oct 21, 2025 at 03:51:08PM -1000, Tejun Heo wrote: > On Tue, Oct 21, 2025 at 06:22:19PM -0700, Matthew Brost wrote: > > > please update the patch so that WQ_MEM_RECLAIM implicitly enables the > > > checking? > > > > Sure, but a bunch of things immediately break—including a convoluted > > case in my driver. I can fix the kernel to the extent that my CI catches > > issues, and fix any obvious cases through manual inspection. However, > > I suspect that if we merge this, we'll be dealing with fallout > > throughout a kernel RC cycle. > > Sure, we're still early in this cycle and can try to resolve as much as > possible and if there's just too much, we can make it optional and so on. > I’ve come to the conclusion that the entire kernel is going to explode if all WQ_MEM_RECLAIM workqueues are annotated with lockdep. I made this change and tried booting Linux, but quickly ran into five issues before giving up. It seems that many parts of the kernel allocate memory with GFP_KERNEL or take locks that allocate memory in workqueues marked with WQ_MEM_RECLAIM. There are literally hundreds of workqueues created with WQ_MEM_RECLAIM, both directly [1] and via the create*workqueue helpers, which also set this flag. So, what’s your advice here? Personally, I don’t have the bandwidth to drive fixing the entire kernel. Maybe we could add the annotation helpers introduced in this series, so drivers that really want to enforce this rule—like the DRM driver—can opt in? And perhaps we could add a export Kconfig option that enables this for all WQ_MEM_RECLAIM workqueues, and let the community gradually enable it and start fixing their code? Matt [1] https://elixir.bootlin.com/linux/v6.17.5/C/ident/WQ_MEM_RECLAIM > Thanks. > > -- > tejun
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
Hello, On Tue, Oct 21, 2025 at 03:06:55PM -0700, Matthew Brost wrote: > > > 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). Oh, I meant that e.g. GPF_ATOMIC or GFP_NOFS reclaims should be fine. It's just that we can't recurse into reclaim from WQ_RECLAIM workqueue, right? Thanks. -- tejun
On Tue, Oct 21, 2025 at 01:25:52PM -1000, Tejun Heo wrote: > Hello, > > On Tue, Oct 21, 2025 at 03:06:55PM -0700, Matthew Brost wrote: > > > > 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). > > Oh, I meant that e.g. GPF_ATOMIC or GFP_NOFS reclaims should be fine. It's > just that we can't recurse into reclaim from WQ_RECLAIM workqueue, right? > Yes, exactly. Will adjust. Matt > Thanks. > > -- > tejun
© 2016 - 2026 Red Hat, Inc.