Re: [PATCH 00/14] Fix bug about invalid wake up problem

[Tejun Heo <tj@kernel.org>]

Hello, Libin.

I'm completely confused by this series....

On Thu, Aug 29, 2013 at 09:57:35PM +0800, Libin wrote:
> 1)Problem Description:
> The prototype of invalid wakeup problem is as follows:
> ========================================================================
> ----------------------------
> Consumer Thread
> ----------------------------
> ...
> if (list_empty(&list)){
> 	//location a 
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	schedule();
> }
> ...
> ----------------------------
> Producer Thread
> ----------------------------
> ...
> list_add_tail(&item, &list);
> wake_up_process(A);
> ...

This is of course broken.  set_current_state() should of course come
*before* the conditions is checked.  This is just plain broken code.

> In most cases, the kernel codes use a form similar to the following:
> --------------------------------------------
>  ...
>  //location a
>  ...
>  set_current_state(TASK_INTERRUPTIBLE);
>  //location b
>  while (list_empty(&product_list)){
> 	schedule();	//location c
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	//location d
> }
> __set_current_state(TASK_RUNNING);
> ...
> --------------------------------------------
> At location a, consumer is preempted, and producer is scheduled,
> adding item to product_list and waking up consumer. After consumer is
> re-scheduled, calling set_current_state to set itself as state
> TASK_INTERRUPTIBLE, if it is preempted again before __set_current_state(TASK_RUNNING),
> also trigger the invalid wakeup problem that the consumer will not be scheduled

Why?  Getting preempted != calling schedule().  A preempted task will
be rescheduled *regardless* of ifs current->state; otherwise, the
whole kernel is severely broken.  Tasks never get deactivated while
preempted.

> and the item be added by producer can't be consumed.
> 
> The following circumstance will also trigger this issue:
> At location c, consumer is scheduled out, and be preempted after calling
> set_current_state(TASK_INTERRUPTIBLE) when it be re-schdeuled.

What?

> 2)Test:
> I have written a test module to trigger the problem by adding some
> synchronization condition. I will post it in the form of an attachment soon.
> 
> Test result as follows:
> [103135.332683] wakeup_test: create two kernel threads - producer & consumer
> [103135.332686] wakeup_test: module loaded successfully
> [103135.332692] wakeup_test: kthread producer try to wake up the kthread consumer
> [103165.299865] wakeup_test: kthread consumer have waited for 30s, indicating
> trigger an invalid wakeup problem!

The most interesting question here is "what were you testing?".  Can
you please post the test code?

For the whole series,

 Nacked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun