block: convert remaining drivers which share a request queue

PATCH In review Omar Sandoval 3 participants 11 messages Mar 28, 2017

Subsystems (2)

Block layer SPARC + UltraSPARC (sparc/sparc64)

lore

Share & tools

lore

Omar Sandoval

author

2017-03-28 06:29 UTC #0 lore raw

cover block: convert remaining drivers which share a request queue

From: Omar Sandoval <[email protected]>

This is clearly the pinnacle of my career: converting all remaining block drivers which share a request queue across gendisks to use a separate request queue per gendisk. These are all compile tested (but the last two platform-specific ones involved hacking the Kconfig and commenting out a bunch of arch-dependent code to get the rest to compile), no runtime testing at all.

Let me know if I missed any. Even better, let me know if we can just delete some of these entirely.

Thanks!

Omar Sandoval (6):
hd: stop sharing request queue across multiple gendisks parport/pd: stop sharing request queue across multiple gendisks parport/pcd: stop sharing request queue across multiple gendisks parport/pf: stop sharing request queue across multiple gendisks
swim: stop sharing request queue across multiple gendisks
jsflash: stop sharing request queue across multiple gendisks

 drivers/block/hd.c          | 62 ++++++++++++++++++++++++++++-----------------
 drivers/block/paride/pcd.c  | 57 +++++++++++++++++++++++++++--------------
 drivers/block/paride/pd.c   | 50 ++++++++++++++++++++++++------------
 drivers/block/paride/pf.c   | 57 +++++++++++++++++++++++++++--------------
 drivers/block/swim.c        | 55 ++++++++++++++++++++++++++--------------
 drivers/sbus/char/jsflash.c | 50 ++++++++++++++++++++++++++----------
 6 files changed, 221 insertions(+), 110 deletions(-)

Signature · 1 line

2.12.1

Omar Sandoval

author

2017-03-28 06:29 UTC #1 lore raw

PATCH 1/6 hd: stop sharing request queue across multiple gendisks

From: Omar Sandoval <[email protected]>

Compile-tested only.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/hd.c | 62 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

drivers/block/hd.c +39 ~~-23~~

index 6043648da1e8..79d63b20a297 100644
@@ -95,7 +95,7 @@
 #define ICRC_ERR		0x80	/* new meaning:  CRC error during transfer */
 
 static DEFINE_SPINLOCK(hd_lock);
-static struct request_queue *hd_queue;
+static unsigned int hd_queue;
 static struct request *hd_req;
 
 #define TIMEOUT_VALUE	(6*HZ)

@@ -537,7 +537,7 @@ static void hd_times_out(unsigned long dummy)
 	if (!hd_req)
 		return;
 
-	spin_lock_irq(hd_queue->queue_lock);
+	spin_lock_irq(&hd_lock);
 	reset = 1;
 	name = hd_req->rq_disk->disk_name;
 	printk("%s: timeout\n", name);

@@ -548,7 +548,7 @@ static void hd_times_out(unsigned long dummy)
 		hd_end_request_cur(-EIO);
 	}
 	hd_request();
-	spin_unlock_irq(hd_queue->queue_lock);
+	spin_unlock_irq(&hd_lock);
 }
 
 static int do_special_op(struct hd_i_struct *disk, struct request *req)

@@ -566,6 +566,25 @@ static int do_special_op(struct hd_i_struct *disk, struct request *req)
 	return 1;
 }
 
