Skip to contentSkip to conversation

CFQ: mark must_alloc flag when a cfqq doesn't allocate any request

PATCH In review Gui Jianfeng 2 participants 6 messages Aug 7, 2009 – Aug 18, 2009 Block layer

lore
Share & tools
lore

Gui Jianfeng

author
#0 lore raw

PATCH 1/1 CFQ: mark must_alloc flag when a cfqq doesn't allocate any request

Hi Jens,

CFQ judges whether must_alloc flag is set, but never marks this flag. This patch set must_alloc flag when there is no request allocated by a cfqq. This change ensures cfqq will get at least one request even if the queue is full.
Hope it helps.

Signed-off-by Gui Jianfeng <[email protected]>
---
 block/cfq-iosched.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
block/cfq-iosched.c +3 -0
index fd7080e..531a404 100644
@@ -2279,6 +2279,9 @@ static void cfq_put_request(struct request *rq)
 		BUG_ON(!cfqq->allocated[rw]);
 		cfqq->allocated[rw]--;
 
+		if (!(cfqq->allocated[0] + cfqq->allocated[1]))
+			cfq_mark_cfqq_must_alloc(cfqq);
+
 		put_io_context(RQ_CIC(rq)->ioc);
 
 		rq->elevator_private = NULL;
Signature · 1 line
1.5.4.rc3
#1 lore raw
Quoted 7 lines · Gui Jianfeng

Hi Jens,

CFQ judges whether must_alloc flag is set, but never marks this flag. This patch set must_alloc flag when there is no request allocated by a cfqq. This change ensures cfqq will get at least one request even if the queue is full.
Hope it helps.

The flag became redundant with the fix to not fail in set_request(). I don't think your below patch is a very good idea, it would basically mark any queue that has previously had a request allocated as a must alloc queue. It would make more sense to mark an idling queue as must alloc, since it's critical that this queue is allowed to allocate a request once we idle a queue on its behalf. But we already check for the wait_request flag in ->may_queue, so that part is already covered. So I'd suggest we just delete any reference to must_alloc instead.

Quoted 24 lines
Signed-off-by Gui Jianfeng <[email protected]>
---
 block/cfq-iosched.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
block/cfq-iosched.c +3 -0
index fd7080e..531a404 100644
@@ -2279,6 +2279,9 @@ static void cfq_put_request(struct request *rq)
 		BUG_ON(!cfqq->allocated[rw]);
 		cfqq->allocated[rw]--;
 
+		if (!(cfqq->allocated[0] + cfqq->allocated[1]))
+			cfq_mark_cfqq_must_alloc(cfqq);
+
 		put_io_context(RQ_CIC(rq)->ioc);
 
 		rq->elevator_private = NULL;
Signature · 1 line
1.5.4.rc3
Signature · 1 line
Jens Axboe
author
#2 lore raw

PATCH 1/1 CFQ: get rid of must_alloc flag

In reply to Gui Jianfeng

Get rid of must_alloc flag since it's redundant now.

Signed-off-by Gui Jianfeng <[email protected]>
---
 block/cfq-iosched.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)
block/cfq-iosched.c +1 -5
index fd7080e..f5770ad 100644
@@ -191,7 +191,6 @@ enum cfqq_state_flags {
 	CFQ_CFQQ_FLAG_on_rr = 0,	/* on round-robin busy list */
 	CFQ_CFQQ_FLAG_wait_request,	/* waiting for a request */
 	CFQ_CFQQ_FLAG_must_dispatch,	/* must be allowed a dispatch */
-	CFQ_CFQQ_FLAG_must_alloc,	/* must be allowed rq alloc */
 	CFQ_CFQQ_FLAG_must_alloc_slice,	/* per-slice must_alloc flag */
 	CFQ_CFQQ_FLAG_fifo_expire,	/* FIFO checked in this slice */
 	CFQ_CFQQ_FLAG_idle_window,	/* slice idling enabled */

@@ -218,7 +217,6 @@ static inline int cfq_cfqq_##name(const struct cfq_queue *cfqq)		\
 CFQ_CFQQ_FNS(on_rr);
 CFQ_CFQQ_FNS(wait_request);
 CFQ_CFQQ_FNS(must_dispatch);
-CFQ_CFQQ_FNS(must_alloc);
 CFQ_CFQQ_FNS(must_alloc_slice);
 CFQ_CFQQ_FNS(fifo_expire);
 CFQ_CFQQ_FNS(idle_window);

@@ -2229,8 +2227,7 @@ static void cfq_prio_boost(struct cfq_queue *cfqq)
 
 static inline int __cfq_may_queue(struct cfq_queue *cfqq)
 {
-	if ((cfq_cfqq_wait_request(cfqq) || cfq_cfqq_must_alloc(cfqq)) &&
-	    !cfq_cfqq_must_alloc_slice(cfqq)) {
+	if ((cfq_cfqq_wait_request(cfqq) && !cfq_cfqq_must_alloc_slice(cfqq)) {
 		cfq_mark_cfqq_must_alloc_slice(cfqq);
 		return ELV_MQUEUE_MUST;
 	}

@@ -2317,7 +2314,6 @@ cfq_set_request(struct request_queue *q, struct request *rq, gfp_t gfp_mask)
 	}
 
 	cfqq->allocated[rw]++;
-	cfq_clear_cfqq_must_alloc(cfqq);
 	atomic_inc(&cfqq->ref);
 
 	spin_unlock_irqrestore(q->queue_lock, flags);
Signature · 1 line
1.5.4.rc3
#5 lore raw

Re: [PATCH] CFQ: get rid of must_alloc flag

Quoted 10 lines · Gui Jianfeng
Quoted 7 lines · Jens Axboe
Quoted 1 line · Gui Jianfeng

Get rid of must_alloc flag since it's redundant now.

I already queued an identical patch in for-2.6.32 a week ago:

http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=87783e4b0213b6b000108f17e2cd91045bee56f7;hp=37571a15498816c1a35f1dd2b68f7ddac5c50592

Ahh.. see, sorry for bothering.

Don't apologize, thanks for noticing!

Signature · 1 line
Jens Axboe