[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87cyla8rae.fsf@linux.ibm.com>
Date: Wed, 11 Sep 2024 11:37:29 +0200
From: "Marc Hartmayer" <mhartmay@...ux.ibm.com>
To: Lai Jiangshan <jiangshanlai@...il.com>
Cc: linux-kernel@...r.kernel.org, Lai Jiangshan
<jiangshan.ljs@...group.com>,
Valentin Schneider <vschneid@...hat.com>, Tejun Heo <tj@...nel.org>,
Heiko Carstens <hca@...ux.ibm.com>,
Sven
Schnelle <svens@...ux.ibm.com>, Mete Durlu <meted@...ux.ibm.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>
Subject: Re: [PATCH 1/4] workqueue: Reap workers via kthread_stop() and
remove detach_completion
On Wed, Sep 11, 2024 at 10:27 AM +0200, "Marc Hartmayer" <mhartmay@...ux.ibm.com> wrote:
> On Wed, Sep 11, 2024 at 11:32 AM +0800, Lai Jiangshan <jiangshanlai@...il.com> wrote:
>> On Wed, Sep 11, 2024 at 11:23 AM Lai Jiangshan <jiangshanlai@...il.com> wrote:
>>>
>>> Hello, Marc
>>>
>
> Hi Lai,
>
> […snip…]
>
>>>
>>> I'm not familiar with s390 asm code, but it might be the case that
>>> `worker->pool` is NULL in the in worker_thread() since detach_worker()
>>> resets worker->pool to NULL.
>>>
>>> If it is the case, READ_ONCE(worker->pool) should be used in worker_thread()
>>> to fix the problem.
>>>
>>> (It is weird to me if worker->pool is read multi-time in worker_thread()
>>> since it is used many times, but since READ_ONCE() is not used, it can
>>> be possible).
>>
>> Oh, it can be possible that the worker is created and then destroyed before
>> being waken-up. And if it is the case, READ_ONCE() won't help. I'm going to
>> explore if "worker->pool = NULL;" can be moved out from
>> detach_worker().
>
> I’ll double check if my assumption is true or not (worker->poll ==
> NULL). It may well be that my assumption is wrong.
I applied the following patch on top of commit bc83b4d1f086 ("Merge tag
'bcachefs-2024-09-09' of git://evilpiepirate.org/bcachefs")
>From 9cd804f8e3183422b05a1b36e2544d1175736519 Mon Sep 17 00:00:00 2001
From: Marc Hartmayer <mhartmay@...ux.ibm.com>
Date: Wed, 11 Sep 2024 09:11:41 +0000
Subject: [PATCH] Add printk-debug statements
Signed-off-by: Marc Hartmayer <mhartmay@...ux.ibm.com>
---
kernel/workqueue.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e7b005ff3750..d4c5c68457f7 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3338,11 +3338,16 @@ static void set_pf_worker(bool val)
static int worker_thread(void *__worker)
{
struct worker *worker = __worker;
- struct worker_pool *pool = worker->pool;
+ if (unlikely(!worker))
+ printk(KERN_ERR "OOOOOHHHH NOOOOOOO, WE DO NOT HAVE A WORKER.\n");
+
+ struct worker_pool *pool = READ_ONCE(worker->pool);
/* tell the scheduler that this is a workqueue worker */
set_pf_worker(true);
woke_up:
+ if (unlikely(!pool))
+ printk(KERN_ERR "OOOOOHHHH NOOOOOOO, WE DO NOT HAVE A POOL.\n");
raw_spin_lock_irq(&pool->lock);
/* am I supposed to die? */
--
2.43.0
And it shows that pool is NULL in case of the crash. Hope this helps.
>
> Thanks for having a look!
>
>>
>> Thanks
>> Lai
> --
> Kind regards / Beste Grüße
> Marc Hartmayer
>
> IBM Deutschland Research & Development GmbH
> Vorsitzender des Aufsichtsrats: Wolfgang Wendt
> Geschäftsführung: David Faller
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Powered by blists - more mailing lists