+static int set_next_request(void)
+{
+	struct request_queue *q;
+	int old_pos = hd_queue;
+
+	do {
+		q = hd_gendisk[hd_queue]->queue;
+		if (++hd_queue == NR_HD)
+			hd_queue = 0;
+		if (q) {
+			hd_req = blk_fetch_request(q);
+			if (hd_req)
+				break;
+		}
+	} while (hd_queue != old_pos);
+
+	return hd_req != NULL;
+}
+
 /*
  * The driver enables interrupts as much as possible.  In order to do this,
  * (a) the device-interrupt is disabled before entering hd_request(),

@@ -587,12 +606,9 @@ static void hd_request(void)
 repeat:
 	del_timer(&device_timer);
 
-	if (!hd_req) {
-		hd_req = blk_fetch_request(hd_queue);
-		if (!hd_req) {
-			do_hd = NULL;
-			return;
-		}
+	if (!hd_req && !set_next_request()) {
+		do_hd = NULL;
+		return;
 	}
 	req = hd_req;

@@ -676,7 +692,7 @@ static irqreturn_t hd_interrupt(int irq, void *dev_id)
 {
 	void (*handler)(void) = do_hd;
 
-	spin_lock(hd_queue->queue_lock);
+	spin_lock(&hd_lock);
 
 	do_hd = NULL;
 	del_timer(&device_timer);

@@ -684,7 +700,7 @@ static irqreturn_t hd_interrupt(int irq, void *dev_id)
 		handler = unexpected_hd_interrupt;
 	handler();
 
-	spin_unlock(hd_queue->queue_lock);
+	spin_unlock(&hd_lock);
 
 	return IRQ_HANDLED;
 }

@@ -700,16 +716,8 @@ static int __init hd_init(void)
 	if (register_blkdev(HD_MAJOR, "hd"))
 		return -1;
 
-	hd_queue = blk_init_queue(do_hd_request, &hd_lock);
-	if (!hd_queue) {
-		unregister_blkdev(HD_MAJOR, "hd");
-		return -ENOMEM;
-	}
-
-	blk_queue_max_hw_sectors(hd_queue, 255);
 	init_timer(&device_timer);
 	device_timer.function = hd_times_out;
-	blk_queue_logical_block_size(hd_queue, 512);
 
 	if (!NR_HD) {
 		/*

@@ -742,7 +750,11 @@ static int __init hd_init(void)
 		sprintf(disk->disk_name, "hd%c", 'a'+drive);
 		disk->private_data = p;
 		set_capacity(disk, p->head * p->sect * p->cyl);
-		disk->queue = hd_queue;
+		disk->queue = blk_init_queue(do_hd_request, &hd_lock);
+		if (!disk->queue)
+			goto Enomem;
+		blk_queue_max_hw_sectors(disk->queue, 255);
+		blk_queue_logical_block_size(disk->queue, 512);
 		p->unit = drive;
 		hd_gendisk[drive] = disk;
 		printk("%s: %luMB, CHS=%d/%d/%d\n",

@@ -781,11 +793,15 @@ static int __init hd_init(void)
 out:
 	del_timer(&device_timer);
 	unregister_blkdev(HD_MAJOR, "hd");
-	blk_cleanup_queue(hd_queue);
 	return -1;
 Enomem:
-	while (drive--)
-		put_disk(hd_gendisk[drive]);
+	for (drive = 0; drive < NR_HD; drive++) {
+		if (hd_gendisk[drive]) {
+			if (hd_gendisk[drive]->queue)
+				blk_cleanup_queue(hd_gendisk[drive]->queue);
+			put_disk(hd_gendisk[drive]);
+		}
+	}
 	goto out;
 }

Signature · 1 line

2.12.1

Omar Sandoval

author

2017-03-28 06:29 UTC #5 lore raw

PATCH 4/6 parport/pf: stop sharing request queue across multiple gendisks

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Compile-tested only.

Cc Tim Waugh <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/paride/pf.c | 57 +++++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 19 deletions(-)

drivers/block/paride/pf.c +38 ~~-19~~

index 14c5d32f5d8b..f24ca7315ddc 100644
@@ -287,6 +287,12 @@ static void __init pf_init_units(void)
 		struct gendisk *disk = alloc_disk(1);
 		if (!disk)
 			continue;
+		disk->queue = blk_init_queue(do_pf_request, &pf_spin_lock);
+		if (!disk->queue) {
+			put_disk(disk);
+			return;
+		}
+		blk_queue_max_segments(disk->queue, cluster);
 		pf->disk = disk;
 		pf->pi = &pf->pia;
 		pf->media_status = PF_NM;

@@ -772,7 +778,28 @@ static int pf_ready(void)
 	return (((status_reg(pf_current) & (STAT_BUSY | pf_mask)) == pf_mask));
 }
 
-static struct request_queue *pf_queue;
+static int pf_queue;
+
+static int set_next_request(void)
+{
+	struct pf_unit *pf;
+	struct request_queue *q;
+	int old_pos = pf_queue;
+
+	do {
+		pf = &units[pf_queue];
+		q = pf->present ? pf->disk->queue : NULL;
+		if (++pf_queue == PF_UNITS)
+			pf_queue = 0;
+		if (q) {
+			pf_req = blk_fetch_request(q);
+			if (pf_req)
+				break;
+		}
+	} while (pf_queue != old_pos);
+
+	return pf_req != NULL;
+}
 
 static void pf_end_request(int err)
 {

@@ -780,16 +807,13 @@ static void pf_end_request(int err)
 		pf_req = NULL;
 }
 
-static void do_pf_request(struct request_queue * q)
+static void pf_request(void)
 {
 	if (pf_busy)
 		return;
 repeat:
-	if (!pf_req) {
-		pf_req = blk_fetch_request(q);
-		if (!pf_req)
-			return;
-	}
+	if (!pf_req && !set_next_request())
+		return;
 
 	pf_current = pf_req->rq_disk->private_data;
 	pf_block = blk_rq_pos(pf_req);

@@ -817,6 +841,11 @@ static void do_pf_request(struct request_queue * q)
 	}
 }
 
+static void do_pf_request(struct request_queue *q)
+{
+	pf_request();
+}
+
 static int pf_next_buf(void)
 {
 	unsigned long saved_flags;

@@ -846,7 +875,7 @@ static inline void next_request(int err)
 	spin_lock_irqsave(&pf_spin_lock, saved_flags);
 	pf_end_request(err);
 	pf_busy = 0;
-	do_pf_request(pf_queue);
+	pf_request();
 	spin_unlock_irqrestore(&pf_spin_lock, saved_flags);
 }

@@ -972,15 +1001,6 @@ static int __init pf_init(void)
 			put_disk(pf->disk);
 		return -EBUSY;
 	}
-	pf_queue = blk_init_queue(do_pf_request, &pf_spin_lock);
-	if (!pf_queue) {
-		unregister_blkdev(major, name);
-		for (pf = units, unit = 0; unit < PF_UNITS; pf++, unit++)
-			put_disk(pf->disk);
-		return -ENOMEM;
-	}
-
-	blk_queue_max_segments(pf_queue, cluster);
 
 	for (pf = units, unit = 0; unit < PF_UNITS; pf++, unit++) {
 		struct gendisk *disk = pf->disk;

@@ -988,7 +1008,6 @@ static int __init pf_init(void)
 		if (!pf->present)
 			continue;
 		disk->private_data = pf;
-		disk->queue = pf_queue;
 		add_disk(disk);
 	}
 	return 0;

@@ -1003,10 +1022,10 @@ static void __exit pf_exit(void)
 		if (!pf->present)
 			continue;
 		del_gendisk(pf->disk);
+		blk_cleanup_queue(pf->disk->queue);
 		put_disk(pf->disk);
 		pi_release(pf->pi);
 	}
-	blk_cleanup_queue(pf_queue);
 }
 
 MODULE_LICENSE("GPL");

Signature · 1 line

2.12.1

Omar Sandoval

author

2017-03-28 06:29 UTC #7 lore raw

PATCH 6/6 jsflash: stop sharing request queue across multiple gendisks

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Compile-tested only (by hacking it to compile on x86).

Cc David S. Miller <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/sbus/char/jsflash.c | 50 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 14 deletions(-)

drivers/sbus/char/jsflash.c +36 ~~-14~~

index 6ff61dad5e21..62fed9dc893e 100644
@@ -183,11 +183,33 @@ static void jsfd_read(char *buf, unsigned long p, size_t togo) {
 	}
 }
 
-static void jsfd_do_request(struct request_queue *q)
+static int jsfd_queue;
+
+static struct request *jsfd_next_request(void)
+{
+	struct request_queue *q;
+	struct request *rq;
+	int old_pos = jsfd_queue;
+
+	do {
+		q = jsfd_disk[jsfd_queue]->queue;
+		if (++jsfd_queue == JSF_MAX)
+			jsfd_queue = 0;
+		if (q) {
+			rq = blk_fetch_request(q);
+			if (rq)
+				return rq;
+		}
+	} while (jsfd_queue != old_pos);
+
+	return NULL;
+}
+
+static void jsfd_request(void)
 {
 	struct request *req;
 
-	req = blk_fetch_request(q);
+	req = jsfd_next_request();
 	while (req) {
 		struct jsfd_part *jdp = req->rq_disk->private_data;
 		unsigned long offset = blk_rq_pos(req) << 9;

@@ -211,10 +233,15 @@ static void jsfd_do_request(struct request_queue *q)
 		err = 0;
 	end:
 		if (!__blk_end_request_cur(req, err))
-			req = blk_fetch_request(q);
+			req = jsfd_next_request();
 	}
 }
 
+static void jsfd_do_request(struct request_queue *q)
+{
+	jsfd_request();
+}
+
 /*
  * The memory devices use the full 32/64 bits of the offset, and so we cannot
  * check against negative addresses: they are ok. The return value is weird,

@@ -544,8 +571,6 @@ static int jsflash_init(void)
 	return 0;
 }
 
-static struct request_queue *jsf_queue;
-
 static int jsfd_init(void)
 {
 	static DEFINE_SPINLOCK(lock);

@@ -562,6 +587,11 @@ static int jsfd_init(void)
 		struct gendisk *disk = alloc_disk(1);
 		if (!disk)
 			goto out;
+		disk->queue = blk_init_queue(jsfd_do_request, &lock);
+		if (!disk->queue) {
+			put_disk(disk);
+			goto out;
+		}
 		jsfd_disk[i] = disk;
 	}

@@ -570,13 +600,6 @@ static int jsfd_init(void)
 		goto out;
 	}
 
-	jsf_queue = blk_init_queue(jsfd_do_request, &lock);
-	if (!jsf_queue) {
-		err = -ENOMEM;
-		unregister_blkdev(JSFD_MAJOR, "jsfd");
-		goto out;
-	}
-
 	for (i = 0; i < JSF_MAX; i++) {
 		struct gendisk *disk = jsfd_disk[i];
 		if ((i & JSF_PART_MASK) >= JSF_NPART) continue;

@@ -589,7 +612,6 @@ static int jsfd_init(void)
 		disk->fops = &jsfd_fops;
 		set_capacity(disk, jdp->dsize >> 9);
 		disk->private_data = jdp;
-		disk->queue = jsf_queue;
 		add_disk(disk);
 		set_disk_ro(disk, 1);
 	}

@@ -619,6 +641,7 @@ static void __exit jsflash_cleanup_module(void)
 	for (i = 0; i < JSF_MAX; i++) {
 		if ((i & JSF_PART_MASK) >= JSF_NPART) continue;
 		del_gendisk(jsfd_disk[i]);
+		blk_cleanup_queue(jsfd_disk[i]->queue);
 		put_disk(jsfd_disk[i]);
 	}
 	if (jsf0.busy)

@@ -628,7 +651,6 @@ static void __exit jsflash_cleanup_module(void)
 
 	misc_deregister(&jsf_dev);
 	unregister_blkdev(JSFD_MAJOR, "jsfd");
-	blk_cleanup_queue(jsf_queue);
 }
 
 module_init(jsflash_init_module);

Signature · 1 line

2.12.1

Omar Sandoval

author

2017-03-28 06:29 UTC #3 lore raw

PATCH 2/6 parport/pd: stop sharing request queue across multiple gendisks

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Compile-tested only.

Cc Tim Waugh <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/paride/pd.c | 50 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 16 deletions(-)

drivers/block/paride/pd.c +34 ~~-16~~

index 9cfd2e06a649..b05e151c9b38 100644
@@ -381,12 +381,33 @@ static enum action do_pd_write_start(void);
 static enum action do_pd_read_drq(void);
 static enum action do_pd_write_done(void);
 
-static struct request_queue *pd_queue;
+static int pd_queue;
 static int pd_claimed;
 
 static struct pd_unit *pd_current; /* current request's drive */
 static PIA *pi_current; /* current request's PIA */
 
