Skip to contentSkip to conversation

modular io schedulers with online switching, #2

PATCH In review Jens Axboe 2 participants 4 messages Sep 17, 2004 – Sep 18, 2004

Subsystems (3)
Block layer s390 s390 DASD driver
lore
Share & tools
lore

Jens Axboe

author
#0 lore raw

PATCH 1/1 modular io schedulers with online switching, #2

Hi,

Changes since yesterday:

- Don't allow AS to be unloaded, it has open io contexts attached to processes so we can't remove the functions that will be called on task exit. AS could be made unloadable by iterating each task in the system, calling exit_io_context() on tsk->io_context and clearing it. AS is still online switchable, you just cannot remove the module after it has been loaded.

- Add module license to each scheduler

- Move the chosen_elevator init and setup into elevator.c. Also sanity check given elevator= boot parameter and fall back to a default if it's invalid. This also gets rid of the ugly 'printed' crap in blk_init_queue()

===== drivers/block/Kconfig.iosched 1.3 vs edited =====

drivers/block/Kconfig.iosched +4 -4
@@ -1,5 +1,5 @@
 config IOSCHED_NOOP
-	bool "No-op I/O scheduler" if EMBEDDED
+	tristate "No-op I/O scheduler"
 	default y
 	---help---
 	  The no-op I/O scheduler is a minimal scheduler that does basic merging

@@ -9,7 +9,7 @@
 	  the kernel.
 
 config IOSCHED_AS
-	bool "Anticipatory I/O scheduler" if EMBEDDED
+	tristate "Anticipatory I/O scheduler"
 	default y
 	---help---
 	  The anticipatory I/O scheduler is the default disk scheduler. It is

@@ -18,7 +18,7 @@
 	  slower in some cases especially some database loads.
 
 config IOSCHED_DEADLINE
-	bool "Deadline I/O scheduler" if EMBEDDED
+	tristate "Deadline I/O scheduler"
 	default y
 	---help---
 	  The deadline I/O scheduler is simple and compact, and is often as

@@ -28,7 +28,7 @@
 	  anticipatory I/O scheduler and so is a good choice.
 
 config IOSCHED_CFQ
-	bool "CFQ I/O scheduler" if EMBEDDED
+	tristate "CFQ I/O scheduler"
 	default y
 	---help---
 	  The CFQ I/O scheduler tries to distribute bandwidth equally

===== drivers/block/as-iosched.c 1.38 vs edited =====

drivers/block/as-iosched.c +67 -42

211 lines not on this page · raw message

===== drivers/block/cfq-iosched.c 1.8 vs edited =====

drivers/block/cfq-iosched.c +74 -39

211 lines not on this page · raw message

===== drivers/block/deadline-iosched.c 1.29 vs edited =====

drivers/block/deadline-iosched.c +53 -35

173 lines not on this page · raw message

===== drivers/block/elevator.c 1.58 vs edited =====

drivers/block/elevator.c +259 -51

475 lines not on this page · raw message

===== drivers/block/ll_rw_blk.c 1.271 vs edited =====

drivers/block/ll_rw_blk.c +73 -50

231 lines not on this page · raw message

===== drivers/block/noop-iosched.c 1.3 vs edited =====

drivers/block/noop-iosched.c +26 -7
@@ -83,12 +83,31 @@
 	return NULL;
 }
 
