Skip to contentSkip to conversation

floppy: cleanup patches

PATCH In review Stephen Hemminger 6 participants 23 messages Jun 9, 2010 – Aug 23, 2010

Subsystems (2)
Block layer Floppy driver
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover floppy: cleanup patches

When going through floppy driver addressing some of the timing bugs, I also saw these leftover things that ought to be cleaned up.

#1 lore raw
Quoted 3 lines · Stephen Hemminger

When going through floppy driver addressing some of the timing bugs, I also saw these leftover things that ought to be cleaned up.

Ack on them all as far as I'm concerned. I suspect the _real_ bugs in floppy.c are about various paths not holding the floppy_lock spinlock, but the patches all look fine and certainly don't make anything worse.

Mind re-sending them after 2.6.35, though? (Or maybe somebody like Greg wants to queue them up in their drievr trees)

Linus

author
#2 lore raw
Quoted 7 lines · On Wed, 9 Jun 2010 11:46:13 -0700 (PDT) Linus Torvalds…
Quoted 3 lines · Stephen Hemminger

When going through floppy driver addressing some of the timing bugs, I also saw these leftover things that ought to be cleaned up.

Quoted 8 lines

Ack on them all as far as I'm concerned. I suspect the _real_ bugs in floppy.c are about various paths not holding the floppy_lock spinlock, but the patches all look fine and certainly don't make anything worse.

Mind re-sending them after 2.6.35, though? (Or maybe somebody like Greg wants to queue them up in their drievr trees)

Linus

Current plan is to convert all the timers and old BH code to go through a single workqueue. This avoids all the races and cleans up the cancellation logic as well without having lots of other changes.

#3 lore raw
Quoted 4 lines · Stephen Hemminger

Current plan is to convert all the timers and old BH code to go through a single workqueue. This avoids all the races and cleans up the cancellation logic as well without having lots of other changes.

That will definitely help. We still end up with the things that call the request queue directly as a second (nonworkqueue) thread, but I think that's what the floppy_lock has _traditionally_ protected against, so it might all work out and largely fix the locking.

Linus

author
#4 lore raw

RFC 1/1 floppy: use single threaded workqueue

Fix races between timers and bottom half by using delayed work and a single threaded queue.

Still needs more testing.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/block/floppy.c |  172 ++++++++++++++++++++++++++-----------------------
 1 file changed, 94 insertions(+), 78 deletions(-)
drivers/block/floppy.c +94 -78
@@ -553,7 +553,7 @@ static void floppy_ready(void);
 static void floppy_start(void);
 static void process_fd_request(void);
 static void recalibrate_floppy(void);
-static void floppy_shutdown(unsigned long);
+static void floppy_shutdown(struct work_struct *);
 
 static int floppy_request_regions(int);
 static void floppy_release_regions(int);

@@ -590,6 +590,8 @@ static int buffer_max = -1;
 static struct floppy_fdc_state fdc_state[N_FDC];
 static int fdc;			/* current fdc */
 
+static struct workqueue_struct *floppy_workq;
+
 static struct floppy_struct *_floppy = floppy_type;
 static unsigned char current_drive;
 static long current_count_sectors;

@@ -626,16 +628,15 @@ static inline void set_debugt(void) { }
 static inline void debugt(const char *func, const char *msg) { }
 #endif /* DEBUGT */
 
-typedef void (*timeout_fn)(unsigned long);
-static DEFINE_TIMER(fd_timeout, floppy_shutdown, 0, 0);
 
+static DECLARE_DELAYED_WORK(fd_timeout, floppy_shutdown);
 static const char *timeout_message;
 
 static void is_alive(const char *func, const char *message)
 {
 	/* this routine checks whether the floppy driver is "alive" */
 	if (test_bit(0, &fdc_busy) && command_status < 2 &&
-	    !timer_pending(&fd_timeout)) {
+	    !delayed_work_pending(&fd_timeout)) {
 		DPRINT("%s: timeout handler died.  %s\n", func, message);
 	}
 }

@@ -663,15 +664,18 @@ static int output_log_pos;
 
 static void __reschedule_timeout(int drive, const char *message)
 {
+	unsigned long delay;
+
 	if (drive == current_reqD)
 		drive = current_drive;
-	del_timer(&fd_timeout);
+
 	if (drive < 0 || drive >= N_DRIVE) {
-		fd_timeout.expires = jiffies + 20UL * HZ;
+		delay = 20UL * HZ;
 		drive = 0;
 	} else
-		fd_timeout.expires = jiffies + UDP->timeout;
-	add_timer(&fd_timeout);
+		delay = UDP->timeout;
+
+	queue_delayed_work(floppy_workq, &fd_timeout, delay);
 	if (UDP->flags & FD_DEBUG)
 		DPRINT("reschedule timeout %s\n", message);
 	timeout_message = message;

@@ -886,11 +890,11 @@ static int _lock_fdc(int drive, bool int
 
 		set_current_state(TASK_RUNNING);
 		remove_wait_queue(&fdc_wait, &wait);
-		flush_scheduled_work();
+		flush_workqueue(floppy_workq);
 	}
 	command_status = FD_COMMAND_NONE;
 
-	__reschedule_timeout(drive, "lock fdc");
+	reschedule_timeout(drive, "lock fdc");
 	set_fdc(drive);
 	return 0;
 }

