On 07/16, Josef Bacik wrote:
>
> - add a comment about why we don't need a mb for the first data.token check
> which I'm sure Oleg will tell me is wrong and I'll have to send a v4
we don't need it because prepare_to_wait_exclusive() does set_current_state()
and this implies mb().
(in fact I think in this particular case it is not needed at all because
rq_qos_wake_function() sets condition == T under wq_head->lock)
I see nothing wrong in this series. Feel free to add
Reviewed-by: Oleg Nesterov <oleg@xxxxxxxxxx>
But why does it use wq_has_sleeper() ? I do not understand why do we need
mb() before waitqueue_active() in this particular case...
and just for record. iiuc acquire_inflight_cb() can't block, so it is not
clear to me why performance-wise this all is actually better than just
void rq_qos_wait(struct rq_wait *rqw, void *private_data,
acquire_inflight_cb_t *acquire_inflight_cb)
{
struct rq_qos_wait_data data = {
.wq = {
.flags = WQ_FLAG_EXCLUSIVE;
.func = rq_qos_wake_function,
.entry = LIST_HEAD_INIT(data.wq.entry),
},
.task = current,
.rqw = rqw,
.cb = acquire_inflight_cb,
.private_data = private_data,
};
if (!wq_has_sleeper(&rqw->wait) && acquire_inflight_cb(rqw, private_data))
return;
spin_lock_irq(&rqw->wait.lock);
if (list_empty(&wq_entry->entry) && acquire_inflight_cb(rqw, private_data))
data.got_token = true;
else
__add_wait_queue_entry_tail(&rqw->wait, &data.wq);
spin_unlock_irq(&rqw->wait.lock);
for (;;) {
set_current_state(TASK_UNINTERRUPTIBLE);
if (data.got_token)
break;
io_schedule();
}
finish_wait(&rqw->wait, &data.wq);
}
note also that the acquire_inflight_cb argument goes away.
Nevermind, feel free to ignore.
Oleg.
