The following changes since commit d992f2f1368ceb92e6bfd8efece174110f4236ff:

Merge remote-tracking branch 'remotes/artyom/tags/pull-sun4v-20170226' into staging (2017-02-26 22:40:23 +0000)

are available in the git repository at:

for you to fetch changes up to 1ab17f9f5c63c2798d707aeb22588e4fcc17b2cd:

tests-aio-multithread: use atomic_read properly (2017-02-27 14:00:53 +0000)

----------------------------------------------------------------

Paolo Bonzini (4):

curl: do not use aio_context_acquire/release

nfs: do not use aio_context_acquire/release

iscsi: do not use aio_context_acquire/release

tests-aio-multithread: use atomic_read properly

block/curl.c | 24 ++++++++-----

block/iscsi.c | 83 ++++++++++++++++++++++++++++++++++----------

block/nfs.c | 23 +++++++++---

tests/test-aio-multithread.c | 4 +--

4 files changed, 100 insertions(+), 34 deletions(-)

2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

nodes[id].next is written by other threads. If atomic_read is not used

(matching atomic_set in mcs_mutex_lock!) the compiler can optimize the

whole "if" away!

Reported-by: Alex Bennée <alex.bennee@linaro.org>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Tested-by: Greg Kurz <groug@kaod.org>

Message-id: 20170227111726.9237-1-pbonzini@redhat.com

tests/test-aio-multithread.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test-aio-multithread.c b/tests/test-aio-multithread.c

--- a/tests/test-aio-multithread.c

+++ b/tests/test-aio-multithread.c

@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_lock(void)

static void mcs_mutex_unlock(void)

int next;

- if (nodes[id].next == -1) {

+ if (atomic_read(&nodes[id].next) == -1) {

if (atomic_read(&mutex_head) == id &&

atomic_cmpxchg(&mutex_head, id, -1) == id) {

/* Last item in the list, exit. */

@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_unlock(void)

/* Wake up the next in line. */

- next = nodes[id].next;

+ next = atomic_read(&nodes[id].next);

nodes[next].locked = 0;

qemu_futex_wake(&nodes[next].locked, 1);

}

2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Now that all bottom halves and callbacks take care of taking the

AioContext lock, we can migrate some users away from it and to a

specific QemuMutex or CoMutex.

Protect libiscsi calls with a QemuMutex. Callbacks are invoked

using bottom halves, so we don't even have to drop it around

callback invocations.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20170222180725.28611-4-pbonzini@redhat.com

block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------

1 file changed, 64 insertions(+), 19 deletions(-)

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

--- a/block/iscsi.c

+++ b/block/iscsi.c

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

int events;

QEMUTimer *nop_timer;

QEMUTimer *event_timer;

+ QemuMutex mutex;

struct scsi_inquiry_logical_block_provisioning lbp;

struct scsi_inquiry_block_limits bl;

unsigned char *zeroblock;

@@ -XXX,XX +XXX,XX @@ static int iscsi_translate_sense(struct scsi_sense *sense)

return ret;

+/* Called (via iscsi_service) with QemuMutex held. */

static void

iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,

void *command_data, void *opaque)

@@ -XXX,XX +XXX,XX @@ static const AIOCBInfo iscsi_aiocb_info = {

static void iscsi_process_read(void *arg);

static void iscsi_process_write(void *arg);

+/* Called with QemuMutex held. */

static void

iscsi_set_events(IscsiLun *iscsilun)

@@ -XXX,XX +XXX,XX @@ iscsi_process_read(void *arg)

IscsiLun *iscsilun = arg;

struct iscsi_context *iscsi = iscsilun->iscsi;

- aio_context_acquire(iscsilun->aio_context);

+ qemu_mutex_lock(&iscsilun->mutex);

iscsi_service(iscsi, POLLIN);

iscsi_set_events(iscsilun);

- aio_context_release(iscsilun->aio_context);

+ qemu_mutex_unlock(&iscsilun->mutex);

}

static void

@@ -XXX,XX +XXX,XX @@ iscsi_process_write(void *arg)

IscsiLun *iscsilun = arg;

struct iscsi_context *iscsi = iscsilun->iscsi;

- aio_context_acquire(iscsilun->aio_context);

+ qemu_mutex_lock(&iscsilun->mutex);

iscsi_service(iscsi, POLLOUT);

iscsi_set_events(iscsilun);

- aio_context_release(iscsilun->aio_context);

+ qemu_mutex_unlock(&iscsilun->mutex);

}