+static int set_next_request(void)
+{
+	struct gendisk *disk;
+	struct request_queue *q;
+	int old_pos = pd_queue;
+
+	do {
+		disk = pd[pd_queue].gd;
+		q = disk ? disk->queue : NULL;
+		if (++pd_queue == PD_UNITS)
+			pd_queue = 0;
+		if (q) {
+			pd_req = blk_fetch_request(q);
+			if (pd_req)
+				break;
+		}
+	} while (pd_queue != old_pos);
+
+	return pd_req != NULL;
+}
+
 static void run_fsm(void)
 {
 	while (1) {

@@ -418,8 +439,7 @@ static void run_fsm(void)
 				spin_lock_irqsave(&pd_lock, saved_flags);
 				if (!__blk_end_request_cur(pd_req,
 						res == Ok ? 0 : -EIO)) {
-					pd_req = blk_fetch_request(pd_queue);
-					if (!pd_req)
+					if (!set_next_request())
 						stop = 1;
 				}
 				spin_unlock_irqrestore(&pd_lock, saved_flags);

@@ -839,7 +859,13 @@ static void pd_probe_drive(struct pd_unit *disk)
 	p->first_minor = (disk - pd) << PD_BITS;
 	disk->gd = p;
 	p->private_data = disk;
-	p->queue = pd_queue;
+	p->queue = blk_init_queue(do_pd_request, &pd_lock);
+	if (!p->queue) {
+		disk->gd = NULL;
+		put_disk(p);
+		return;
+	}
+	blk_queue_max_hw_sectors(p->queue, cluster);
 
 	if (disk->drive == -1) {
 		for (disk->drive = 0; disk->drive <= 1; disk->drive++)

@@ -919,26 +945,18 @@ static int __init pd_init(void)
 	if (disable)
 		goto out1;
 
-	pd_queue = blk_init_queue(do_pd_request, &pd_lock);
-	if (!pd_queue)
-		goto out1;
-
-	blk_queue_max_hw_sectors(pd_queue, cluster);
-
 	if (register_blkdev(major, name))
-		goto out2;
+		goto out1;
 
 	printk("%s: %s version %s, major %d, cluster %d, nice %d\n",
 	       name, name, PD_VERSION, major, cluster, nice);
 	if (!pd_detect())
-		goto out3;
+		goto out2;
 
 	return 0;
 
-out3:
-	unregister_blkdev(major, name);
 out2:
-	blk_cleanup_queue(pd_queue);
+	unregister_blkdev(major, name);
 out1:
 	return -ENODEV;
 }

@@ -953,11 +971,11 @@ static void __exit pd_exit(void)
 		if (p) {
 			disk->gd = NULL;
 			del_gendisk(p);
+			blk_cleanup_queue(p->queue);
 			put_disk(p);
 			pi_release(disk->pi);
 		}
 	}
-	blk_cleanup_queue(pd_queue);
 }
 
 MODULE_LICENSE("GPL");

