Messages in this thread | | | Date | Tue, 22 Mar 2016 12:32:21 +0100 | From | Heiko Carstens <> | Subject | Re: [PATCH 4/3] rtmutex: Avoid barrier in rt_mutex_handle_deadlock |
| |
On Tue, Mar 22, 2016 at 11:21:53AM +0100, Peter Zijlstra wrote: > On Mon, Mar 21, 2016 at 11:16:22AM -0700, Davidlohr Bueso wrote: > > > +/* > > + * Helpers for modifying the state of either the current task, or a foreign > > + * task. Each of these calls come in both full barrier and weak flavors: > > + * > > + * Weak > > + * set_task_state() __set_task_state() > > + * set_current_state() __set_current_state() > > + * > > + * Where set_current_state() and set_task_state() includes a full smp barrier > > + * -after- the write of ->state is correctly serialized with the later test > > + * of whether to actually sleep: > > + * > > + * for (;;) { > > + * set_current_state(TASK_UNINTERRUPTIBLE); > > + * if (event_indicated) > > + * break; > > + * schedule(); > > + * } > > + * > > + * This is commonly necessary for processes sleeping and waking through flag > > + * based events. If the caller does not need such serialization, then use > > + * weaker counterparts, which simply writes the state. > > + * > > + * Refer to Documentation/memory-barriers.txt > > + */ > > I would prefer to pretend set_task_state() does not exist, using it on > anything other than task==current is very very tricky. > > With the below patch; we're only left with: > > arch/s390/mm/fault.c: __set_task_state(tsk, TASK_UNINTERRUPTIBLE); > arch/s390/mm/fault.c: __set_task_state(tsk, TASK_UNINTERRUPTIBLE); > drivers/md/bcache/btree.c: set_task_state(c->gc_thread, TASK_INTERRUPTIBLE); > kernel/exit.c: set_task_state(tsk, TASK_UNINTERRUPTIBLE); > kernel/exit.c: __set_task_state(tsk, TASK_RUNNING); > > exit most probably also has tsk==current, but I didn't check. > > bacache seems to rely on the fact that the task is not running after > kthread_create() to change the state. But I've no idea why; the only > think I can come up with is because load accounting, a new thread blocks > in UNINTERRUPTIBLE which adds to load. But by setting it to > INTERRUPTIBLE before waking up it can actually mess that up. This really > should be fixed. > > And s390 does something entirely vile, no idea what.
For the two s390 usages tsk equals current. So it could be easily replaced with set_current_state().
| |