Commit 140eaaa0 authored by Jim Harris's avatar Jim Harris
Browse files

bdev: submit queued IO after disabling QoS 



There's no reason to abort IO that have been queued
due to QoS limits, when QoS is switched from enabled
to disabled.  Submit them to the bdev instead.

Fixes issue #357.

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

Reviewed-on: https://review.gerrithub.io/418128


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarSeth Howell <seth.howell5141@gmail.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent 436c0c18

lib/bdev/bdev.c

+19 −1
Original line number Diff line number Diff line
@@ -3238,6 +3238,7 @@ _spdk_bdev_disable_qos_done(void *cb_arg) 
{

	struct set_qos_limit_ctx *ctx = cb_arg;

	struct spdk_bdev *bdev = ctx->bdev;

	struct spdk_bdev_io *bdev_io;

	struct spdk_bdev_qos *qos;



	pthread_mutex_lock(&bdev->internal.mutex);
@@ -3245,7 +3246,24 @@ _spdk_bdev_disable_qos_done(void *cb_arg) 
	bdev->internal.qos = NULL;

	pthread_mutex_unlock(&bdev->internal.mutex);



	_spdk_bdev_abort_queued_io(&qos->queued, qos->ch);

	while (!TAILQ_EMPTY(&qos->queued)) {

		/* Send queued I/O back to their original thread for resubmission. */

		bdev_io = TAILQ_FIRST(&qos->queued);

		TAILQ_REMOVE(&qos->queued, bdev_io, internal.link);



		if (bdev_io->internal.io_submit_ch) {

			/*

			 * Channel was changed when sending it to the QoS thread - change it back

			 *  before sending it back to the original thread.

			 */

			bdev_io->internal.ch = bdev_io->internal.io_submit_ch;

			bdev_io->internal.io_submit_ch = NULL;

		}



		spdk_thread_send_msg(spdk_io_channel_get_thread(bdev_io->internal.ch->channel),

				     _spdk_bdev_io_submit, bdev_io);

	}



	spdk_put_io_channel(spdk_io_channel_from_ctx(qos->ch));

	spdk_poller_unregister(&qos->poller);

test/unit/lib/bdev/mt/bdev.c/bdev_ut.c

+50 −1
Original line number Diff line number Diff line
@@ -1061,7 +1061,8 @@ qos_dynamic_enable(void) 
	struct spdk_io_channel *io_ch[2];

	struct spdk_bdev_channel *bdev_ch[2];

	struct spdk_bdev *bdev;

	int status, second_status;

	enum spdk_bdev_io_status bdev_io_status[2];

	int status, second_status, rc, i;



	setup_test();

	reset_time();
@@ -1091,6 +1092,38 @@ qos_dynamic_enable(void) 
	CU_ASSERT((bdev_ch[0]->flags & BDEV_CH_QOS_ENABLED) != 0);

	CU_ASSERT((bdev_ch[1]->flags & BDEV_CH_QOS_ENABLED) != 0);



	/*

	 * Submit and complete 10 I/O to fill the QoS allotment for this timeslice.

	 * Additional I/O will then be queued.

	 */

	set_thread(0);

	for (i = 0; i < 10; i++) {

		bdev_io_status[0] = SPDK_BDEV_IO_STATUS_PENDING;

		rc = spdk_bdev_read_blocks(g_desc, io_ch[0], NULL, 0, 1, io_during_io_done, &bdev_io_status[0]);

		CU_ASSERT(rc == 0);

		CU_ASSERT(bdev_io_status[0] == SPDK_BDEV_IO_STATUS_PENDING);

		poll_thread(0);

		stub_complete_io(g_bdev.io_target, 0);

		CU_ASSERT(bdev_io_status[0] == SPDK_BDEV_IO_STATUS_SUCCESS);

	}



	/*

	 * Send two more I/O.  These I/O will be queued since the current timeslice allotment has been

	 * filled already.  We want to test that when QoS is disabled that these two I/O:

	 *  1) are not aborted

	 *  2) are sent back to their original thread for resubmission

	 */

	bdev_io_status[0] = SPDK_BDEV_IO_STATUS_PENDING;

	rc = spdk_bdev_read_blocks(g_desc, io_ch[0], NULL, 0, 1, io_during_io_done, &bdev_io_status[0]);

	CU_ASSERT(rc == 0);

	CU_ASSERT(bdev_io_status[0] == SPDK_BDEV_IO_STATUS_PENDING);

	set_thread(1);

	bdev_io_status[1] = SPDK_BDEV_IO_STATUS_PENDING;

	rc = spdk_bdev_read_blocks(g_desc, io_ch[1], NULL, 0, 1, io_during_io_done, &bdev_io_status[1]);

	CU_ASSERT(rc == 0);

	CU_ASSERT(bdev_io_status[1] == SPDK_BDEV_IO_STATUS_PENDING);

	poll_threads();



	/* Disable QoS */

	status = -1;

	spdk_bdev_set_qos_limit_iops(bdev, 0, qos_dynamic_enable_done, &status);
@@ -1099,6 +1132,22 @@ qos_dynamic_enable(void) 
	CU_ASSERT((bdev_ch[0]->flags & BDEV_CH_QOS_ENABLED) == 0);

	CU_ASSERT((bdev_ch[1]->flags & BDEV_CH_QOS_ENABLED) == 0);



	/*

	 * All I/O should have been resubmitted back on their original thread.  Complete

	 *  all I/O on thread 0, and ensure that only the thread 0 I/O was completed.

	 */

	set_thread(0);

	stub_complete_io(g_bdev.io_target, 0);

	poll_threads();

	CU_ASSERT(bdev_io_status[0] == SPDK_BDEV_IO_STATUS_SUCCESS);

	CU_ASSERT(bdev_io_status[1] == SPDK_BDEV_IO_STATUS_PENDING);



	/* Now complete all I/O on thread 1 and ensure the thread 1 I/O was completed. */

	set_thread(1);

	stub_complete_io(g_bdev.io_target, 0);

	poll_threads();

	CU_ASSERT(bdev_io_status[1] == SPDK_BDEV_IO_STATUS_SUCCESS);



	/* Disable QoS again */

	status = -1;

	spdk_bdev_set_qos_limit_iops(bdev, 0, qos_dynamic_enable_done, &status);