Signature · 1 line

2.12.1

Omar Sandoval

author

2017-03-28 06:29 UTC #6 lore raw

PATCH 5/6 swim: stop sharing request queue across multiple gendisks

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Compile-tested only (by hacking it to compile on x86).

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/swim.c | 55 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 19 deletions(-)

drivers/block/swim.c +36 ~~-19~~

index b5afd495d482..3064be6cf375 100644
@@ -211,7 +211,7 @@ enum head {
 struct swim_priv {
 	struct swim __iomem *base;
 	spinlock_t lock;
-	struct request_queue *queue;
+	int fdc_queue;
 	int floppy_count;
 	struct floppy_state unit[FD_MAX_UNIT];
 };

@@ -525,12 +525,33 @@ static int floppy_read_sectors(struct floppy_state *fs,
 	return 0;
 }
 
-static void redo_fd_request(struct request_queue *q)
+static struct request *swim_next_request(struct swim_priv *swd)
 {
+	struct request_queue *q;
+	struct request *rq;
+	int old_pos = swd->fdc_queue;
+
+	do {
+		q = swd->unit[swd->fdc_queue].disk->queue;
+		if (++swd->fdc_queue == swd->floppy_count)
+			swd->fdc_queue = 0;
+		if (q) {
+			rq = blk_fetch_request(q);
+			if (rq)
+				return rq;
+		}
+	} while (swd->fdc_queue != old_pos);
+
+	return NULL;
+}
+
+static void do_fd_request(struct request_queue *q)
+{
+	struct swim_priv *swd = q->queuedata;
 	struct request *req;
 	struct floppy_state *fs;
 
-	req = blk_fetch_request(q);
+	req = swim_next_request(swd);
 	while (req) {
 		int err = -EIO;

@@ -554,15 +575,10 @@ static void redo_fd_request(struct request_queue *q)
 		}
 	done:
 		if (!__blk_end_request_cur(req, err))
-			req = blk_fetch_request(q);
+			req = swim_next_request(swd);
 	}
 }
 
