cfq-prio #2

Hi,

drivers/block/ll_rw_blk.c +4 -0

@@ -1553,6 +1559,10 @@
 	struct io_context *ioc = get_io_context(gfp_mask);
 
 	spin_lock_irq(q->queue_lock);
+
+	if (!elv_may_queue(q, rw))
+		goto out_lock;
+
 	if (rl->count[rw]+1 >= q->nr_requests) {
 		/*
 		 * The queue will fill after this allocation, so set it as

Quoted 3 lines · Jens Axboe

Hi,

Hi Jens

Quoted 15 lines

drivers/block/ll_rw_blk.c +4 -0

@@ -1553,6 +1559,10 @@
 	struct io_context *ioc = get_io_context(gfp_mask);
 
 	spin_lock_irq(q->queue_lock);
+
+	if (!elv_may_queue(q, rw))
+		goto out_lock;
+
 	if (rl->count[rw]+1 >= q->nr_requests) {
 		/*
 		 * The queue will fill after this allocation, so set it as

@@ -1566,15 +1576,12 @@
 		}
 	}
 

drivers/block/ll_rw_blk.c line 1585

Quoted 3 lines

drivers/block/ll_rw_blk.c +0 -2

-	if (blk_queue_full(q, rw)
-			&& !ioc_batching(ioc) && !elv_may_queue(q, rw)) {
 

I know I hijacked elv_may_queue from you... any chance we could seperate these so our schedulers can live in peace? ;)

Maybe my version should be called elv_force_queue?

Quoted 26 lines · Nick Piggin

Quoted 4 lines · Jens Axboe

Hi,

Hi Jens

Quoted 15 lines

drivers/block/ll_rw_blk.c +4 -0

@@ -1553,6 +1559,10 @@
 	struct io_context *ioc = get_io_context(gfp_mask);
 
 	spin_lock_irq(q->queue_lock);
+
+	if (!elv_may_queue(q, rw))
+		goto out_lock;
+
 	if (rl->count[rw]+1 >= q->nr_requests) {
 		/*
 		 * The queue will fill after this allocation, so set it as

@@ -1566,15 +1576,12 @@
 		}
 	}
 

Quoted 7 lines

drivers/block/ll_rw_blk.c

Quoted 4 lines

drivers/block/ll_rw_blk.c +0 -2

-	if (blk_queue_full(q, rw)
-			&& !ioc_batching(ioc) && !elv_may_queue(q, rw)) {
 
 

I know I hijacked elv_may_queue from you... any chance we could seperate these so our schedulers can live in peace? ;)

IOW, you completely broke it! I'm just changing it back to the original. When was this done, btw? Just discovered it when updating the patch. Pretty annoying...

Quoted 3 lines

Maybe my version should be called elv_force_queue?

I just hate to see more of these, really. The original idea for may_queue was just that, may this process queue io or not. We can make it return something else, though, to indicate whether the process must be able to queue. Is it really needed?

Quoted 36 lines · Jens Axboe

Quoted 33 lines · Nick Piggin

Quoted 4 lines · Jens Axboe

Hi,

Hi Jens

Quoted 12 lines

drivers/block/ll_rw_blk.c +4 -0

@@ -1553,6 +1559,10 @@
 	struct io_context *ioc = get_io_context(gfp_mask);
 
 	spin_lock_irq(q->queue_lock);
+
+	if (!elv_may_queue(q, rw))
+		goto out_lock;
+
 	if (rl->count[rw]+1 >= q->nr_requests) {
 		/*
 		 * The queue will fill after this allocation, so set it as

drivers/block/ll_rw_blk.c line 1580

Quoted 7 lines

drivers/block/ll_rw_blk.c +0 -2

@@ -1566,15 +1576,12 @@
 		}
 	}
 
-	if (blk_queue_full(q, rw)
-			&& !ioc_batching(ioc) && !elv_may_queue(q, rw)) {
 
 

I know I hijacked elv_may_queue from you... any chance we could seperate these so our schedulers can live in peace? ;)

Quoted 4 lines

IOW, you completely broke it! I'm just changing it back to the original. When was this done, btw? Just discovered it when updating the patch. Pretty annoying...

You acked the change actually :P
I guess it was done in mainline when AS was merged.

Quoted 8 lines

Quoted 2 lines

Maybe my version should be called elv_force_queue?

I just hate to see more of these, really. The original idea for may_queue was just that, may this process queue io or not. We can make it return something else, though, to indicate whether the process must be able to queue. Is it really needed?

Its quite important. If the queue is full, and AS is waiting for a process to submit a request, its got a long wait.

Maybe a lower limit for per process nr_requests. Ie. you may queue if this queue has less than 128 requests _or_ you have less than 8 requests outstanding. This would solve my problem. It would also give you a much more appropriate scaling for server workloads, I think. Still, thats quite a change in behaviour (simple to code though).

Quoted 4 lines · Nick Piggin

You acked the change actually :P
I guess it was done in mainline when AS was merged.

Probably missed the semantic change of may_queue.

Quoted 13 lines

Quoted 10 lines

Quoted 4 lines

Maybe my version should be called elv_force_queue?

I just hate to see more of these, really. The original idea for may_queue was just that, may this process queue io or not. We can make it return something else, though, to indicate whether the process must be able to queue. Is it really needed?

Its quite important. If the queue is full, and AS is waiting for a process to submit a request, its got a long wait.

Quoted 6 lines

Maybe a lower limit for per process nr_requests. Ie. you may queue if this queue has less than 128 requests _or_ you have less than 8 requests outstanding. This would solve my problem. It would also give you a much more appropriate scaling for server workloads, I think. Still, thats quite a change in behaviour (simple to code though).

That basically belongs inside your may_queue for the io scheduler, imo.

Quoted 3 lines · Jens Axboe

Quoted 1 line · Nick Piggin

Quoted 6 lines

Quoted 3 lines

You acked the change actually :P
I guess it was done in mainline when AS was merged.

Probably missed the semantic change of may_queue.

Anyway I won't bother digging up the email, its been done now.

Quoted 20 lines

Quoted 19 lines

Quoted 9 lines

Quoted 3 lines

Maybe my version should be called elv_force_queue?

I just hate to see more of these, really. The original idea for may_queue was just that, may this process queue io or not. We can make it return something else, though, to indicate whether the process must be able to queue. Is it really needed?

Its quite important. If the queue is full, and AS is waiting for a process to submit a request, its got a long wait.

Maybe a lower limit for per process nr_requests. Ie. you may queue if this queue has less than 128 requests _or_ you have less than 8 requests outstanding. This would solve my problem. It would also give you a much more
appropriate scaling for server workloads, I think. Still, thats quite a change in behaviour (simple to code though).

Quoted 2 lines

That basically belongs inside your may_queue for the io scheduler, imo.

You can force it to disallow the request, but you can't force it to allow one (depending on a successful memory allocation, of course).

Quoted 21 lines · Nick Piggin

Quoted 17 lines · Jens Axboe

Quoted 11 lines · Nick Piggin

Quoted 1 line

Its quite important. If the queue is full, and AS is waiting for a process to submit a request, its got a long wait.

Maybe a lower limit for per process nr_requests. Ie. you may queue if this queue has less than 128 requests _or_ you have less than 8 requests outstanding. This would solve my problem. It would also give you a much more appropriate scaling for server workloads, I think. Still, thats quite a change in behaviour (simple to code though).

That basically belongs inside your may_queue for the io scheduler, imo.

Quoted 3 lines

You can force it to disallow the request, but you can't force it to allow one (depending on a successful memory allocation, of course).

Well that's back two mails then, make may_queue return whether you must queue, may queue, or can't queue.

Quoted 20 lines

Quoted 19 lines

Quoted 13 lines

Quoted 4 lines

Its quite important. If the queue is full, and AS is waiting for a process
to submit a request, its got a long wait.

Quoted 9 lines

Maybe a lower limit for per process nr_requests. Ie. you may queue if this
queue has less than 128 requests _or_ you have less than 8 requests outstanding. This would solve my problem. It would also give you a much more
appropriate scaling for server workloads, I think. Still, thats quite a change in behaviour (simple to code though).

Quoted 3 lines

That basically belongs inside your may_queue for the io scheduler, imo.

You can force it to disallow the request, but you can't force it to allow one (depending on a successful memory allocation, of course).

Quoted 3 lines

Well that's back two mails then, make may_queue return whether you must queue, may queue, or can't queue.

Yep, sounds good. I'll make a patch for it for 2.6.x > 0 sometime unless you beat me to it.