Re: [PATCH 7/8] wbt: add general throttling mechanism

[Jens Axboe]

On 04/27/2016 06:06 AM, xiakaixu wrote:
> > +void __wbt_done(struct rq_wb *rwb)
> > +{
> > +	int inflight, limit = rwb->wb_normal;
> > +
> > +	/*
> > +	 * If the device does write back caching, drop further down
> > +	 * before we wake people up.
> > +	 */
> > +	if (rwb->wc && !atomic_read(&rwb->bdi->wb.dirty_sleeping))
> > +		limit = 0;
> > +	else
> > +		limit = rwb->wb_normal;
> > +
> > +	/*
> > +	 * Don't wake anyone up if we are above the normal limit. If
> > +	 * throttling got disabled (limit == 0) with waiters, ensure
> > +	 * that we wake them up.
> > +	 */
> > +	inflight = atomic_dec_return(&rwb->inflight);
> > +	if (limit && inflight >= limit) {
> > +		if (!rwb->wb_max)
> > +			wake_up_all(&rwb->wait);
> > +		return;
> > +	}
> > +
> Hi Jens,
>
> Just a little confused about this. The rwb->wb_max can't be 0 if the variable
> 'limit' does not equal to 0. So the if (!rwb->wb_max) branch maybe does not
> make sense.

You are right, it doesn't make a lot of sense. I think it suffers from 
code shuffling. How about the attached? The important part is that we 
wake up waiters, if wbt got disabled while we had tracked IO in flight.

--
Jens Axboe

diff --git a/lib/wbt.c b/lib/wbt.c
index 650da911f24f..a6b80c135510 100644
--- a/lib/wbt.c
+++ b/lib/wbt.c
@@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb)
 	else
 		limit = rwb->wb_normal;
 
+	inflight = atomic_dec_return(&rwb->inflight);
+
 	/*
-	 * Don't wake anyone up if we are above the normal limit. If
-	 * throttling got disabled (limit == 0) with waiters, ensure
-	 * that we wake them up.
+	 * wbt got disabled with IO in flight. Wake up any potential
+	 * waiters, we don't have to do more than that.
 	 */
-	inflight = atomic_dec_return(&rwb->inflight);
-	if (limit && inflight >= limit) {
-		if (!rwb->wb_max)
-			wake_up_all(&rwb->wait);
+	if (!rwb_enabled(rwb)) {
+		wake_up_all(&rwb->wait);
 		return;
 	}
 
+	/*
+	 * Don't wake anyone up if we are above the normal limit.
+	 */
+	if (inflight >= limit)
+		return;
+
 	if (waitqueue_active(&rwb->wait)) {
 		int diff = limit - inflight;