-static void do_fd_request(struct request_queue *q)
-{
-	redo_fd_request(q);
-}
-
 static struct floppy_struct floppy_type[4] = {
 	{    0,  0, 0,  0, 0, 0x00, 0x00, 0x00, 0x00, NULL }, /* no testing   */
 	{  720,  9, 1, 80, 0, 0x2A, 0x02, 0xDF, 0x50, NULL }, /* 360KB SS 3.5"*/

@@ -833,22 +849,25 @@ static int swim_floppy_init(struct swim_priv *swd)
 		return -EBUSY;
 	}
 
+	spin_lock_init(&swd->lock);
+
 	for (drive = 0; drive < swd->floppy_count; drive++) {
 		swd->unit[drive].disk = alloc_disk(1);
 		if (swd->unit[drive].disk == NULL) {
 			err = -ENOMEM;
 			goto exit_put_disks;
 		}
+		swd->unit[drive].disk->queue = blk_init_queue(do_fd_request,
+							      &swd->lock);
+		if (!swd->unit[drive].disk->queue) {
+			err = -ENOMEM;
+			put_disk(swd->unit[drive].disk);
+			goto exit_put_disks;
+		}
+		swd->unit[drive].disk->queue->queuedata = swd;
 		swd->unit[drive].swd = swd;
 	}
 
