On Wed, Jul 10, 2019 at 02:23:23PM -0600, Jens Axboe wrote:
> On 7/10/19 1:52 PM, Josef Bacik wrote:
> > rq-qos sits in the io path so we want to take locks as sparingly as
> > possible. To accomplish this we try not to take the waitqueue head lock
> > unless we are sure we need to go to sleep, and we have an optimization
> > to make sure that we don't starve out existing waiters. Since we check
> > if there are existing waiters locklessly we need to be able to update
> > our view of the waitqueue list after we've added ourselves to the
> > waitqueue. Accomplish this by adding this helper to see if there are
> > more than two waiters on the waitqueue.
> >
> > Suggested-by: Jens Axboe <axboe@xxxxxxxxx>
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > ---
> > include/linux/wait.h | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/include/linux/wait.h b/include/linux/wait.h
> > index b6f77cf60dd7..89c41a7b3046 100644
> > --- a/include/linux/wait.h
> > +++ b/include/linux/wait.h
> > @@ -126,6 +126,27 @@ static inline int waitqueue_active(struct wait_queue_head *wq_head)
> > return !list_empty(&wq_head->head);
> > }
> >
> > +/**
> > + * wq_has_multiple_sleepers - check if there are multiple waiting prcesses
> > + * @wq_head: wait queue head
> > + *
> > + * Returns true of wq_head has multiple waiting processes.
> > + *
> > + * Please refer to the comment for waitqueue_active.
> > + */
> > +static inline bool wq_has_multiple_sleepers(struct wait_queue_head *wq_head)
> > +{
> > + /*
> > + * We need to be sure we are in sync with the
> > + * add_wait_queue modifications to the wait queue.
> > + *
> > + * This memory barrier should be paired with one on the
> > + * waiting side.
> > + */
> > + smp_mb();
> > + return !list_is_singular(&wq_head->head);
> > +}
> > +
> > /**
> > * wq_has_sleeper - check if there are any waiting processes
> > * @wq_head: wait queue head
>
> This (and 2/2) looks good to me, better than v1 for sure. Peter/Ingo,
> are you OK with adding this new helper? For reference, this (and the
> next patch) replace the alternative, which is an open-coding of
> prepare_to_wait():
>
> https://lore.kernel.org/linux-block/20190710190514.86911-1-josef@xxxxxxxxxxxxxx/
Yet another approach would be to have prepare_to_wait*() return this
state, but I think this is ok.
The smp_mb() is superfluous -- in your specific case -- since
preprare_to_wait*() already does one through set_current_state().
So you could do without it, I think.