static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)

@@ -XXX,XX +XXX,XX @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,

uint64_t lba;

uint32_t num_sectors;

bool fua = flags & BDRV_REQ_FUA;

+ int r = 0;

if (fua) {

assert(iscsilun->dpofua);

@@ -XXX,XX +XXX,XX @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,

lba = sector_qemu2lun(sector_num, iscsilun);

num_sectors = sector_qemu2lun(nb_sectors, iscsilun);

iscsi_co_init_iscsitask(iscsilun, &iTask);

+ qemu_mutex_lock(&iscsilun->mutex);

retry:

if (iscsilun->use_16_for_rw) {

#if LIBISCSI_API_VERSION >= (20160603)

@@ -XXX,XX +XXX,XX @@ retry:

#endif

while (!iTask.complete) {

iscsi_set_events(iscsilun);

+ qemu_mutex_unlock(&iscsilun->mutex);

qemu_coroutine_yield();

+ qemu_mutex_lock(&iscsilun->mutex);

if (iTask.task != NULL) {

@@ -XXX,XX +XXX,XX @@ retry:

if (iTask.status != SCSI_STATUS_GOOD) {

iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors);

- return iTask.err_code;

+ r = iTask.err_code;

+ goto out_unlock;

}

iscsi_allocmap_set_allocated(iscsilun, sector_num, nb_sectors);

- return 0;

+out_unlock:

+ qemu_mutex_unlock(&iscsilun->mutex);

+ return r;

}

@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,

goto out;

}

+ qemu_mutex_lock(&iscsilun->mutex);

retry:

if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,

sector_qemu2lun(sector_num, iscsilun),

8 + 16, iscsi_co_generic_cb,

&iTask) == NULL) {

ret = -ENOMEM;

- goto out;

+ goto out_unlock;

}

while (!iTask.complete) {

iscsi_set_events(iscsilun);

+ qemu_mutex_unlock(&iscsilun->mutex);

qemu_coroutine_yield();

+ qemu_mutex_lock(&iscsilun->mutex);

}

if (iTask.do_retry) {

@@ -XXX,XX +XXX,XX @@ retry:

* because the device is busy or the cmd is not

* supported) we pretend all blocks are allocated

* for backwards compatibility */

- goto out;

+ goto out_unlock;

}

lbas = scsi_datain_unmarshall(iTask.task);

if (lbas == NULL) {

ret = -EIO;

- goto out;

+ goto out_unlock;

}

lbasd = &lbas->descriptors[0];

if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {

ret = -EIO;

- goto out;

+ goto out_unlock;

}

*pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);

@@ -XXX,XX +XXX,XX @@ retry:

if (*pnum > nb_sectors) {

*pnum = nb_sectors;

}

+out_unlock:

+ qemu_mutex_unlock(&iscsilun->mutex);

out:

