Date: Mon, 14 Feb 2011 08:09:03 +0000 (UTC) From: Luigi Rizzo <luigi@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r218675 - head/sys/geom/sched Message-ID: <201102140809.p1E8937L084414@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: luigi Date: Mon Feb 14 08:09:02 2011 New Revision: 218675 URL: http://svn.freebsd.org/changeset/base/218675 Log: Correct a subtle bug in the 'gsched_rr' disk scheduler. The algorithm is supposed to work as follows: in order to prevent starvation, when a new client starts being served we record the start time and reset the counter of bytes served. We then switch to a new client after a certain amount of time or bytes, even if the current one still has pending requests. To avoid charging a new client the time of the first seek, we start counting time when the first request is served. Unfortunately a bug in the previous version of the code failed to set the start time in certain cases, resulting in some processes exceeding their timeslice. The fix (in this patch) is trivial, though it took a while to find out and replicate the bug. Thanks to Tommaso Caprai for investigating and fixing the problem. Submitted by: Tommaso Caprai MFC after: 1 week Modified: head/sys/geom/sched/gs_rr.c Modified: head/sys/geom/sched/gs_rr.c ============================================================================== --- head/sys/geom/sched/gs_rr.c Mon Feb 14 06:06:20 2011 (r218674) +++ head/sys/geom/sched/gs_rr.c Mon Feb 14 08:09:02 2011 (r218675) @@ -71,6 +71,7 @@ enum g_rr_state { /* possible queue flags */ enum g_rr_flags { + /* G_FLAG_COMPLETED means that the field q_slice_end is valid. */ G_FLAG_COMPLETED = 1, /* Completed a req. in the current budget. */ }; @@ -87,7 +88,7 @@ struct g_rr_queue { enum g_rr_state q_status; unsigned int q_service; /* service received so far */ - int q_slice_end; /* actual slice end in ticks */ + int q_slice_end; /* actual slice end time, in ticks */ enum g_rr_flags q_flags; /* queue flags */ struct bio_queue_head q_bioq; @@ -638,14 +639,25 @@ g_rr_done(void *data, struct bio *bp) sc->sc_in_flight--; qp = bp->bio_caller1; - if (qp == sc->sc_active && qp->q_status == G_QUEUE_BUSY) { - if (!(qp->q_flags & G_FLAG_COMPLETED)) { - qp->q_flags |= G_FLAG_COMPLETED; - /* in case we want to make the slice adaptive */ - qp->q_slice_duration = get_bounded(&me.quantum_ms, 2); - qp->q_slice_end = ticks + qp->q_slice_duration; - } + /* + * When the first request for this queue completes, update the + * duration and end of the slice. We do not do it when the + * slice starts to avoid charging to the queue the time for + * the first seek. + */ + if (!(qp->q_flags & G_FLAG_COMPLETED)) { + qp->q_flags |= G_FLAG_COMPLETED; + /* + * recompute the slice duration, in case we want + * to make it adaptive. This is not used right now. + * XXX should we do the same for q_quantum and q_wait_ticks ? + */ + qp->q_slice_duration = get_bounded(&me.quantum_ms, 2); + qp->q_slice_end = ticks + qp->q_slice_duration; + } + + if (qp == sc->sc_active && qp->q_status == G_QUEUE_BUSY) { /* The queue is trying anticipation, start the timer. */ qp->q_status = G_QUEUE_IDLING; /* may make this adaptive */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201102140809.p1E8937L084414>