@@ -901,23 +905,15 @@ static int _lock_fdc(int drive, bool int
 /* unlocks the driver */
 static void unlock_fdc(void)
 {
-	unsigned long flags;
-
-	raw_cmd = NULL;
 	if (!test_bit(0, &fdc_busy))
 		DPRINT("FDC access conflict!\n");
 
-	if (do_floppy)
-		DPRINT("device interrupt still active at FDC release: %pf!\n",
-		       do_floppy);
+	raw_cmd = NULL;
 	command_status = FD_COMMAND_NONE;
-	spin_lock_irqsave(&floppy_lock, flags);
-	del_timer(&fd_timeout);
+	__cancel_delayed_work(&fd_timeout);
+	do_floppy = NULL;
 	cont = NULL;
 	clear_bit(0, &fdc_busy);
-	if (current_req || blk_peek_request(floppy_queue))
-		do_fd_request(floppy_queue);
-	spin_unlock_irqrestore(&floppy_lock, flags);
 	wake_up(&fdc_wait);
 }
 

@@ -989,26 +985,24 @@ static DECLARE_WORK(floppy_work, NULL);
 
 static void schedule_bh(void (*handler)(void))
 {
+	WARN_ON(work_pending(&floppy_work));
+
 	PREPARE_WORK(&floppy_work, (work_func_t)handler);
-	schedule_work(&floppy_work);
+	queue_work(floppy_workq, &floppy_work);
 }
 
-static DEFINE_TIMER(fd_timer, NULL, 0, 0);
+static DECLARE_DELAYED_WORK(fd_timer, NULL);
 
 static void cancel_activity(void)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&floppy_lock, flags);
 	do_floppy = NULL;
-	PREPARE_WORK(&floppy_work, (work_func_t)empty);
-	del_timer(&fd_timer);
-	spin_unlock_irqrestore(&floppy_lock, flags);
+	cancel_delayed_work_sync(&fd_timer);
+	cancel_work_sync(&floppy_work);
 }
 
 /* this function makes sure that the disk stays in the drive during the
  * transfer */
-static void fd_watchdog(void)
+static void fd_watchdog(struct work_struct *arg)
 {
 	debug_dcl(DP->flags, "calling disk change from watchdog\n");
 

@@ -1018,21 +1012,21 @@ static void fd_watchdog(void)
 		cont->done(0);
 		reset_fdc();
 	} else {
-		del_timer(&fd_timer);
-		fd_timer.function = (timeout_fn)fd_watchdog;
-		fd_timer.expires = jiffies + HZ / 10;
-		add_timer(&fd_timer);
+		cancel_delayed_work(&fd_timer);
+		PREPARE_DELAYED_WORK(&fd_timer, fd_watchdog);
+		queue_delayed_work(floppy_workq,
+				   &fd_timer, HZ / 10);
 	}
 }
 
 static void main_command_interrupt(void)
 {
-	del_timer(&fd_timer);
+	cancel_delayed_work(&fd_timer);
 	cont->interrupt();
 }
 
 /* waits for a delay (spinup or select) to pass */
-static int fd_wait_for_completion(unsigned long delay, timeout_fn function)
+static int fd_wait_for_completion(unsigned long expires, work_func_t function)
 {
 	if (FDCS->reset) {
 		reset_fdc();	/* do the reset during sleep to win time

@@ -1041,11 +1035,11 @@ static int fd_wait_for_completion(unsign
 		return 1;
 	}
 
-	if (time_before(jiffies, delay)) {
-		del_timer(&fd_timer);
-		fd_timer.function = function;
-		fd_timer.expires = delay;
-		add_timer(&fd_timer);
+	if (time_before(jiffies, expires)) {
+		cancel_delayed_work(&fd_timer);
+		PREPARE_DELAYED_WORK(&fd_timer, function);
+		queue_delayed_work(floppy_workq, &fd_timer,
+				   expires - jiffies);
 		return 1;
 	}
 	return 0;

@@ -1394,7 +1388,7 @@ static int fdc_dtr(void)
 	 */
 	FDCS->dtr = raw_cmd->rate & 3;
 	return fd_wait_for_completion(jiffies + 2UL * HZ / 100,
-				      (timeout_fn)floppy_ready);
+				      (work_func_t)floppy_ready);
 }				/* fdc_dtr */
 
 static void tell_sector(void)

@@ -1499,7 +1493,7 @@ static void setup_rw_floppy(void)
 	int flags;
 	int dflags;
 	unsigned long ready_date;
-	timeout_fn function;
+	work_func_t function;
 
 	flags = raw_cmd->flags;
 	if (flags & (FD_RAW_READ | FD_RAW_WRITE))

@@ -1513,9 +1507,9 @@ static void setup_rw_floppy(void)
 		 */
 		if (time_after(ready_date, jiffies + DP->select_delay)) {
 			ready_date -= DP->select_delay;
-			function = (timeout_fn)floppy_start;
+			function = (work_func_t)floppy_start;
 		} else
-			function = (timeout_fn)setup_rw_floppy;
+			function = (work_func_t)setup_rw_floppy;
 
 		/* wait until the floppy is spinning fast enough */
 		if (fd_wait_for_completion(ready_date, function))

@@ -1545,7 +1539,7 @@ static void setup_rw_floppy(void)
 		inr = result();
 		cont->interrupt();
 	} else if (flags & FD_RAW_NEED_DISK)
-		fd_watchdog();
+		fd_watchdog(NULL);
 }
 
 static int blind_seek;

@@ -1855,20 +1849,22 @@ static void show_floppy(void)
 		pr_info("do_floppy=%pf\n", do_floppy);
 	if (work_pending(&floppy_work))
 		pr_info("floppy_work.func=%pf\n", floppy_work.func);
-	if (timer_pending(&fd_timer))
-		pr_info("fd_timer.function=%pf\n", fd_timer.function);
-	if (timer_pending(&fd_timeout)) {
-		pr_info("timer_function=%pf\n", fd_timeout.function);
-		pr_info("expires=%lu\n", fd_timeout.expires - jiffies);
-		pr_info("now=%lu\n", jiffies);
-	}
+	if (delayed_work_pending(&fd_timer))
+		pr_info("delayed work.function=%p expires=%ld\n",
+		       fd_timer.work.func,
+		       fd_timer.timer.expires - jiffies);
+	if (delayed_work_pending(&fd_timeout))
+		pr_info("timer_function=%p expires=%ld\n",
+		       fd_timeout.work.func,
+		       fd_timeout.timer.expires - jiffies);
+
 	pr_info("cont=%p\n", cont);
 	pr_info("current_req=%p\n", current_req);
 	pr_info("command_status=%d\n", command_status);
 	pr_info("\n");
 }
 
