Re: [PATCH 1/2] wait: Introduce per-task wait_queue_t

[Oleg Nesterov]

On 06/10, Rasmus Villemoes wrote:
>
> This introduces a single wait_queue_t into the task structure.
> Functions which need to wait, but which do not call other functions
> that might wait while on the wait queue, may use current->__wq

I am not going to argue, but I am not sure that wait_event() (changed
by the next patch) meets this criteria...

wait_event(wq, something_nontrivial_which_uses_wait_event_too()) is
legal currently although perhaps nobody does this.

> +static inline wait_queue_t *current_wq_get(void)
> +{
> +	wait_queue_t *wq = &current->__wq;
> +	BUG_ON(wq->flags != WQ_FLAG_AVAILABLE);
> +	wq->flags = 0;
> +	return wq;
> +}
> +static inline void current_wq_put(wait_queue_t *wq)
> +{
> +	BUG_ON(wq != &current->__wq);
> +	wq->flags = WQ_FLAG_AVAILABLE;
> +}

Or, perhaps, current_wq_get() can simply check list_empty(->task_list)
initialized by copy_process().

This way you do not need current_wq_put(), WQ_FLAG_AVAILABLE, and you
can kill INIT_LIST_HEAD() in ___wait_event().

Honestly, I am not sure about this patch... sizeof(wait_queue_t) is not
that large, and otoh it is not good to have yet another "rarely used"
member in the already huge task_struct. But again, I won't insist.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/