if (iTask.task != NULL) {

scsi_free_scsi_task(iTask.task);

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,

num_sectors = sector_qemu2lun(nb_sectors, iscsilun);

iscsi_co_init_iscsitask(iscsilun, &iTask);

+ qemu_mutex_lock(&iscsilun->mutex);

retry:

if (iscsilun->use_16_for_rw) {

#if LIBISCSI_API_VERSION >= (20160603)

@@ -XXX,XX +XXX,XX @@ retry:

#endif

while (!iTask.complete) {

iscsi_set_events(iscsilun);

+ qemu_mutex_unlock(&iscsilun->mutex);

qemu_coroutine_yield();

+ qemu_mutex_lock(&iscsilun->mutex);

}

if (iTask.task != NULL) {

@@ -XXX,XX +XXX,XX @@ retry:

iTask.complete = 0;

goto retry;

}

+ qemu_mutex_unlock(&iscsilun->mutex);

if (iTask.status != SCSI_STATUS_GOOD) {

return iTask.err_code;

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)

struct IscsiTask iTask;

iscsi_co_init_iscsitask(iscsilun, &iTask);

+ qemu_mutex_lock(&iscsilun->mutex);

retry:

if (iscsi_synchronizecache10_task(iscsilun->iscsi, iscsilun->lun, 0, 0, 0,

0, iscsi_co_generic_cb, &iTask) == NULL) {

@@ -XXX,XX +XXX,XX @@ retry:

while (!iTask.complete) {

iscsi_set_events(iscsilun);

+ qemu_mutex_unlock(&iscsilun->mutex);

qemu_coroutine_yield();

+ qemu_mutex_lock(&iscsilun->mutex);

}

if (iTask.task != NULL) {

@@ -XXX,XX +XXX,XX @@ retry:

iTask.complete = 0;

goto retry;

}

+ qemu_mutex_unlock(&iscsilun->mutex);

if (iTask.status != SCSI_STATUS_GOOD) {

return iTask.err_code;

@@ -XXX,XX +XXX,XX @@ retry:

}

#ifdef __linux__

+/* Called (via iscsi_service) with QemuMutex held. */

static void

iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,

void *command_data, void *opaque)

@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,

acb->task->expxferlen = acb->ioh->dxfer_len;

data.size = 0;

+ qemu_mutex_lock(&iscsilun->mutex);

if (acb->task->xfer_dir == SCSI_XFER_WRITE) {

if (acb->ioh->iovec_count == 0) {

data.data = acb->ioh->dxferp;

@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,

iscsi_aio_ioctl_cb,

(data.size > 0) ? &data : NULL,

acb) != 0) {

+ qemu_mutex_unlock(&iscsilun->mutex);

scsi_free_scsi_task(acb->task);

qemu_aio_unref(acb);

return NULL;

@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,

}

iscsi_set_events(iscsilun);

+ qemu_mutex_unlock(&iscsilun->mutex);

return &acb->common;

}

@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)

IscsiLun *iscsilun = bs->opaque;

struct IscsiTask iTask;

struct unmap_list list;

+ int r = 0;

if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {

return -ENOTSUP;

@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)

list.num = count / iscsilun->block_size;

iscsi_co_init_iscsitask(iscsilun, &iTask);

+ qemu_mutex_lock(&iscsilun->mutex);

retry:

if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1,

iscsi_co_generic_cb, &iTask) == NULL) {

- return -ENOMEM;

+ r = -ENOMEM;

+ goto out_unlock;

}

while (!iTask.complete) {

iscsi_set_events(iscsilun);

+ qemu_mutex_unlock(&iscsilun->mutex);

qemu_coroutine_yield();

+ qemu_mutex_lock(&iscsilun->mutex);

}

if (iTask.task != NULL) {

@@ -XXX,XX +XXX,XX @@ retry:

/* the target might fail with a check condition if it

is not happy with the alignment of the UNMAP request

we silently fail in this case */

- return 0;

+ goto out_unlock;

}

if (iTask.status != SCSI_STATUS_GOOD) {

- return iTask.err_code;

+ r = iTask.err_code;

+ goto out_unlock;

}

iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,

count >> BDRV_SECTOR_BITS);

- return 0;

+out_unlock:

+ qemu_mutex_unlock(&iscsilun->mutex);

+ return r;

}

static int

@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,

uint64_t lba;

uint32_t nb_blocks;

bool use_16_for_ws = iscsilun->use_16_for_rw;