-elevator_t elevator_noop = {
-	.elevator_merge_fn		= elevator_noop_merge,
-	.elevator_merge_req_fn		= elevator_noop_merge_requests,
-	.elevator_next_req_fn		= elevator_noop_next_request,
-	.elevator_add_req_fn		= elevator_noop_add_request,
-	.elevator_name			= "noop",
+static struct elevator_type elevator_noop = {
+	.ops = {
+		.elevator_merge_fn		= elevator_noop_merge,
+		.elevator_merge_req_fn		= elevator_noop_merge_requests,
+		.elevator_next_req_fn		= elevator_noop_next_request,
+		.elevator_add_req_fn		= elevator_noop_add_request,
+	},
+	.elevator_name = "noop",
+	.elevator_owner = THIS_MODULE,
 };
 
-EXPORT_SYMBOL(elevator_noop);
+int noop_init(void)
+{
+	return elv_register(&elevator_noop);
+}
+
+void noop_exit(void)
+{
+	elv_unregister(&elevator_noop);
+}
+
+module_init(noop_init);
+module_exit(noop_exit);
+
+
+MODULE_AUTHOR("Jens Axboe");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("No-op IO scheduler");

===== drivers/s390/block/dasd.c 1.77 vs edited =====

drivers/s390/block/dasd.c +2 -2
@@ -1595,8 +1595,8 @@
 
 	device->request_queue->queuedata = device;
 #if 0
-	elevator_exit(device->request_queue);
-	rc = elevator_init(device->request_queue, &elevator_noop);
+	elevator_exit(device->request_queue->elevator);
+	rc = elevator_init(device->request_queue, "noop");
 	if (rc) {
 		blk_cleanup_queue(device->request_queue);
 		return rc;

===== drivers/s390/char/tape_block.c 1.12 vs edited =====

drivers/s390/char/tape_block.c +2 -2
@@ -225,8 +225,8 @@
 	if (!blkdat->request_queue)
 		return -ENOMEM;
 
-	elevator_exit(blkdat->request_queue);
-	rc = elevator_init(blkdat->request_queue, &elevator_noop);
+	elevator_exit(blkdat->request_queue->elevator);
+	rc = elevator_init(blkdat->request_queue, "noop");
 	if (rc)
 		goto cleanup_queue;

===== include/linux/blkdev.h 1.152 vs edited =====

include/linux/blkdev.h +7 -3
@@ -19,8 +19,8 @@
 
 struct request_queue;
 typedef struct request_queue request_queue_t;
-struct elevator_s;
-typedef struct elevator_s elevator_t;
+struct elevator_queue;
+typedef struct elevator_queue elevator_t;
 struct request_pm_state;
 
 #define BLKDEV_MIN_RQ	4

@@ -80,6 +80,7 @@
 	int count[2];
 	mempool_t *rq_pool;
 	wait_queue_head_t wait[2];
+	wait_queue_head_t drain;
 };
 
 #define BLK_MAX_CDB	16

@@ -279,7 +280,7 @@
 	 */
 	struct list_head	queue_head;
 	struct request		*last_merge;
-	elevator_t		elevator;
+	elevator_t		*elevator;
 
 	/*
 	 * the queue request freelist, one for reads and one for writes

@@ -381,6 +382,7 @@
 #define QUEUE_FLAG_REENTER	6	/* Re-entrancy avoidance */
 #define QUEUE_FLAG_PLUGGED	7	/* queue is plugged */
 #define QUEUE_FLAG_ORDERED	8	/* supports ordered writes */
+#define QUEUE_FLAG_DRAIN	9	/* draining queue for sched switch */
 
 #define blk_queue_plugged(q)	test_bit(QUEUE_FLAG_PLUGGED, &(q)->queue_flags)
 #define blk_queue_tagged(q)	test_bit(QUEUE_FLAG_QUEUED, &(q)->queue_flags)

@@ -617,6 +619,8 @@
 extern void generic_unplug_device(request_queue_t *);
 extern void __generic_unplug_device(request_queue_t *);
 extern long nr_blockdev_pages(void);
+extern void blk_wait_queue_drained(request_queue_t *);
+extern void blk_finish_queue_drain(request_queue_t *);
 
 int blk_get_queue(request_queue_t *);
 request_queue_t *blk_alloc_queue(int);

===== include/linux/elevator.h 1.31 vs edited =====

include/linux/elevator.h +33 -22

82 lines not on this page · raw message

Signature · 1 line
Jens Axboe
#1 lore raw

Jens Axboe wrote

Quoted 12 lines
drivers/block/ll_rw_blk.c +12 -0
+void blk_wait_queue_drained(request_queue_t *q)
+{
+	struct request_list *rl = &q->rq;
+	DEFINE_WAIT(wait);
+
+	set_bit(QUEUE_FLAG_DRAIN, &q->queue_flags);
+
+	prepare_to_wait(&rl->drain, &wait, TASK_UNINTERRUPTIBLE);
+	do {
+		spin_lock_irq(q->queue_lock);
+		if (!rl->count[READ] && !rl->count[WRITE]) {
+			spin_unlock_irq(q->queue_lock);

drivers/block/ll_rw_blk.c line 2501

Quoted 10 lines
drivers/block/ll_rw_blk.c +10 -0
+			break;
+		}
+
+		__generic_unplug_device(q);
+		spin_unlock_irq(q->queue_lock);
+		io_schedule();
+	} while (1);
+	finish_wait(&rl->drain, &wait);
+}
+

Jens,

If a driver does not allocate requests through blk_get_request, will the rl->count[] tests need to be changed or do those drivers need to be changed? For example, if SCSI insterts a special request into the queue, then someone swaps the io scheduler with no outstanding normal requests (so the rl->counts will be zero), could the special request still be in the queue since it allocated its request using kmalloc (the request is allocated as part of the scsi command).

Mike

author
#2 lore raw
Quoted 26 lines · Mike Christie

Jens Axboe wrote

Quoted 12 lines
drivers/block/ll_rw_blk.c +12 -0
+void blk_wait_queue_drained(request_queue_t *q)
+{
+	struct request_list *rl = &q->rq;
+	DEFINE_WAIT(wait);
+
+	set_bit(QUEUE_FLAG_DRAIN, &q->queue_flags);
+
+	prepare_to_wait(&rl->drain, &wait, TASK_UNINTERRUPTIBLE);
+	do {
+		spin_lock_irq(q->queue_lock);
+		if (!rl->count[READ] && !rl->count[WRITE]) {
+			spin_unlock_irq(q->queue_lock);

drivers/block/ll_rw_blk.c line 2501

Quoted 10 lines
drivers/block/ll_rw_blk.c +10 -0
+			break;
+		}
+
+		__generic_unplug_device(q);
+		spin_unlock_irq(q->queue_lock);
+		io_schedule();
+	} while (1);
+	finish_wait(&rl->drain, &wait);
+}
+

Jens,

Quoted 7 lines

If a driver does not allocate requests through blk_get_request, will the rl->count[] tests need to be changed or do those drivers need to be changed? For example, if SCSI insterts a special request into the queue, then someone swaps the io scheduler with no outstanding normal requests (so the rl->counts will be zero), could the special request still be in the queue since it allocated its request using kmalloc (the request is allocated as part of the scsi command).

Yes, we need to eliminate stack and kmalloc'ed requests for this to be completely solid. This is something we have been doing for some time already, now would be a good time to fully complete it.

There's no way to block incoming request at ->add_request_fn() time, so we have to do it on merge (first path) or request allocation.

Signature · 1 line
Jens Axboe
author
#3 lore raw
Quoted 35 lines · Jens Axboe
Quoted 26 lines · Mike Christie

Jens Axboe wrote

Quoted 12 lines
drivers/block/ll_rw_blk.c +12 -0
+void blk_wait_queue_drained(request_queue_t *q)
+{
+	struct request_list *rl = &q->rq;
+	DEFINE_WAIT(wait);
+
+	set_bit(QUEUE_FLAG_DRAIN, &q->queue_flags);
+
+	prepare_to_wait(&rl->drain, &wait, TASK_UNINTERRUPTIBLE);
+	do {
+		spin_lock_irq(q->queue_lock);
+		if (!rl->count[READ] && !rl->count[WRITE]) {
+			spin_unlock_irq(q->queue_lock);

drivers/block/ll_rw_blk.c line 2501

Quoted 10 lines
drivers/block/ll_rw_blk.c +10 -0
+			break;
+		}
+
+		__generic_unplug_device(q);
+		spin_unlock_irq(q->queue_lock);
+		io_schedule();
+	} while (1);
+	finish_wait(&rl->drain, &wait);
+}
+

Jens,

Quoted 7 lines

If a driver does not allocate requests through blk_get_request, will the rl->count[] tests need to be changed or do those drivers need to be changed? For example, if SCSI insterts a special request into the queue, then someone swaps the io scheduler with no outstanding normal requests (so the rl->counts will be zero), could the special request still be in the queue since it allocated its request using kmalloc (the request is allocated as part of the scsi command).

Quoted 6 lines

Yes, we need to eliminate stack and kmalloc'ed requests for this to be completely solid. This is something we have been doing for some time already, now would be a good time to fully complete it.

There's no way to block incoming request at ->add_request_fn() time, so we have to do it on merge (first path) or request allocation.

Actually, I already thought of this the other day and there's no problem even currently with stack or privately allocated requests. These don't go through blk_alloc_request(), which in turn calls the io scheduler private ->elv_set_request() function, so they don't have any io scheduler private memory allocated to them. This also means that they don't reside inside the io scheduler, but are directly attached to the queue head.

So I don't think we have a problem there at all, it should just work.

Signature · 1 line
Jens Axboe