-	spin_lock_init(&swd->lock);
-	swd->queue = blk_init_queue(do_fd_request, &swd->lock);
-	if (!swd->queue) {
-		err = -ENOMEM;
-		goto exit_put_disks;
-	}
-
 	for (drive = 0; drive < swd->floppy_count; drive++) {
 		swd->unit[drive].disk->flags = GENHD_FL_REMOVABLE;
 		swd->unit[drive].disk->major = FLOPPY_MAJOR;

@@ -856,7 +875,6 @@ static int swim_floppy_init(struct swim_priv *swd)
 		sprintf(swd->unit[drive].disk->disk_name, "fd%d", drive);
 		swd->unit[drive].disk->fops = &floppy_fops;
 		swd->unit[drive].disk->private_data = &swd->unit[drive];
-		swd->unit[drive].disk->queue = swd->queue;
 		set_capacity(swd->unit[drive].disk, 2880);
 		add_disk(swd->unit[drive].disk);
 	}

@@ -943,13 +961,12 @@ static int swim_remove(struct platform_device *dev)
 
 	for (drive = 0; drive < swd->floppy_count; drive++) {
 		del_gendisk(swd->unit[drive].disk);
+		blk_cleanup_queue(swd->unit[drive].disk->queue);
 		put_disk(swd->unit[drive].disk);
 	}
 
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 
-	blk_cleanup_queue(swd->queue);
-
 	/* eject floppies */
 
 	for (drive = 0; drive < swd->floppy_count; drive++)