+ int r = 0;

if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {

return -ENOTSUP;

@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,

2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Now that all bottom halves and callbacks take care of taking the

AioContext lock, we can migrate some users away from it and to a

specific QemuMutex or CoMutex.

Protect libnfs calls with a QemuMutex. Callbacks are invoked

using bottom halves, so we don't even have to drop it around

callback invocations.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20170222180725.28611-3-pbonzini@redhat.com

block/nfs.c | 23 +++++++++++++++++++----

1 file changed, 19 insertions(+), 4 deletions(-)

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

--- a/block/nfs.c

+++ b/block/nfs.c

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

int events;

bool has_zero_init;

AioContext *aio_context;

+ QemuMutex mutex;

blkcnt_t st_blocks;

bool cache_used;

NFSServer *server;

@@ -XXX,XX +XXX,XX @@ static void nfs_parse_filename(const char *filename, QDict *options,

static void nfs_process_read(void *arg);

static void nfs_process_write(void *arg);

+/* Called with QemuMutex held. */

static void nfs_set_events(NFSClient *client)

int ev = nfs_which_events(client->context);

@@ -XXX,XX +XXX,XX @@ static void nfs_process_read(void *arg)

{

NFSClient *client = arg;

- aio_context_acquire(client->aio_context);

+ qemu_mutex_lock(&client->mutex);

nfs_service(client->context, POLLIN);

nfs_set_events(client);

- aio_context_release(client->aio_context);

+ qemu_mutex_unlock(&client->mutex);

}

static void nfs_process_write(void *arg)

{

NFSClient *client = arg;

- aio_context_acquire(client->aio_context);

+ qemu_mutex_lock(&client->mutex);

nfs_service(client->context, POLLOUT);

nfs_set_events(client);

- aio_context_release(client->aio_context);

+ qemu_mutex_unlock(&client->mutex);

}

static void nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)

@@ -XXX,XX +XXX,XX @@ static void nfs_co_generic_bh_cb(void *opaque)

aio_co_wake(task->co);

}

+/* Called (via nfs_service) with QemuMutex held. */

static void

nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,

void *private_data)

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,

nfs_co_init_task(bs, &task);

task.iov = iov;

+ qemu_mutex_lock(&client->mutex);

if (nfs_pread_async(client->context, client->fh,

offset, bytes, nfs_co_generic_cb, &task) != 0) {

+ qemu_mutex_unlock(&client->mutex);

return -ENOMEM;

nfs_set_events(client);

+ qemu_mutex_unlock(&client->mutex);

while (!task.complete) {

qemu_coroutine_yield();

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,

buf = iov->iov[0].iov_base;

}

+ qemu_mutex_lock(&client->mutex);

if (nfs_pwrite_async(client->context, client->fh,

offset, bytes, buf,

nfs_co_generic_cb, &task) != 0) {

+ qemu_mutex_unlock(&client->mutex);

if (my_buffer) {

g_free(buf);

}

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,

}

nfs_set_events(client);

+ qemu_mutex_unlock(&client->mutex);

while (!task.complete) {

qemu_coroutine_yield();

}

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs)

nfs_co_init_task(bs, &task);

+ qemu_mutex_lock(&client->mutex);

if (nfs_fsync_async(client->context, client->fh, nfs_co_generic_cb,

&task) != 0) {

+ qemu_mutex_unlock(&client->mutex);

return -ENOMEM;

}

nfs_set_events(client);

+ qemu_mutex_unlock(&client->mutex);

while (!task.complete) {

qemu_coroutine_yield();

}

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

{

NFSClient *client = bs->opaque;

nfs_client_close(client);

+ qemu_mutex_destroy(&client->mutex);

}

static NFSServer *nfs_config(QDict *options, Error **errp)

@@ -XXX,XX +XXX,XX @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,

if (ret < 0) {

return ret;

}

+ qemu_mutex_init(&client->mutex);