-static void floppy_shutdown(unsigned long data)
+static void floppy_shutdown(struct work_struct *arg)
 {
 	unsigned long flags;
 

@@ -1923,7 +1919,7 @@ static int start_motor(void (*function)(
 
 	/* wait_for_completion also schedules reset if needed. */
 	return fd_wait_for_completion(DRS->select_date + DP->select_delay,
-				      (timeout_fn)function);
+				      (work_func_t)function);
 }
 
 static void floppy_ready(void)

@@ -2853,6 +2849,20 @@ static int make_raw_rw_request(void)
 	return 2;
 }
 
+static struct request *get_fd_request(void)
+{
+	struct request *req;
+
+	spin_lock_bh(floppy_queue->queue_lock);
+	req = blk_fetch_request(floppy_queue);
+	if (!req)
+		unlock_fdc();
+	spin_unlock_bh(floppy_queue->queue_lock);
+
+	return req;
+}
+
+
 static void redo_fd_request(void)
 {
 	int drive;

@@ -2864,16 +2874,10 @@ static void redo_fd_request(void)
 
 do_request:
 	if (!current_req) {
-		struct request *req;
+		struct request *req = get_fd_request();
 
-		spin_lock_irq(floppy_queue->queue_lock);
-		req = blk_fetch_request(floppy_queue);
-		spin_unlock_irq(floppy_queue->queue_lock);
-		if (!req) {
-			do_floppy = NULL;
-			unlock_fdc();
+		if (!req)
 			return;
-		}
 		current_req = req;
 	}
 	drive = (long)current_req->rq_disk->private_data;

@@ -2949,13 +2953,17 @@ static void do_fd_request(struct request
 			current_req->cmd_flags);
 		return;
 	}
-	if (test_bit(0, &fdc_busy)) {
+	if (test_and_set_bit(0, &fdc_busy)) {
 		/* fdc busy, this new request will be treated when the
 		   current one is done */
 		is_alive(__func__, "old request running");
 		return;
 	}
-	lock_fdc(MAXTIMEOUT, false);
+
+	command_status = FD_COMMAND_NONE;
+
+	__reschedule_timeout(MAXTIMEOUT, "fd_request");
+	set_fdc(0);
 	process_fd_request();
 	is_alive(__func__, "");
 }

@@ -4211,10 +4219,16 @@ static int __init floppy_init(void)
 	if (err)
 		goto out_unreg_blkdev;
 
+	floppy_workq = create_singlethread_workqueue("floppy");
+	if (!floppy_workq) {
+		err = -ENOMEM;
+		goto out_unreg_driver;
+	}
+
 	floppy_queue = blk_init_queue(do_fd_request, &floppy_lock);
 	if (!floppy_queue) {
 		err = -ENOMEM;
-		goto out_unreg_driver;
+		goto out_destroy_workq;
 	}
 	blk_queue_max_hw_sectors(floppy_queue, 64);
 

@@ -4247,7 +4261,7 @@ static int __init floppy_init(void)
 	use_virtual_dma = can_use_virtual_dma & 1;
 	fdc_state[0].address = FDC1;
 	if (fdc_state[0].address == -1) {
-		del_timer(&fd_timeout);
+		cancel_delayed_work(&fd_timeout);
 		err = -ENODEV;
 		goto out_unreg_region;
 	}

@@ -4258,7 +4272,7 @@ static int __init floppy_init(void)
 	fdc = 0;		/* reset fdc in case of unexpected interrupt */
 	err = floppy_grab_irq_and_dma();
 	if (err) {
-		del_timer(&fd_timeout);
+		cancel_delayed_work(&fd_timeout);
 		err = -EBUSY;
 		goto out_unreg_region;
 	}

@@ -4315,13 +4329,13 @@ static int __init floppy_init(void)
 		user_reset_fdc(-1, FD_RESET_ALWAYS, false);
 	}
 	fdc = 0;
