[dm-devel] [PATCH 4/5] blk-mq: introduce blk_get_request_notify
Bart Van Assche
Bart.VanAssche at wdc.com
Mon Jan 22 17:13:03 UTC 2018
On Mon, 2018-01-22 at 11:35 +0800, Ming Lei wrote:
> DM-MPATH need to allocate request from underlying queue, but when the
> allocation fails, there is no way to make underlying queue's RESTART
> to restart DM's queue.
>
> This patch introduces blk_get_request_notify() for this purpose, and
> caller need to pass 'wait_queue_entry_t' to this function, and make
> sure it is initialized well, so after the current allocation fails,
> DM will get notified when there is request available from underlying
> queue.
Please mention that this is only a partial solution because the case when
e.g. blk_insert_cloned_request() returns BLK_STS_RESOURCE is not handled.
This could help for drivers that support a very low queue depth (lpfc) but
probably won't be that useful for other drivers.
> + /*
> + * If caller requires notification when tag is available, add
> + * wait entry of 'data->notifier' to the wait queue.
> + */
> + if (data->flags & BLK_MQ_REQ_NOWAIT) {
> + bool added = false;
> +
> + spin_lock_irq(&ws->wait.lock);
> + if (list_empty(&data->notifier->entry))
> + __add_wait_queue(&ws->wait, data->notifier);
> + else
> + added = true;
> + spin_unlock_irq(&ws->wait.lock);
> +
> + if (added)
> + return BLK_MQ_TAG_FAIL;
> +
> + tag = __blk_mq_get_tag(data, bt);
> + if (tag != -1)
> + goto found_tag;
> + return BLK_MQ_TAG_FAIL;
> + }
Sorry but I don't like this approach. Adding "data->notifier" to the wait
queue creates a link between two request queues, e.g. a dm-mpath queue and
one of the paths that is a member of that dm-mpath queue. This creates the
potential for ugly races between e.g. "data->notifier" being triggered and
removal of the dm-mpath queue.
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 88c558f71819..bec2f675f8f1 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -160,6 +160,7 @@ struct blk_mq_alloc_data {
> struct request_queue *q;
> blk_mq_req_flags_t flags;
> unsigned int shallow_depth;
> + wait_queue_entry_t *notifier;
If others would agree with the approach of this patch please use another name
than "notifier". In the context of the Linux kernel a notifier is an instance
of struct notifier_block. The above "notifier" member is not a notifier but a
wait queue entry.
Thanks,
Bart.
More information about the dm-devel
mailing list