Signature · 1 line

2.12.1

Omar Sandoval

author

2017-03-28 06:30 UTC #4 lore raw

PATCH 3/6 parport/pcd: stop sharing request queue across multiple gendisks

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Compile-tested only.

Cc Tim Waugh <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/paride/pcd.c | 57 ++++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 19 deletions(-)

drivers/block/paride/pcd.c +38 ~~-19~~

index 939641d6e262..b1267ef34d5a 100644
@@ -300,6 +300,11 @@ static void pcd_init_units(void)
 		struct gendisk *disk = alloc_disk(1);
 		if (!disk)
 			continue;
+		disk->queue = blk_init_queue(do_pcd_request, &pcd_lock);
+		if (!disk->queue) {
+			put_disk(disk);
+			continue;
+		}
 		cd->disk = disk;
 		cd->pi = &cd->pia;
 		cd->present = 0;

@@ -735,18 +740,36 @@ static int pcd_detect(void)
 }
 
 /* I/O request processing */
-static struct request_queue *pcd_queue;
+static int pcd_queue;
+
+static int set_next_request(void)
+{
+	struct pcd_unit *cd;
+	struct request_queue *q;
+	int old_pos = pcd_queue;
+
+	do {
+		cd = &pcd[pcd_queue];
+		q = cd->present ? cd->disk->queue : NULL;
+		if (++pcd_queue == PCD_UNITS)
+			pcd_queue = 0;
+		if (q) {
+			pcd_req = blk_fetch_request(q);
+			if (pcd_req)
+				break;
+		}
+	} while (pcd_queue != old_pos);
+
+	return pcd_req != NULL;
+}
 
-static void do_pcd_request(struct request_queue * q)
+static void pcd_request(void)
 {
 	if (pcd_busy)
 		return;
 	while (1) {
-		if (!pcd_req) {
-			pcd_req = blk_fetch_request(q);
-			if (!pcd_req)
-				return;
-		}
+		if (!pcd_req && !set_next_request())
+			return;
 
 		if (rq_data_dir(pcd_req) == READ) {
 			struct pcd_unit *cd = pcd_req->rq_disk->private_data;

@@ -766,6 +789,11 @@ static void do_pcd_request(struct request_queue * q)
 	}
 }
 
+static void do_pcd_request(struct request_queue *q)
+{
+	pcd_request();
+}
+
 static inline void next_request(int err)
 {
 	unsigned long saved_flags;

@@ -774,7 +802,7 @@ static inline void next_request(int err)
 	if (!__blk_end_request_cur(pcd_req, err))
 		pcd_req = NULL;
 	pcd_busy = 0;
-	do_pcd_request(pcd_queue);
+	pcd_request();
 	spin_unlock_irqrestore(&pcd_lock, saved_flags);
 }

@@ -849,7 +877,7 @@ static void do_pcd_read_drq(void)
 
 	do_pcd_read();
 	spin_lock_irqsave(&pcd_lock, saved_flags);
-	do_pcd_request(pcd_queue);
+	pcd_request();
 	spin_unlock_irqrestore(&pcd_lock, saved_flags);
 }

