From 030be573f39fe4f8faf4ba2fb01e94dfdc0b08e5 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Mon, 10 Feb 2020 07:54:06 -0500 Subject: [PATCH] lib/blob: queue up blob persists when one already is ongoing It is possible for multiple blob persists to affect one another. Either by blob->state changes or blob mutable data. Safe way to prevent that is to queue up the persists. Next persist will be executed only after previous one completes. Fixes #1170 Fixes #960 Signed-off-by: Tomasz Zawadzki Change-Id: Iaf95d9238510100b629050bc0d5c2c96c982a60c Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/776 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/blob/blobstore.c | 25 ++++++++++++++++++++++++- lib/blob/blobstore.h | 3 +++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index d7903a5c6..362ad57ad 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -247,6 +247,7 @@ _spdk_blob_alloc(struct spdk_blob_store *bs, spdk_blob_id id) TAILQ_INIT(&blob->xattrs); TAILQ_INIT(&blob->xattrs_internal); + TAILQ_INIT(&blob->pending_persists); return blob; } @@ -268,6 +269,7 @@ static void _spdk_blob_free(struct spdk_blob *blob) { assert(blob != NULL); + assert(TAILQ_EMPTY(&blob->pending_persists)); free(blob->active.extent_pages); free(blob->clean.extent_pages); @@ -1520,6 +1522,7 @@ struct spdk_blob_persist_ctx { spdk_bs_sequence_t *seq; spdk_bs_sequence_cpl cb_fn; void *cb_arg; + TAILQ_ENTRY(spdk_blob_persist_ctx) link; }; static void @@ -1540,22 +1543,34 @@ spdk_bs_batch_clear_dev(struct spdk_blob_persist_ctx *ctx, spdk_bs_batch_t *batc } } +static void _spdk_blob_persist_check_dirty(struct spdk_blob_persist_ctx *ctx); + static void _spdk_blob_persist_complete(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) { struct spdk_blob_persist_ctx *ctx = cb_arg; + struct spdk_blob_persist_ctx *next_persist; struct spdk_blob *blob = ctx->blob; if (bserrno == 0) { _spdk_blob_mark_clean(blob); } + assert(ctx == TAILQ_FIRST(&blob->pending_persists)); + TAILQ_REMOVE(&blob->pending_persists, ctx, link); + + next_persist = TAILQ_FIRST(&blob->pending_persists); + /* Call user callback */ ctx->cb_fn(seq, ctx->cb_arg, bserrno); /* Free the memory */ spdk_free(ctx->pages); free(ctx); + + if (next_persist != NULL) { + _spdk_blob_persist_check_dirty(next_persist); + } } static void @@ -2089,7 +2104,7 @@ _spdk_blob_persist(spdk_bs_sequence_t *seq, struct spdk_blob *blob, _spdk_blob_verify_md_op(blob); - if (blob->state == SPDK_BLOB_STATE_CLEAN) { + if (blob->state == SPDK_BLOB_STATE_CLEAN && TAILQ_EMPTY(&blob->pending_persists)) { cb_fn(seq, cb_arg, 0); return; } @@ -2105,6 +2120,14 @@ _spdk_blob_persist(spdk_bs_sequence_t *seq, struct spdk_blob *blob, ctx->cb_arg = cb_arg; ctx->next_extent_page = 0; + /* Multiple blob persists can affect one another, via blob->state or + * blob mutable data changes. To prevent it, queue up the persists. */ + if (!TAILQ_EMPTY(&blob->pending_persists)) { + TAILQ_INSERT_TAIL(&blob->pending_persists, ctx, link); + return; + } + TAILQ_INSERT_HEAD(&blob->pending_persists, ctx, link); + _spdk_blob_persist_check_dirty(ctx); } diff --git a/lib/blob/blobstore.h b/lib/blob/blobstore.h index 29c30f8db..e55595af5 100644 --- a/lib/blob/blobstore.h +++ b/lib/blob/blobstore.h @@ -166,6 +166,9 @@ struct spdk_blob { bool extent_table_found; bool use_extent_table; + /* A list of pending metadata pending_persists */ + TAILQ_HEAD(, spdk_blob_persist_ctx) pending_persists; + /* Number of data clusters retrived from extent table, * that many have to be read from extent pages. */ uint64_t remaining_clusters_in_et;