Messages in this thread | | | From | chi wu <> | Date | Wed, 15 Jun 2022 11:30:51 +0800 | Subject | Re: [PATCH] lib/klist: Remove mb() before wake_up_process |
| |
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
| |