From ed7848f2df2e1bc0e72013b8096e70f4de27e541 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Wed, 19 Aug 2020 12:11:41 -0700 Subject: [PATCH] blob: handle overlapping open case We only create one spdk_blob object for a given blob, and just increase the ref_count if it is opened multiple times. bs_open_blob would do the lookup for existing opened blobs. But if the blob is opened again, before the previous open operation has completed, we would end up with two spdk_blob objects for the same blob. Solution is to do another lookup when the open operation completes. If we find the blob, free the one we just finished opening and return the existing one instead. Also added unit test that failed on the existing code but passes now with this patch. Signed-off-by: Jim Harris Reported-by: Mike Cui Change-Id: I00c3a913b413deddf06f0b63f7a669efb2b5658f Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3855 Reviewed-by: Tomasz Zawadzki Reviewed-by: Shuhei Matsumoto Reviewed-by: Ben Walker Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins --- lib/blob/blobstore.c | 10 +++++++ test/unit/lib/blob/blob.c/blob_ut.c | 42 ++++++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index a906cb1e0..286acfe8e 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -6648,6 +6648,7 @@ static void bs_open_blob_cpl(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) { struct spdk_blob *blob = cb_arg; + struct spdk_blob *existing; if (bserrno != 0) { blob_free(blob); @@ -6656,6 +6657,15 @@ bs_open_blob_cpl(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) return; } + existing = blob_lookup(blob->bs, blob->id); + if (existing) { + blob_free(blob); + existing->open_ref++; + seq->cpl.u.blob_handle.blob = existing; + bs_sequence_finish(seq, 0); + return; + } + blob->open_ref++; spdk_bit_array_set(blob->bs->open_blobids, blob->id); diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 6e51842e3..d20eb70d7 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -47,8 +47,8 @@ struct spdk_blob_store *g_bs; spdk_blob_id g_blobid; -struct spdk_blob *g_blob; -int g_bserrno; +struct spdk_blob *g_blob, *g_blob2; +int g_bserrno, g_bserrno2; struct spdk_xattr_names *g_names; int g_done; char *g_xattr_names[] = {"first", "second", "third"}; @@ -170,6 +170,18 @@ blob_op_with_handle_complete(void *cb_arg, struct spdk_blob *blb, int bserrno) g_bserrno = bserrno; } +static void +blob_op_with_handle_complete2(void *cb_arg, struct spdk_blob *blob, int bserrno) +{ + if (g_blob == NULL) { + g_blob = blob; + g_bserrno = bserrno; + } else { + g_blob2 = blob; + g_bserrno2 = bserrno; + } +} + static void ut_bs_reload(struct spdk_blob_store **bs, struct spdk_bs_opts *opts) { @@ -322,8 +334,32 @@ blob_open(void) CU_ASSERT(g_bserrno == 0); CU_ASSERT(g_blob != NULL); blob = g_blob; + spdk_blob_close(blob, blob_op_complete, NULL); + poll_threads(); + CU_ASSERT(g_bserrno == 0); - ut_blob_close_and_delete(bs, blob); + /* Try to open file twice in succession. This should return the same + * blob object. + */ + g_blob = NULL; + g_blob2 = NULL; + g_bserrno = -1; + g_bserrno2 = -1; + spdk_bs_open_blob(bs, blobid, blob_op_with_handle_complete2, NULL); + spdk_bs_open_blob(bs, blobid, blob_op_with_handle_complete2, NULL); + poll_threads(); + CU_ASSERT(g_bserrno == 0); + CU_ASSERT(g_bserrno2 == 0); + CU_ASSERT(g_blob != NULL); + CU_ASSERT(g_blob2 != NULL); + CU_ASSERT(g_blob == g_blob2); + + g_bserrno = -1; + spdk_blob_close(g_blob, blob_op_complete, NULL); + poll_threads(); + CU_ASSERT(g_bserrno == 0); + + ut_blob_close_and_delete(bs, g_blob); } static void