Commit 86d77e2e authored by Jim Harris's avatar Jim Harris
Browse files

bdev: account for missed qos timeslice timeouts



There could be cases (especially in virtualized and/or test
environments) where we could accumulate significant skew in
the timeslice frequency.  Rather than depend on the application
framework to try to guarantee the rate of timeslice poller
callbacks, keep track internally of the last time the poller
was invoked.  If/when we accumulate and detect skew equivalent
to one or more timeslices, increase the allowed IO and bandwidth
of the next timeslice to accomodate.

Since bdev poller now calls spdk_get_ticks() to do accounting,
this patch also fixes up the increment_time() unit test function
and the test env layer to properly increment the fake TSC.

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

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


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarGangCao <gang.cao@intel.com>
parent 526fa366
Loading
Loading
Loading
Loading
+26 −4
Original line number Diff line number Diff line
@@ -132,6 +132,12 @@ struct spdk_bdev_qos {
	/** Queue of I/O waiting to be issued. */
	bdev_io_tailq_t queued;

	/** Size of a timeslice in tsc ticks. */
	uint64_t timeslice_size;

	/** Timestamp of start of last timeslice. */
	uint64_t last_timeslice;

	/** Maximum allowed IOs to be issued in one timeslice (e.g., 1ms) and
	 *  only valid for the master channel which manages the outstanding IOs. */
	uint64_t max_ios_per_timeslice;
@@ -1384,10 +1390,19 @@ static int
spdk_bdev_channel_poll_qos(void *arg)
{
	struct spdk_bdev_qos *qos = arg;
	uint64_t now = spdk_get_ticks();

	/* Reset for next round of rate limiting */
	qos->io_remaining_this_timeslice = qos->max_ios_per_timeslice;
	if (now < (qos->last_timeslice + qos->timeslice_size)) {
		/* We received our callback earlier than expected - return
		 *  immediately and wait to do accounting until at least one
		 *  timeslice has actually expired.  This should never happen
		 *  with a well-behaved timer implementation.
		 */
		return 0;
	}

	/* Reset for next round of rate limiting */
	qos->io_remaining_this_timeslice = 0;
	/* We may have allowed the bytes to slightly overrun in the last timeslice.
	 * byte_remaining_this_timeslice is signed, so if it's negative here, we'll
	 * account for the overrun so that the next timeslice will be appropriately
@@ -1396,7 +1411,12 @@ spdk_bdev_channel_poll_qos(void *arg)
	if (qos->byte_remaining_this_timeslice > 0) {
		qos->byte_remaining_this_timeslice = 0;
	}

	while (now >= (qos->last_timeslice + qos->timeslice_size)) {
		qos->last_timeslice += qos->timeslice_size;
		qos->io_remaining_this_timeslice += qos->max_ios_per_timeslice;
		qos->byte_remaining_this_timeslice += qos->max_byte_per_timeslice;
	}

	_spdk_bdev_qos_io_submit(qos->ch);

@@ -1458,7 +1478,9 @@ _spdk_bdev_enable_qos(struct spdk_bdev *bdev, struct spdk_bdev_channel *ch)
			spdk_bdev_qos_update_max_quota_per_timeslice(qos);
			qos->io_remaining_this_timeslice = qos->max_ios_per_timeslice;
			qos->byte_remaining_this_timeslice = qos->max_byte_per_timeslice;

			qos->timeslice_size =
				SPDK_BDEV_QOS_TIMESLICE_IN_USEC * spdk_get_ticks_hz() / SPDK_SEC_TO_USEC;
			qos->last_timeslice = spdk_get_ticks();
			qos->poller = spdk_poller_register(spdk_bdev_channel_poll_qos,
							   qos,
							   SPDK_BDEV_QOS_TIMESLICE_IN_USEC);
+1 −0
Original line number Diff line number Diff line
@@ -378,6 +378,7 @@ spdk_get_ticks_hz(void)
void
spdk_delay_us(unsigned int us)
{
	/* spdk_get_ticks_hz is 1000000, meaning 1 tick per us. */
	ut_spdk_get_ticks += us;
}

+1 −0
Original line number Diff line number Diff line
@@ -170,6 +170,7 @@ void
increment_time(uint64_t time_in_us)
{
	g_current_time_in_us += time_in_us;
	spdk_delay_us(time_in_us);
}

static void