The following changes since commit 171199f56f5f9bdf1e5d670d09ef1351d8f01bae:

Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20200619-3' into staging (2020-06-22 14:45:25 +0100)

for you to fetch changes up to 7838c67f22a81fcf669785cd6c0876438422071a:

block/nvme: support nested aio_poll() (2020-06-23 15:46:08 +0100)

Daniele Buono (4):

coroutine: support SafeStack in ucontext backend

coroutine: add check for SafeStack in sigaltstack

configure: add flags to support SafeStack

check-block: enable iotests with SafeStack

Stefan Hajnoczi (8):

minikconf: explicitly set encoding to UTF-8

block/nvme: poll queues without q->lock

block/nvme: drop tautologous assertion

block/nvme: don't access CQE after moving cq.head

block/nvme: switch to a NVMeRequest freelist

block/nvme: clarify that free_req_queue is protected by q->lock

block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair

block/nvme: support nested aio_poll()

configure | 73 ++++++++++++

include/qemu/coroutine_int.h | 5 +

block/nvme.c | 220 +++++++++++++++++++++++++----------

util/coroutine-sigaltstack.c | 4 +

util/coroutine-ucontext.c | 28 +++++

block/trace-events | 2 +-

scripts/minikconf.py | 6 +-

tests/check-block.sh | 12 +-

8 files changed, 284 insertions(+), 66 deletions(-)

2.26.2

QEMU block drivers are supposed to support aio_poll() from I/O

completion callback functions. This means completion processing must be

re-entrant.

The standard approach is to schedule a BH during completion processing

and cancel it at the end of processing. If aio_poll() is invoked by a

callback function then the BH will run. The BH continues the suspended

completion processing.

All of this means that request A's cb() can synchronously wait for

request B to complete. Previously the nvme block driver would hang

because it didn't process completions from nested aio_poll().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Sergio Lopez <slp@redhat.com>

Message-id: 20200617132201.1832152-8-stefanha@redhat.com

block/nvme.c | 67 ++++++++++++++++++++++++++++++++++++++++------

block/trace-events | 2 +-

2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c

--- a/block/nvme.c

+++ b/block/nvme.c

@@ -XXX,XX +XXX,XX @@ typedef struct {

int cq_phase;

int free_req_head;

NVMeRequest reqs[NVME_NUM_REQS];

- bool busy;

int need_kick;

int inflight;

+

+ /* Thread-safe, no lock necessary */

+ QEMUBH *completion_bh;

} NVMeQueuePair;

/* Memory mapped registers */

@@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState {

#define NVME_BLOCK_OPT_DEVICE "device"

#define NVME_BLOCK_OPT_NAMESPACE "namespace"

+static void nvme_process_completion_bh(void *opaque);

+

static QemuOptsList runtime_opts = {

.name = "nvme",

.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),

@@ -XXX,XX +XXX,XX @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q,

static void nvme_free_queue_pair(NVMeQueuePair *q)

{

+ if (q->completion_bh) {

+ qemu_bh_delete(q->completion_bh);

+ }

qemu_vfree(q->prp_list_pages);

qemu_vfree(q->sq.queue);

qemu_vfree(q->cq.queue);

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,

q->index = idx;

qemu_co_queue_init(&q->free_req_queue);

q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);

+ q->completion_bh = aio_bh_new(bdrv_get_aio_context(bs),

+ nvme_process_completion_bh, q);

r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages,

s->page_size * NVME_NUM_REQS,

false, &prp_list_iova);

@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)

NvmeCqe *c;

trace_nvme_process_completion(s, q->index, q->inflight);

- if (q->busy || s->plugged) {

- trace_nvme_process_completion_queue_busy(s, q->index);

+ if (s->plugged) {

+ trace_nvme_process_completion_queue_plugged(s, q->index);

return false;

}

- q->busy = true;

+

+ /*

+ * Support re-entrancy when a request cb() function invokes aio_poll().

+ * Pending completions must be visible to aio_poll() so that a cb()

+ * function can wait for the completion of another request.

+ *

+ * The aio_poll() loop will execute our BH and we'll resume completion

+ * processing there.

+ */

+ qemu_bh_schedule(q->completion_bh);

+

assert(q->inflight >= 0);

while (q->inflight) {

int ret;

@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)

assert(req.cb);

nvme_put_free_req_locked(q, preq);

preq->cb = preq->opaque = NULL;

- qemu_mutex_unlock(&q->lock);

- req.cb(req.opaque, ret);

- qemu_mutex_lock(&q->lock);

q->inflight--;

+ qemu_mutex_unlock(&q->lock);

+ req.cb(req.opaque, ret);

+ qemu_mutex_lock(&q->lock);

progress = true;

}

if (progress) {

@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)

*q->cq.doorbell = cpu_to_le32(q->cq.head);

nvme_wake_free_req_locked(q);

}

- q->busy = false;

+

+ qemu_bh_cancel(q->completion_bh);

+

return progress;

+static void nvme_process_completion_bh(void *opaque)

+{

+ NVMeQueuePair *q = opaque;

+ /*

+ * We're being invoked because a nvme_process_completion() cb() function

+ * called aio_poll(). The callback may be waiting for further completions

+ * so notify the device that it has space to fill in more completions now.

+ */

+ smp_mb_release();

+ *q->cq.doorbell = cpu_to_le32(q->cq.head);

+ nvme_wake_free_req_locked(q);

+

+ nvme_process_completion(q);

static void nvme_trace_command(const NvmeCmd *cmd)

{

int i;

@@ -XXX,XX +XXX,XX @@ static void nvme_detach_aio_context(BlockDriverState *bs)

{

BDRVNVMeState *s = bs->opaque;

+ for (int i = 0; i < s->nr_queues; i++) {

+ NVMeQueuePair *q = s->queues[i];

+

+ qemu_bh_delete(q->completion_bh);

+ q->completion_bh = NULL;

+ }

+

aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,

false, NULL, NULL);

}

@@ -XXX,XX +XXX,XX @@ static void nvme_attach_aio_context(BlockDriverState *bs,

s->aio_context = new_context;

aio_set_event_notifier(new_context, &s->irq_notifier,

false, nvme_handle_event, nvme_poll_cb);

+

+ for (int i = 0; i < s->nr_queues; i++) {

+ NVMeQueuePair *q = s->queues[i];

+

+ q->completion_bh =

+ aio_bh_new(new_context, nvme_process_completion_bh, q);

+ }

}

static void nvme_aio_plug(BlockDriverState *bs)

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ nvme_kick(void *s, int queue) "s %p queue %d"

nvme_dma_flush_queue_wait(void *s) "s %p"

nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"

nvme_process_completion(void *s, int index, int inflight) "s %p queue %d inflight %d"

-nvme_process_completion_queue_busy(void *s, int index) "s %p queue %d"

+nvme_process_completion_queue_plugged(void *s, int index) "s %p queue %d"

nvme_complete_command(void *s, int index, int cid) "s %p queue %d cid %d"

nvme_submit_command(void *s, int index, int cid) "s %p queue %d cid %d"

nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x"

2.26.2