On 02/10/2015 04:03 AM, Peter Zijlstra wrote:
> On Mon, Feb 09, 2015 at 11:06:17PM -0500, Jason Baron wrote:
>> On 02/09/2015 04:50 PM, Peter Zijlstra wrote:
>>> On Mon, Feb 09, 2015 at 08:05:57PM +0000, Jason Baron wrote:
>>>> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
>>>> index 852143a..17d1039 100644
>>>> --- a/kernel/sched/wait.c
>>>> +++ b/kernel/sched/wait.c
>>>> @@ -71,8 +71,11 @@ static void __wake_up_common(wait_queue_head_t *q,
>>>> unsigned int mode,
>>>> unsigned flags = curr->flags;
>>>>
>>>> if (curr->func(curr, mode, wake_flags, key) &&
>>>> - (flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive)
>>>> + (flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive) {
>>>> + if (flags & WQ_FLAG_ROUND_ROBIN)
>>>> + list_move_tail(&curr->task_list, &q->task_list);
>>>> break;
>>>> + }
>>>> }
>>>> }
>>> I think you meant to write something like:
>>>
>>> if (curr->func(curr, mode, wake_flags, key) &&
>>> (flags & WQ_FLAG_EXCLUSIVE)) {
>>> if (flag & WQ_FLAG_ROUND_ROBIN)
>>> list_move_tail(&curr->task_list, &q->task_list);
>>> if (!--nr_exclusive)
>>> break;
>>> }
>>>
>>> Otherwise can only work for nr_exclusive==1.
>> Indeed. I'm also wondering if its worth avoiding the list_move_tail()
>> for the case where nr_exclusive is initially 0. IE the wake all case,
>> where we are just going to end up doing a bunch of list_move_tail()
>> calls, but end up in the same state.
> After writing this email, it occurred to me that you could probably do
> this with a custom wake function.
>
> Where autoremove_wake_function() does a list_del_init() you could do a
> rotate_wake_function() that does list_move_tail().
>
> That would avoid the entire WQ flag muckery.
hmmm...but don't we need the head/tail of the list to add it back too?
Further, we can't just append to tail while walking the list b/c
otherwise it can result in multiple wakeups to the same item. So I could
add to a local list, for example, in __wake_up_common(). And then just
add that to the tail once the list_for_each() finishes.
In terms of the flag, maybe another option would be to have the
wait_queue_func_t return a 'ROTATE_ME' value instead
of 1, since I think we currently only make use of 0 and 1?
Thanks,
-Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
