* [PATCH] lib/klist: Remove mb() before wake_up_process
@ 2022-06-14 14:44 wuchi
2022-06-14 14:58 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: wuchi @ 2022-06-14 14:44 UTC (permalink / raw)
To: alexios.zavras, allison, armijn; +Cc: akpm, gregkh, linux-kernel
Function wake_up_process always executes a general memory barrier,
so remove the mb() before it.
Signed-off-by: wuchi <wuchi.zero@gmail.com>
---
lib/klist.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/klist.c b/lib/klist.c
index 332a4fbf18ff..c44498e31d91 100644
--- a/lib/klist.c
+++ b/lib/klist.c
@@ -194,7 +194,6 @@ static void klist_release(struct kref *kref)
list_del(&waiter->list);
waiter->woken = 1;
- mb();
wake_up_process(waiter->process);
}
spin_unlock(&klist_remove_lock);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] lib/klist: Remove mb() before wake_up_process
2022-06-14 14:44 [PATCH] lib/klist: Remove mb() before wake_up_process wuchi
@ 2022-06-14 14:58 ` Greg KH
2022-06-15 3:30 ` chi wu
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2022-06-14 14:58 UTC (permalink / raw)
To: wuchi; +Cc: alexios.zavras, allison, armijn, akpm, linux-kernel
On Tue, Jun 14, 2022 at 10:44:43PM +0800, wuchi wrote:
> Function wake_up_process always executes a general memory barrier,
> so remove the mb() before it.
Really? On all systems? I do not see that, where does it happen?
> Signed-off-by: wuchi <wuchi.zero@gmail.com>
We need a "real name" for commits.
How did you test this patch?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lib/klist: Remove mb() before wake_up_process
2022-06-14 14:58 ` Greg KH
@ 2022-06-15 3:30 ` chi wu
2022-06-15 6:19 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: chi wu @ 2022-06-15 3:30 UTC (permalink / raw)
To: Greg KH; +Cc: alexios.zavras, allison, armijn, Andrew Morton, linux-kernel
Greg KH <gregkh@linuxfoundation.org> 于2022年6月14日周二 22:58写道:
>
> On Tue, Jun 14, 2022 at 10:44:43PM +0800, wuchi wrote:
> > Function wake_up_process always executes a general memory barrier,
> > so remove the mb() before it.
>
> Really? On all systems? I do not see that, where does it happen?
>
As I understand it, it is on all systems. Please help correct the
mistake, thanks.
1. Follow Documentation/memory-barriers.txt line 2128 ~ 2278,
especially line 2187 ~ 2202 snippet:
A general memory barrier is executed by wake_up() if it wakes
something up.
If it doesn't wake anything up then a memory barrier may or may not be
executed; you must not rely on it. The barrier occurs before
the task state
is accessed, in particular, it sits between the STORE to
indicate the event
and the STORE to set TASK_RUNNING:
CPU 1 (Sleeper) CPU 2 (Waker)
=============================== ===============================
set_current_state(); STORE
event_indicated
smp_store_mb(); wake_up();
STORE current->state ...
<general barrier>
<general barrier>
[*1-1*]
LOAD event_indicated if
((LOAD task->state) & TASK_NORMAL)
STORE task->state
where "task" is the thread being woken up and it equals CPU
1's "current".
2. Follow code wake_up_process in kernel/sched/core.c
wake_up_process
try_to_wake_up
....
raw_spin_lock_irqsave [*2-1*]
smp_mb__after_spinlock [*2-2*]
....
[*2-1*] and [*2-2*] will match [*1-1*], though smp_mb__after_spinlock
does nothing on most architectures,
but the architectures implement ACQUIRE with an smp_mb() after the
LL/SC loop, Following include/linux/spinlock.h
line 172 ~ 178.
3. Following lib/klist.c
klist_release and klist_remove conform to model "SLEEP AND WAKE-UP
FUNCTIONS" in Documentation/memory-barriers.txt,
so we do as the patch show.
> > Signed-off-by: wuchi <wuchi.zero@gmail.com>
>
> We need a "real name" for commits.
>
> How did you test this patch?
Sorry, I didn't. Just found the mb before wake_up_process could be
remove when reading the code,
Maybe you can view this as a question I asked.
thanks for your time
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lib/klist: Remove mb() before wake_up_process
2022-06-15 3:30 ` chi wu
@ 2022-06-15 6:19 ` Greg KH
0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2022-06-15 6:19 UTC (permalink / raw)
To: chi wu; +Cc: alexios.zavras, allison, armijn, Andrew Morton, linux-kernel
On Wed, Jun 15, 2022 at 11:30:51AM +0800, chi wu wrote:
> Greg KH <gregkh@linuxfoundation.org> 于2022年6月14日周二 22:58写道:
> >
> > On Tue, Jun 14, 2022 at 10:44:43PM +0800, wuchi wrote:
> > > Function wake_up_process always executes a general memory barrier,
> > > so remove the mb() before it.
> >
> > Really? On all systems? I do not see that, where does it happen?
> >
> As I understand it, it is on all systems. Please help correct the
> mistake, thanks.
>
> 1. Follow Documentation/memory-barriers.txt line 2128 ~ 2278,
> especially line 2187 ~ 2202 snippet:
> A general memory barrier is executed by wake_up() if it wakes
> something up.
> If it doesn't wake anything up then a memory barrier may or may not be
> executed; you must not rely on it.
So as the documentation states, it might not be there, so if you have to
have a memory barrier, you must not rely on this function to provide it.
So unless you have testing proof otherwise, the code should be correct
as-is.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-15 7:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-14 14:44 [PATCH] lib/klist: Remove mb() before wake_up_process wuchi
2022-06-14 14:58 ` Greg KH
2022-06-15 3:30 ` chi wu
2022-06-15 6:19 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).