ocf: lock queue list during create/delete

OCF queue list needs to be managed synchronously.
This patch uses our own mutex to achieve that because we cannot rely on
  ocf_mngt_cache_lock() as it may produce deadlocks when using
  cleaner.

Alternative way would be to use trylock, but
  we need to register a poller for it and do locking concurently
  which doesn't seem to be possible in callbacks of io channel.

We agreed with OCF team that we are going to change this part
  when OCF will deliver safe ocf_mngt_cache_lock() function.

This patch fixes a very rare failure on our CI that looked like this:
```
04:33:02 vbdev_ocf.c: 134:stop_vbdev: *NOTICE*: Not stopping cache instance 'Malloc0' because it is referenced by other OCF bdev
04:33:03 MalCache1: Core core2 successfully removed
04:33:03 MalCache1: Stopping cache
04:33:03 MalCache1: Done saving cache state!
04:33:03 src/ocf/mngt/ocf_mngt_cache.c:1576:2: runtime error: pointer index expression with base 0x000000000000 overflowed to 0xffffffffffffffa8
04:33:03     #0 0x7f3c52d54c26 in _ocf_mngt_cache_stop src/ocf/mngt/ocf_mngt_cache.c:1576
04:33:03     #1 0x7f3c52d5579f in ocf_mngt_cache_stop src/ocf/mngt/ocf_mngt_cache.c:1657
04:33:03     #2 0x7f3c52cbe9f4 in stop_vbdev /var/jenkins/workspace/NVMe_tests/nvme_phy_autotest/spdk/lib/bdev/ocf/vbdev_ocf.c:147
04:33:03     #3 0x7f3c52cbf4a0 in vbdev_ocf_destruct /var/jenkins/workspace/NVMe_tests/nvme_phy_autotest/spdk/lib/bdev/ocf/vbdev_ocf.c:216
```

Change-Id: Id6fafb444958f3becdc480e44762074c6c081e1f
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450682
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This commit is contained in:
Vitaliy Mysak 2019-04-23 22:34:25 +00:00 committed by Darek Stojaczyk
parent 4e7fb25066
commit 244d6e3daa
3 changed files with 31 additions and 2 deletions

View File

@ -288,9 +288,31 @@ vbdev_ocf_ctx_data_secure_erase(ctx_data_t *ctx_data)
}
}
int vbdev_ocf_queue_create(ocf_cache_t cache, ocf_queue_t *queue, const struct ocf_queue_ops *ops)
{
int rc;
struct vbdev_ocf_cache_ctx *ctx = ocf_cache_get_priv(cache);
pthread_mutex_lock(&ctx->lock);
rc = ocf_queue_create(cache, queue, ops);
pthread_mutex_unlock(&ctx->lock);
return rc;
}
void vbdev_ocf_queue_put(ocf_queue_t queue)
{
ocf_cache_t cache = ocf_queue_get_cache(queue);
struct vbdev_ocf_cache_ctx *ctx = ocf_cache_get_priv(cache);
pthread_mutex_lock(&ctx->lock);
ocf_queue_put(queue);
pthread_mutex_unlock(&ctx->lock);
}
void vbdev_ocf_cache_ctx_put(struct vbdev_ocf_cache_ctx *ctx)
{
if (env_atomic_dec_return(&ctx->refcnt) == 0) {
pthread_mutex_destroy(&ctx->lock);
free(ctx);
}
}

View File

@ -44,6 +44,7 @@ extern ocf_ctx_t vbdev_ocf_ctx;
/* Context of cache instance */
struct vbdev_ocf_cache_ctx {
pthread_mutex_t lock;
env_atomic refcnt;
};
@ -53,4 +54,9 @@ void vbdev_ocf_cache_ctx_get(struct vbdev_ocf_cache_ctx *ctx);
int vbdev_ocf_ctx_init(void);
void vbdev_ocf_ctx_cleanup(void);
/* Thread safe queue creation and deletion
* These are wrappers for original ocf_queue_create() and ocf_queue_put() */
int vbdev_ocf_queue_create(ocf_cache_t cache, ocf_queue_t *queue, const struct ocf_queue_ops *ops);
void vbdev_ocf_queue_put(ocf_queue_t queue);
#endif

View File

@ -708,7 +708,7 @@ io_device_create_cb(void *io_device, void *ctx_buf)
struct vbdev_ocf_qcxt *qctx = ctx_buf;
int rc;
rc = ocf_queue_create(vbdev->ocf_cache, &qctx->queue, &queue_ops);
rc = vbdev_ocf_queue_create(vbdev->ocf_cache, &qctx->queue, &queue_ops);
if (rc) {
return rc;
}
@ -742,7 +742,7 @@ io_device_destroy_cb(void *io_device, void *ctx_buf)
spdk_strerror(ENOMEM));
}
ocf_queue_put(qctx->queue);
vbdev_ocf_queue_put(qctx->queue);
}
static void
@ -854,6 +854,7 @@ start_cache(struct vbdev_ocf *vbdev)
}
vbdev_ocf_cache_ctx_get(vbdev->cache_ctx);
pthread_mutex_init(&vbdev->cache_ctx->lock, NULL);
rc = ocf_mngt_cache_start(vbdev_ocf_ctx, &vbdev->ocf_cache, &vbdev->cfg.cache);
if (rc) {