Commit dc371729 authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

bdev: handle unlock v. lock race



When we unlock a range, we remove the range from the
locked bdev list before doing the for_each_channel
iteration to remove the range from each channel.

But at the same time, right after removing from the
locked list, a new lock on that range could start.
In that case, we also do a for_each_channel to add
the range to each channel, and that will race with
the for_each_channel remove.  When the lock start
wins, it finds the range already in the channel,
but doesn't set the owner_range which results in
a seg fault when the for_each_channel completes.

The fix is actually rather simple.  We just add the
locked_ctx to the comparison when checking if the
range is already in the channel.  If the locked_ctx
matches, then we know it was added as part of
initializing a new channel.  If it doesn't, then
we create a new range object pointing to the new
locked_ctx.  The first one will get removed when
the remove for_each_channel catches up.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I94f8b20376dd437f404add35744d42fc148303ff

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482620


Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarPaul Luse <paul.e.luse@intel.com>
Reviewed-by: default avatarMaciej Szwed <maciej.szwed@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent da11a464
Loading
Loading
Loading
Loading
+6 −2
Original line number Diff line number Diff line
@@ -2532,6 +2532,7 @@ bdev_channel_create(void *io_device, void *ctx_buf)
		}
		new_range->length = range->length;
		new_range->offset = range->offset;
		new_range->locked_ctx = range->locked_ctx;
		TAILQ_INSERT_TAIL(&ch->locked_ranges, new_range, tailq);
	}

@@ -6110,7 +6111,9 @@ bdev_lock_lba_range_get_channel(struct spdk_io_channel_iter *i)
	struct lba_range *range;

	TAILQ_FOREACH(range, &ch->locked_ranges, tailq) {
		if (range->length == ctx->range.length && range->offset == ctx->range.offset) {
		if (range->length == ctx->range.length &&
		    range->offset == ctx->range.offset &&
		    range->locked_ctx == ctx->range.locked_ctx) {
			/* This range already exists on this channel, so don't add
			 * it again.  This can happen when a new channel is created
			 * while the for_each_channel operation is in progress.
@@ -6260,7 +6263,8 @@ bdev_unlock_lba_range_get_channel(struct spdk_io_channel_iter *i)

	TAILQ_FOREACH(range, &ch->locked_ranges, tailq) {
		if (ctx->range.offset == range->offset &&
		    ctx->range.length == range->length) {
		    ctx->range.length == range->length &&
		    ctx->range.locked_ctx == range->locked_ctx) {
			TAILQ_REMOVE(&ch->locked_ranges, range, tailq);
			free(range);
			break;