-	del_timer(&fd_timeout);
+	cancel_delayed_work(&fd_timeout);
 	current_drive = 0;
 	initialized = true;
 	if (have_no_fdc) {
 		DPRINT("no floppy controllers found\n");
 		err = have_no_fdc;
-		goto out_flush_work;
+		goto out_release_dma;
 	}
 
 	for (drive = 0; drive < N_DRIVE; drive++) {

@@ -4336,7 +4350,7 @@ static int __init floppy_init(void)
 
 		err = platform_device_register(&floppy_device[drive]);
 		if (err)
-			goto out_flush_work;
+			goto out_release_dma;
 
 		err = device_create_file(&floppy_device[drive].dev,
 					 &dev_attr_cmos);

@@ -4355,13 +4369,14 @@ static int __init floppy_init(void)
 
 out_unreg_platform_dev:
 	platform_device_unregister(&floppy_device[drive]);
-out_flush_work:
-	flush_scheduled_work();
+out_release_dma:
 	if (atomic_read(&usage_count))
 		floppy_release_irq_and_dma();
 out_unreg_region:
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
 	blk_cleanup_queue(floppy_queue);
+out_destroy_workq:
+	destroy_workqueue(floppy_workq);
 out_unreg_driver:
 	platform_driver_unregister(&floppy_driver);
 out_unreg_blkdev:

@@ -4426,7 +4441,7 @@ static int floppy_grab_irq_and_dma(void)
 	 * We might have scheduled a free_irq(), wait it to
 	 * drain first:
 	 */
-	flush_scheduled_work();
+	flush_workqueue(floppy_workq);
 
 	if (fd_request_irq()) {
 		DPRINT("Unable to grab IRQ%d for the floppy driver\n",

@@ -4518,9 +4533,9 @@ static void floppy_release_irq_and_dma(v
 			pr_info("motor off timer %d still active\n", drive);
 #endif
 
-	if (timer_pending(&fd_timeout))
+	if (delayed_work_pending(&fd_timeout))
 		pr_info("floppy timer still active:%s\n", timeout_message);
-	if (timer_pending(&fd_timer))
+	if (delayed_work_pending(&fd_timer))
 		pr_info("auxiliary floppy timer still active\n");
 	if (work_pending(&floppy_work))
 		pr_info("work still pending\n");

@@ -4580,8 +4595,9 @@ static void __exit floppy_module_exit(vo
 		put_disk(disks[drive]);
 	}
 
-	del_timer_sync(&fd_timeout);
-	del_timer_sync(&fd_timer);
+	cancel_delayed_work_sync(&fd_timeout);
+	cancel_delayed_work_sync(&fd_timer);
+	destroy_workqueue(floppy_workq);
 	blk_cleanup_queue(floppy_queue);
 
 	if (atomic_read(&usage_count))
#5 lore raw

Re: [RFC] floppy: use single threaded workqueue

Quoted 4 lines · Stephen Hemminger

Fix races between timers and bottom half by using delayed work and a single threaded queue.

Still needs more testing.

Looks simple and straightforward to me. Ack. (And ack on the "needs more testing" part too, of course).

Linus

#6 lore raw

Re: [RFC] floppy: use single threaded workqueue

In reply to Stephen Hemminger
Quoted 10 lines · Stephen Hemminger

Fix races between timers and bottom half by using delayed work and a single threaded queue.

Still needs more testing.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/block/floppy.c |  172 ++++++++++++++++++++++++++-----------------------
 1 file changed, 94 insertions(+), 78 deletions(-)

Stephen,

Can confirm this fixes the following oops seen under KVM guests.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000035
IP: [<ffffffff812141f0>] setup_rw_floppy+0x229/0x2ca
PGD 284b8067 PUD 3d79f067 PMD 0
Oops: 0000 [#1] SMP
last sysfs file: /sys/devices/pci0000:00/0000:00:03.0/resource
CPU 1
Modules linked in: xt_tcpudp iptable_filter ip_tables x_tables

scsi_wait_scan uhci_hcd ohci_hcd ehci_hcd

Pid: 28, comm: events/1 Not tainted 2.6.35-rc5-35033-g14e84ff #221
/Bochs RIP: 0010:[<ffffffff812141f0>] [<ffffffff812141f0>] setup_rw_floppy+0x229/0x2ca

RSP: 0018:ffff88003e1a1e60  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000009 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000286 RDI: 0000000000000000
RBP: 00000000000000da R08: 0000000000000000 R09: ffffffff81214bdb
R10: 0000000000000000 R11: ffffffff817f7660 R12: 0000000000000000
R13: 0000000000000008 R14: ffffffff81214bdb R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff880001a40000(0000)

knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000035 CR3: 000000003a428000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process events/1 (pid: 28, threadinfo ffff88003e1a0000, task ffff88003e19a8f0)

Stack:
 0000000000000000 ffff88003e1a1ef8 ffffffff81692f00 ffff880001a56680
<0> ffff88003e19a8f0 ffffffff810466f1 0000000000000000 ffff88003e19a8f0
<0> ffffffff810495ad ffff88003e1a1ea8 ffff88003e1a1ea8 ffff88003e1a1ef8
Call Trace:
 [<ffffffff810466f1>] ? worker_thread+0x150/0x1f0
 [<ffffffff810495ad>] ? autoremove_wake_function+0x0/0x2e
 [<ffffffff810465a1>] ? worker_thread+0x0/0x1f0
 [<ffffffff810492bf>] ? kthread+0x79/0x81
 [<ffffffff81002be4>] ? kernel_thread_helper+0x4/0x10
#7 lore raw

Re: [RFC] floppy: use single threaded workqueue

Quoted 4 lines · Marcelo Tosatti

Can confirm this fixes the following oops seen under KVM guests.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000035

Hmm. That does make me want to merge it early, but at the same time I suspect it has gotten almost no testing on real hardware. So despite looking "obvious" and fixing one bug, I'm a bit nervous and I suspect that the prudent thing to do is still to take it early in the 2.3.36 merge window, and just mark it for stable.

Is anybody a big floppy user (on _real_ hardware, preferably with more than a couple machines) and could test it? I do feel kind of stupid for not taking it, but I don't have the hardware or the interest in really installing any in my current machines, and even if it looks fine and fixes some issue, I just don't feel safe enough without some more confirmation... As far as I know, the floppy oops thing has _only_ been seen on KVM (likely due to timing that just doesn't happen on real hardware), and while the KVM floppy emulation is probably fairly good, it also clearly isn't equivalent to hardware.

Linus

#8 lore raw

Re: [RFC] floppy: use single threaded workqueue

I don't use floopies much anymore, but I've got a fair number of machines around that have floppy drives (purchased across about 6 years with from a couple vendors and a variety of motherboards)

If someone can go to the effort of documenting what testing you want done I may be able to do it.

I would be installing new builds onto the machines to test them, so you can do testing that locks up the boxes without impacting anything.

the easier it is to do the tests, the more machines I can test on (bootable CD images that I could just pop in and then check the screen later would be ideal ;-)

these systems will not have network access.

David Lang

Quoted 29 lines · Linus Torvalds
Quoted 4 lines · Marcelo Tosatti

Can confirm this fixes the following oops seen under KVM guests.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000035

Hmm. That does make me want to merge it early, but at the same time I suspect it has gotten almost no testing on real hardware. So despite looking "obvious" and fixing one bug, I'm a bit nervous and I suspect that the prudent thing to do is still to take it early in the 2.3.36 merge window, and just mark it for stable.

Is anybody a big floppy user (on _real_ hardware, preferably with more than a couple machines) and could test it? I do feel kind of stupid for not taking it, but I don't have the hardware or the interest in really installing any in my current machines, and even if it looks fine and fixes some issue, I just don't feel safe enough without some more confirmation... As far as I know, the floppy oops thing has _only_ been seen on KVM (likely due to timing that just doesn't happen on real hardware), and while the KVM floppy emulation is probably fairly good, it also clearly isn't equivalent to hardware.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#9 lore raw

Re: [RFC] floppy: use single threaded workqueue

Quoted 7 lines · On Thu, Jul 22, 2010 at 3:22 PM, <[email protected]> wrote:

I don't use floopies much anymore, but I've got a fair number of machines around that have floppy drives (purchased across about 6 years with from a couple vendors and a variety of motherboards)

If someone can go to the effort of documenting what testing you want done I may be able to do it.

It doesn't really need to be all that extensive. The "interesting" operations tend to be (apart from just reading and writing data, of course):

- formatting a floppy (it's special, and historically relatively
often broke without people noticing for a while)

- the floppy format auto-detection

- floppy disk change detection

so just formatting floppies to a couple of different formats (ie do you possibly have DD and HD floppies?), mkfs them and write something to them, and then moving them to another machine and checking that reading the data off them through the auto-detected formats (/dev/fd0) works and gives the right results (just sha1sum the files).

The disk change detect can be a bit hard to see. It's unreliable with some media, so iirc we always flush caches after the last close (we didn't use to do that, and the disk change detection needed to just be reliable - and you could test that the disk change logic worked by just timing a read of the media and seeing if it came out of the cache). I think for floppies, the thing to see is if format detection works correctly when you switch between formats. Perhaps also the read-only marker (ie switch the floppy between read-only and read-write, and see that the status of the floppy is correctly
noticed: you should get a nice error if you try to write a read-only floppy, rather than getting IO errors).

I can't think of anything else relevant.

Linus

#10 lore raw

Re: [RFC] floppy: use single threaded workqueue

In reply to [email protected]
Quoted 10 lines · On Fri, Jul 23, 2010 at 6:22 AM, <[email protected]> wrote:

I don't use floopies much anymore, but I've got a fair number of machines around that have floppy drives (purchased across about 6 years with from a couple vendors and a variety of motherboards)

If someone can go to the effort of documenting what testing you want done I may be able to do it.

I would be installing new builds onto the machines to test them, so you can do testing that locks up the boxes without impacting anything.

Quoted 7 lines

the easier it is to do the tests, the more machines I can test on (bootable CD images that I could just pop in and then check the screen later would be ideal ;-)

these systems will not have network access.

David Lang

Hello Davide,

Any feedback of testing this patch on real hardware with Linus' methods ? Thanks.

Amos

author
#11 lore raw

Re: [RFC] floppy: use single threaded workqueue

Quoted 17 lines · On Sun, 22 Aug 2010 17:07:49 +0800 Amos Kong <[email protected]> wrote:
Quoted 16 lines · On Fri, Jul 23, 2010 at 6:22 AM, <[email protected]> wrote:

I don't use floopies much anymore, but I've got a fair number of machines around that have floppy drives (purchased across about 6 years with from a couple vendors and a variety of motherboards)

If someone can go to the effort of documenting what testing you want done I may be able to do it.

I would be installing new builds onto the machines to test them, so you can do testing that locks up the boxes without impacting anything.

the easier it is to do the tests, the more machines I can test on (bootable CD images that I could just pop in and then check the screen later would be ideal ;-)

these systems will not have network access.

Quoted 7 lines
Quoted 1 line

David Lang

Hello Davide,

Any feedback of testing this patch on real hardware with Linus' methods ? Thanks.

I did some testing, saw a glitch but wasn't sure if it was a 2.6.35 or floppy specific. But haven't investigated furthur because of impending Vyatta product release. Plan to revisit this week.

author
#12 lore raw

PATCH 1/8 floppy: initialize debug jiffies offset

In reply to Stephen Hemminger

Set debug jiffies offset at initialization. Avoids wierd values showing up if debugging enabled.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +3 -0
@@ -4175,6 +4175,9 @@ static int __init floppy_init(void)
 	int i, unit, drive;
 	int err, dr;
 
+	set_debugt();
+	interruptjiffies = resultjiffies = jiffies;
+
 #if defined(CONFIG_PPC)
 	if (check_legacy_ioport(FDC1))
 		return -ENODEV;
author
#13 lore raw

PATCH 2/8 floppy: remove unnecessary inlines

In reply to Stephen Hemminger

These routines are all big enough that is better to let the compiler decide to inline or not.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +7 -7
@@ -899,7 +899,7 @@ static int _lock_fdc(int drive, bool int
 	_lock_fdc(drive, interruptible, __LINE__)
 
 /* unlocks the driver */
-static inline void unlock_fdc(void)
+static void unlock_fdc(void)
 {
 	unsigned long flags;
 

@@ -1224,7 +1224,7 @@ static int need_more_output(void)
 /* Set perpendicular mode as required, based on data rate, if supported.
  * 82077 Now tested. 1Mbps data rate only possible with 82077-1.
  */
-static inline void perpendicular_mode(void)
+static void perpendicular_mode(void)
 {
 	unsigned char perp_mode;
 

@@ -3033,7 +3033,7 @@ static inline int fd_copyin(void __user 
 	return copy_from_user(address, param, size) ? -EFAULT : 0;
 }
 
-static inline const char *drive_name(int type, int drive)
+static const char *drive_name(int type, int drive)
 {
 	struct floppy_struct *floppy;
 

@@ -3103,7 +3103,7 @@ static struct cont_t raw_cmd_cont = {
 	.done		= raw_cmd_done
 };
 
-static inline int raw_cmd_copyout(int cmd, void __user *param,
+static int raw_cmd_copyout(int cmd, void __user *param,
 				  struct floppy_raw_cmd *ptr)
 {
 	int ret;

@@ -3148,7 +3148,7 @@ static void raw_cmd_free(struct floppy_r
 	}
 }
 
-static inline int raw_cmd_copyin(int cmd, void __user *param,
+static int raw_cmd_copyin(int cmd, void __user *param,
 				 struct floppy_raw_cmd **rcmd)
 {
 	struct floppy_raw_cmd *ptr;

@@ -3266,7 +3266,7 @@ static int invalidate_drive(struct block
 	return 0;
 }
 
-static inline int set_geometry(unsigned int cmd, struct floppy_struct *g,
+static int set_geometry(unsigned int cmd, struct floppy_struct *g,
 			       int drive, int type, struct block_device *bdev)
 {
 	int cnt;

@@ -3365,7 +3365,7 @@ static int ioctl_table[] = {
 	FDTWADDLE
 };
 
-static inline int normalize_ioctl(int *cmd, int *size)
+static int normalize_ioctl(int *cmd, int *size)
 {
 	int i;
author
#14 lore raw

PATCH 3/8 floppy: silence warning during disk test

In reply to Stephen Hemminger

The first thing the floppy does is read block 0 to test geometry and to test for disk presence. If disk is not present this causes a console warning message about failed I/O. Set flag to silence.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +1 -0
@@ -3829,6 +3829,7 @@ static int __floppy_read_block_0(struct 
 	bio.bi_size = size;
 	bio.bi_bdev = bdev;
 	bio.bi_sector = 0;
+	bio.bi_flags = BIO_QUIET;
 	init_completion(&complete);
 	bio.bi_private = &complete;
 	bio.bi_end_io = floppy_rb0_complete;
author
#15 lore raw

PATCH 4/8 floppy: use atomic type for usage_count

In reply to Stephen Hemminger

The usage_count was being protected by a lock which was only there to create an atomic counter.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +12 -30
@@ -578,7 +578,7 @@ static void reset_fdc(void);
 #define NEED_1_RECAL	-2
 #define NEED_2_RECAL	-3
 
-static int usage_count;
+static atomic_t usage_count = ATOMIC_INIT(0);
 
 /* buffer related variables */
 static int buffer_track = -1;

@@ -860,7 +860,7 @@ static void set_fdc(int drive)
 /* locks the driver */
 static int _lock_fdc(int drive, bool interruptible, int line)
 {
-	if (!usage_count) {
+	if (atomic_read(&usage_count) == 0) {
 		pr_err("Trying to lock fdc while usage count=0 at line %d\n",
 		       line);
 		return -1;

@@ -2941,7 +2941,7 @@ static void do_fd_request(struct request
 		return;
 	}
 
-	if (usage_count == 0) {
+	if (atomic_read(&usage_count) == 0) {
 		pr_info("warning: usage count=0, current_req=%p exiting\n",
 			current_req);
 		pr_info("sect=%ld type=%x flags=%x\n",

@@ -3858,7 +3858,7 @@ static int floppy_revalidate(struct gend
 	if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
 	    test_bit(FD_VERIFY_BIT, &UDRS->flags) ||
 	    test_bit(drive, &fake_change) || NO_GEOM) {
-		if (usage_count == 0) {
+		if (atomic_read(&usage_count) == 0) {
 			pr_info("VFS: revalidate called on non-open device.\n");
 			return -EFAULT;
 		}

@@ -4357,7 +4357,7 @@ out_unreg_platform_dev:
 	platform_device_unregister(&floppy_device[drive]);
 out_flush_work:
 	flush_scheduled_work();
-	if (usage_count)
+	if (atomic_read(&usage_count))
 		floppy_release_irq_and_dma();
 out_unreg_region:
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);

@@ -4374,8 +4374,6 @@ out_put_disk:
 	return err;
 }
 
-static DEFINE_SPINLOCK(floppy_usage_lock);
-
 static const struct io_region {
 	int offset;
 	int size;

@@ -4421,14 +4419,8 @@ static void floppy_release_regions(int f
 
 static int floppy_grab_irq_and_dma(void)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&floppy_usage_lock, flags);
-	if (usage_count++) {
-		spin_unlock_irqrestore(&floppy_usage_lock, flags);
+	if (atomic_inc_return(&usage_count) > 1)
 		return 0;
-	}
-	spin_unlock_irqrestore(&floppy_usage_lock, flags);
 
 	/*
 	 * We might have scheduled a free_irq(), wait it to

@@ -4439,9 +4431,7 @@ static int floppy_grab_irq_and_dma(void)
 	if (fd_request_irq()) {
 		DPRINT("Unable to grab IRQ%d for the floppy driver\n",
 		       FLOPPY_IRQ);
-		spin_lock_irqsave(&floppy_usage_lock, flags);
-		usage_count--;
-		spin_unlock_irqrestore(&floppy_usage_lock, flags);
+		atomic_dec(&usage_count);
 		return -1;
 	}
 	if (fd_request_dma()) {

@@ -4451,9 +4441,7 @@ static int floppy_grab_irq_and_dma(void)
 			use_virtual_dma = can_use_virtual_dma = 1;
 		if (!(can_use_virtual_dma & 1)) {
 			fd_free_irq();
-			spin_lock_irqsave(&floppy_usage_lock, flags);
-			usage_count--;
-			spin_unlock_irqrestore(&floppy_usage_lock, flags);
+			atomic_dec(&usage_count);
 			return -1;
 		}
 	}

@@ -4488,9 +4476,7 @@ cleanup:
 	fd_free_dma();
 	while (--fdc >= 0)
 		floppy_release_regions(fdc);
-	spin_lock_irqsave(&floppy_usage_lock, flags);
-	usage_count--;
-	spin_unlock_irqrestore(&floppy_usage_lock, flags);
+	atomic_dec(&usage_count);
 	return -1;
 }
 

@@ -4502,14 +4488,10 @@ static void floppy_release_irq_and_dma(v
 #endif
 	long tmpsize;
 	unsigned long tmpaddr;
-	unsigned long flags;
 
-	spin_lock_irqsave(&floppy_usage_lock, flags);
-	if (--usage_count) {
-		spin_unlock_irqrestore(&floppy_usage_lock, flags);
+	if (!atomic_dec_and_test(&usage_count))
 		return;
-	}
-	spin_unlock_irqrestore(&floppy_usage_lock, flags);
+
 	if (irqdma_allocated) {
 		fd_disable_dma();
 		fd_free_dma();

@@ -4602,7 +4584,7 @@ static void __exit floppy_module_exit(vo
 	del_timer_sync(&fd_timer);
 	blk_cleanup_queue(floppy_queue);
 
-	if (usage_count)
+	if (atomic_read(&usage_count))
 		floppy_release_irq_and_dma();
 
 	/* eject disk, if any */
author
#16 lore raw

PATCH 5/8 floppy: cmos attribute should be static

In reply to Stephen Hemminger

As reported by sparse, cmos attribute is local.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +1 -1
@@ -4127,7 +4127,7 @@ static ssize_t floppy_cmos_show(struct d
 	return sprintf(buf, "%X\n", UDP->cmos);
 }
 
-DEVICE_ATTR(cmos, S_IRUGO, floppy_cmos_show, NULL);
+static DEVICE_ATTR(cmos, S_IRUGO, floppy_cmos_show, NULL);
 
 static void floppy_device_release(struct device *dev)
 {
author
#17 lore raw

PATCH 6/8 floppy: fix signed/unsigned warnings

In reply to Stephen Hemminger

Ioctl cmd value is unsigned, so change normalize_ioctl

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +2 -2
@@ -3337,7 +3337,7 @@ static int set_geometry(unsigned int cmd
 }
 
 /* handle obsolete ioctl's */
-static int ioctl_table[] = {
+static unsigned int ioctl_table[] = {
 	FDCLRPRM,
 	FDSETPRM,
 	FDDEFPRM,

@@ -3365,7 +3365,7 @@ static int ioctl_table[] = {
 	FDTWADDLE
 };
 
-static int normalize_ioctl(int *cmd, int *size)
+static int normalize_ioctl(unsigned int *cmd, int *size)
 {
 	int i;
author
#18 lore raw

PATCH 7/8 floppy: use wait_event_interruptible

In reply to Stephen Hemminger

Convert wait loops to use wait_event_ macros.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +9 -50
@@ -514,8 +514,6 @@ static unsigned long fdc_busy;
 static DECLARE_WAIT_QUEUE_HEAD(fdc_wait);
 static DECLARE_WAIT_QUEUE_HEAD(command_done);
 
-#define NO_SIGNAL (!interruptible || !signal_pending(current))
-
 /* Errors during formatting are counted here. */
 static int format_errors;
 

@@ -858,36 +856,15 @@ static void set_fdc(int drive)
 }
 
 /* locks the driver */
-static int _lock_fdc(int drive, bool interruptible, int line)
+static int lock_fdc(int drive, bool interruptible)
 {
-	if (atomic_read(&usage_count) == 0) {
-		pr_err("Trying to lock fdc while usage count=0 at line %d\n",
-		       line);
+	if (WARN(atomic_read(&usage_count) == 0,
+		 "Trying to lock fdc while usage count=0\n"))
 		return -1;
-	}
 
-	if (test_and_set_bit(0, &fdc_busy)) {
-		DECLARE_WAITQUEUE(wait, current);
-		add_wait_queue(&fdc_wait, &wait);
-
-		for (;;) {
-			set_current_state(TASK_INTERRUPTIBLE);
-
-			if (!test_and_set_bit(0, &fdc_busy))
-				break;
-
-			schedule();
-
-			if (!NO_SIGNAL) {
-				remove_wait_queue(&fdc_wait, &wait);
-				return -EINTR;
-			}
-		}
+	if (wait_event_interruptible(fdc_wait, !test_and_set_bit(0, &fdc_busy)))
+		return -EINTR;
 
-		set_current_state(TASK_RUNNING);
-		remove_wait_queue(&fdc_wait, &wait);
-		flush_scheduled_work();
-	}
 	command_status = FD_COMMAND_NONE;
 
 	__reschedule_timeout(drive, "lock fdc");

@@ -895,9 +872,6 @@ static int _lock_fdc(int drive, bool int
 	return 0;
 }
 
-#define lock_fdc(drive, interruptible)			\
-	_lock_fdc(drive, interruptible, __LINE__)
-
 /* unlocks the driver */
 static void unlock_fdc(void)
 {

@@ -2015,25 +1989,10 @@ static int wait_til_done(void (*handler)
 
 	schedule_bh(handler);
 
-	if (command_status < 2 && NO_SIGNAL) {
-		DECLARE_WAITQUEUE(wait, current);
-
-		add_wait_queue(&command_done, &wait);
-		for (;;) {
-			set_current_state(interruptible ?
-					  TASK_INTERRUPTIBLE :
-					  TASK_UNINTERRUPTIBLE);
-
-			if (command_status >= 2 || !NO_SIGNAL)
-				break;
-
-			is_alive(__func__, "");
-			schedule();
-		}
-
-		set_current_state(TASK_RUNNING);
-		remove_wait_queue(&command_done, &wait);
-	}
+	if (interruptible)
+		wait_event_interruptible(command_done, command_status >= 2);
+	else
+		wait_event(command_done, command_status >= 2);
 
 	if (command_status < 2) {
 		cancel_activity();
author
#19 lore raw

PATCH 8/8 floppy: use warning macros

In reply to Stephen Hemminger

Convert assertions to use WARN(). There are several error checks in the code for things that should never happen. Convert them to standard warnings so kerneloops.org will see them.

Signed-off-by Stephen Hemminger <[email protected]>
drivers/block/floppy.c +11 -16
@@ -2532,10 +2532,8 @@ static int make_raw_rw_request(void)
 	int tracksize;
 	int ssize;
 
-	if (max_buffer_sectors == 0) {
-		pr_info("VFS: Block I/O scheduled on unopened device\n");
+	if (WARN(max_buffer_sectors == 0, "VFS: Block I/O scheduled on unopened device\n"))
 		return 0;
-	}
 
 	set_fdc((long)current_req->rq_disk->private_data);
 

@@ -2895,19 +2893,16 @@ static void process_fd_request(void)
 
 static void do_fd_request(struct request_queue *q)
 {
-	if (max_buffer_sectors == 0) {
-		pr_info("VFS: %s called on non-open device\n", __func__);
+	if (WARN(max_buffer_sectors == 0,
+		 "VFS: %s called on non-open device\n", __func__))
 		return;
-	}
 
-	if (atomic_read(&usage_count) == 0) {
-		pr_info("warning: usage count=0, current_req=%p exiting\n",
-			current_req);
-		pr_info("sect=%ld type=%x flags=%x\n",
-			(long)blk_rq_pos(current_req), current_req->cmd_type,
-			current_req->cmd_flags);
+	if (WARN(atomic_read(&usage_count) == 0,
+		 "warning: usage count=0, current_req=%p sect=%ld type=%x flags=%x\n",
+		 current_req, (long)blk_rq_pos(current_req), current_req->cmd_type,
+		 current_req->cmd_flags))
 		return;
-	}
+
 	if (test_bit(0, &fdc_busy)) {
 		/* fdc busy, this new request will be treated when the
 		   current one is done */

@@ -3817,10 +3812,10 @@ static int floppy_revalidate(struct gend
 	if (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
 	    test_bit(FD_VERIFY_BIT, &UDRS->flags) ||
 	    test_bit(drive, &fake_change) || NO_GEOM) {
-		if (atomic_read(&usage_count) == 0) {
-			pr_info("VFS: revalidate called on non-open device.\n");
+		if (WARN(atomic_read(&usage_count) == 0,
+			 "VFS: revalidate called on non-open device.\n"))
 			return -EFAULT;
-		}
+
 		lock_fdc(drive, false);
 		cf = (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||
 		      test_bit(FD_VERIFY_BIT, &UDRS->flags));
#20 lore raw
In reply to Stephen Hemminger

Am Mittwoch, den 09.06.2010, 11:34 -0700 schrieb Stephen Hemminger:

Quoted 3 lines

When going through floppy driver addressing some of the timing bugs, I also saw these leftover things that ought to be cleaned up.

With your patches applied on top of 2.6.34 I still get this timeout while resuming from ram:

[ 1086.971283] ata2.00: configured for UDMA/133
[ 1089.986024] 
[ 1089.986026] floppy driver state
[ 1089.986026] -------------------
[ 1089.986032] now=4295757282 last interrupt=4295281727 diff=475555 last called handler=main_command_interrupt
[ 1089.986033] timeout_message=lock fdc
[ 1089.986034] last output bytes:
[ 1089.986035]  2 90 4295192821
[ 1089.986036]  1 90 4295192821
[ 1089.986037]  d 90 4295192821
[ 1089.986038]  2 90 4295192821
[ 1089.986039]  e 90 4295192821
[ 1089.986040] 1b 90 4295192821
[ 1089.986041] ff 90 4295192821
[ 1089.986042]  f 80 4295281121
[ 1089.986043]  0 90 4295281121
[ 1089.986044]  0 91 4295281121
[ 1089.986045]  8 81 4295281129
[ 1089.986046] 45 80 4295281499
[ 1089.986047]  0 90 4295281499
[ 1089.986048]  0 90 4295281499
[ 1089.986049]  0 90 4295281499
[ 1089.986050]  3 90 4295281499
[ 1089.986051]  2 90 4295281499
[ 1089.986052]  4 90 4295281499
[ 1089.986053] 1b 90 4295281499
[ 1089.986054] ff 90 4295281499
[ 1089.986055] last result at 4295281741
[ 1089.986056] last redo_fd_request at 4295281741
[ 1089.986060] status=a
[ 1089.986061] fdc_busy=1
[ 1089.986063] do_floppy=reset_interrupt
[ 1089.986064] cont=ffffffffa0009ae0
[ 1089.986065] current_req=(null)
[ 1089.986066] command_status=-1
[ 1089.986067] 
[ 1089.986069] floppy0: floppy timeout called
[ 1089.986094] PM: resume of devices complete after 10536.229 msecs
[ 1089.986319] PM: resume devices took 10.537 seconds
[ 1089.986320] ------------[ cut here ]------------
[ 1089.986325] WARNING: at kernel/power/suspend_test.c:53 suspend_devices_and_enter+0xd2/0x220()
[ 1089.986326] Hardware name: MS-7250
[ 1089.986327] Component: resume devices, time: 10537
[ 1089.986328] Modules linked in: rndis_wlan olympic forcedeth floppy [last unloaded: scsi_wait_scan]
[ 1089.986333] Pid: 15510, comm: pm-suspend Not tainted 2.6.34 #1
[ 1089.986334] Call Trace:
[ 1089.986340]  [<ffffffff81063283>] ? warn_slowpath_common+0x73/0xb0
[ 1089.986343]  [<ffffffff81063320>] ? warn_slowpath_fmt+0x40/0x50
[ 1089.986345]  [<ffffffff81095132>] ? suspend_devices_and_enter+0xd2/0x220
[ 1089.986348]  [<ffffffff810953a8>] ? enter_state+0x128/0x150
[ 1089.986350]  [<ffffffff810949cd>] ? state_store+0x8d/0xf0
[ 1089.986353]  [<ffffffff81165215>] ? sysfs_write_file+0xd5/0x160
[ 1089.986356]  [<ffffffff811029e8>] ? vfs_write+0xb8/0x1a0
[ 1089.986360]  [<ffffffff810aff51>] ? audit_syscall_entry+0x1c1/0x1f0
[ 1089.986362]  [<ffffffff811032ce>] ? sys_write+0x4e/0x90
[ 1089.986365]  [<ffffffff81027e6b>] ? system_call_fastpath+0x16/0x1b
[ 1089.986367] ---[ end trace 210bc7fac7be9fbd ]---
[ 1089.986432] PM: Finishing wakeup.
[ 1089.986433] Restarting tasks ...

Is this okay?

author
#21 lore raw
Quoted 7 lines · On Thu, 10 Jun 2010 08:43:32 +0200 Thomas Meyer <[email protected]> wrote:

Am Mittwoch, den 09.06.2010, 11:34 -0700 schrieb Stephen Hemminger:

Quoted 3 lines

When going through floppy driver addressing some of the timing bugs, I also saw these leftover things that ought to be cleaned up.

With your patches applied on top of 2.6.34 I still get this timeout while resuming from ram:

Not clear, did the problem occur before my patches? The posted set of patches should not change visible functionalty or fix any observable bug. Floppy driver would be bug for bug the same.

#22 lore raw

----- Original Message ----- From: "Stephen Hemminger" <[email protected]>

Quoted 4 lines
Quoted 1 line · On Thu, 10 Jun 2010 08:43:32 +0200 Thomas Meyer <[email protected]> wrote:

Am Mittwoch, den 09.06.2010, 11:34 -0700 schrieb Stephen Hemminger:

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

When going through floppy driver addressing some of the timing bugs, I also saw these leftover things that ought to be cleaned up.

With your patches applied on top of 2.6.34 I still get this timeout while resuming from ram:

Not clear, did the problem occur before my patches?

Yes, the timeout occured before your patches, i.e. plain 2.6.34.

Quoted 3 lines

The posted set
of patches should not change visible functionalty or fix any observable bug. Floppy driver would be bug for bug the same.

I guess, it's okay then.

kind regards
thomas