@@ -957,19 +985,10 @@ static int __init pcd_init(void)
 		return -EBUSY;
 	}
 
-	pcd_queue = blk_init_queue(do_pcd_request, &pcd_lock);
-	if (!pcd_queue) {
-		unregister_blkdev(major, name);
-		for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++)
-			put_disk(cd->disk);
-		return -ENOMEM;
-	}
-
 	for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++) {
 		if (cd->present) {
 			register_cdrom(&cd->info);
 			cd->disk->private_data = cd;
-			cd->disk->queue = pcd_queue;
 			add_disk(cd->disk);
 		}
 	}

@@ -988,9 +1007,9 @@ static void __exit pcd_exit(void)
 			pi_release(cd->pi);
 			unregister_cdrom(&cd->info);
 		}
+		blk_cleanup_queue(cd->disk->queue);
 		put_disk(cd->disk);
 	}
-	blk_cleanup_queue(pcd_queue);
 	unregister_blkdev(major, name);
 	pi_unregister_driver(par_drv);
 }

Signature · 1 line

2.12.1

Christoph Hellwig

2017-03-28 06:32 UTC #2 lore raw

Re: [PATCH 1/6] hd: stop sharing request queue across multiple gendisks

In reply to Omar Sandoval

Can we just remove this driver entirely? I'm pretty sure Linux will have all kinds of problems with hardware of that age anyway.

Christoph Hellwig

2017-03-28 06:33 UTC #8 lore raw

In reply to Omar Sandoval

Quoted 11 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>
This is clearly the pinnacle of my career: converting all remaining block drivers which share a request queue across gendisks to use a separate request queue per gendisk. These are all compile tested (but the last two platform-specific ones involved hacking the Kconfig and commenting out a bunch of arch-dependent code to get the rest to compile), no runtime testing at all.
Let me know if I missed any. Even better, let me know if we can just delete some of these entirely.

Weren't the floppy drivers the prime example of shared request queue? I haven't looked at any of them for a while, so I'm not sure if that's still the case.

Omar Sandoval

author

2017-03-28 06:34 UTC #9 lore raw

Quoted 10 lines · Christoph Hellwig

Quoted 9 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
This is clearly the pinnacle of my career: converting all remaining block drivers which share a request queue across gendisks to use a separate request queue per gendisk. These are all compile tested (but the last two platform-specific ones involved hacking the Kconfig and commenting out a bunch of arch-dependent code to get the rest to compile), no runtime testing at all.

Quoted 6 lines

Quoted 2 lines
Let me know if I missed any. Even better, let me know if we can just delete some of these entirely.
Weren't the floppy drivers the prime example of shared request queue? I haven't looked at any of them for a while, so I'm not sure if that's still the case.

Those have all been fixed up, besides the swim one in this series:

786029ff810f ("amiga floppy: Stop sharing request queue across multiple gendisks") 639e2f2aa76e ("atari floppy: Stop sharing request queue across multiple gendisks") 488211844e0c ("floppy: switch to one queue per drive instead of sharing a queue")

Jens Axboe

maintainer

2017-03-28 21:07 UTC #10 lore raw

In reply to Omar Sandoval

Quoted 9 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>
This is clearly the pinnacle of my career: converting all remaining block drivers which share a request queue across gendisks to use a separate request queue per gendisk. These are all compile tested (but the last two platform-specific ones involved hacking the Kconfig and commenting out a bunch of arch-dependent code to get the rest to compile), no runtime testing at all.

Quoted 3 lines

Let me know if I missed any. Even better, let me know if we can just delete some of these entirely.

It's all downhill from here, Omar, I don't see how you are ever going to top this in terms of impact.

Applied for 4.12!

Signature · 1 line

Jens Axboe