bs->total_sectors = ret;

ret = 0;

return ret;

@@ -XXX,XX +XXX,XX @@ static int nfs_has_zero_init(BlockDriverState *bs)

return client->has_zero_init;

}

+/* Called (via nfs_service) with QemuMutex held. */

static void

nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data,

void *private_data)

2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Now that all bottom halves and callbacks take care of taking the

AioContext lock, we can migrate some users away from it and to a

specific QemuMutex or CoMutex.

Protect BDRVCURLState access with a QemuMutex.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20170222180725.28611-2-pbonzini@redhat.com

block/curl.c | 24 +++++++++++++++---------

1 file changed, 15 insertions(+), 9 deletions(-)

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

--- a/block/curl.c

+++ b/block/curl.c

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

char *cookie;

bool accept_range;

AioContext *aio_context;

+ QemuMutex mutex;

char *username;

char *password;

char *proxyusername;

@@ -XXX,XX +XXX,XX @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,

return FIND_RET_NONE;

+/* Called with s->mutex held. */

static void curl_multi_check_completion(BDRVCURLState *s)

int msgs_in_queue;

@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)

continue;

}

+ qemu_mutex_unlock(&s->mutex);

acb->common.cb(acb->common.opaque, -EPROTO);

+ qemu_mutex_lock(&s->mutex);

qemu_aio_unref(acb);

state->acb[i] = NULL;

}

@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)

+/* Called with s->mutex held. */

static void curl_multi_do_locked(CURLState *s)

{

CURLSocket *socket, *next_socket;

@@ -XXX,XX +XXX,XX @@ static void curl_multi_do(void *arg)

{

CURLState *s = (CURLState *)arg;

- aio_context_acquire(s->s->aio_context);

+ qemu_mutex_lock(&s->s->mutex);

curl_multi_do_locked(s);

- aio_context_release(s->s->aio_context);

+ qemu_mutex_unlock(&s->s->mutex);

}

static void curl_multi_read(void *arg)

{

CURLState *s = (CURLState *)arg;

- aio_context_acquire(s->s->aio_context);

+ qemu_mutex_lock(&s->s->mutex);

curl_multi_do_locked(s);

curl_multi_check_completion(s->s);

- aio_context_release(s->s->aio_context);

+ qemu_mutex_unlock(&s->s->mutex);

}

static void curl_multi_timeout_do(void *arg)

@@ -XXX,XX +XXX,XX @@ static void curl_multi_timeout_do(void *arg)

- aio_context_acquire(s->aio_context);

+ qemu_mutex_lock(&s->mutex);

curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);

curl_multi_check_completion(s);

- aio_context_release(s->aio_context);

+ qemu_mutex_unlock(&s->mutex);

#else

abort();

#endif

@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,

curl_easy_cleanup(state->curl);

state->curl = NULL;

+ qemu_mutex_init(&s->mutex);

curl_attach_aio_context(bs, bdrv_get_aio_context(bs));

qemu_opts_del(opts);

@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)

CURLAIOCB *acb = p;

BlockDriverState *bs = acb->common.bs;

BDRVCURLState *s = bs->opaque;

- AioContext *ctx = bdrv_get_aio_context(bs);

size_t start = acb->sector_num * BDRV_SECTOR_SIZE;

size_t end;

- aio_context_acquire(ctx);

+ qemu_mutex_lock(&s->mutex);

// In case we have the requested data already (e.g. read-ahead),

// we can just call the callback and be done.

@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)

curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);

out:

- aio_context_release(ctx);

+ qemu_mutex_unlock(&s->mutex);

if (ret != -EINPROGRESS) {

acb->common.cb(acb->common.opaque, ret);

qemu_aio_unref(acb);

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

DPRINTF("CURL: Close\n");

curl_detach_aio_context(bs);

+ qemu_mutex_destroy(&s->mutex);

g_free(s->cookie);

g_free(s->url);

2.9.3