All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@vyatta.com>
To: Jiri Kosina <jkosina@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jens Axboe <axboe@kernel.dk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] floppy: convert to delayed work and single-thread wq
Date: Wed, 16 May 2012 07:57:17 -0700	[thread overview]
Message-ID: <20120516075717.18402f00@nehalam.linuxnetplumber.net> (raw)
In-Reply-To: <alpine.LNX.2.00.1205160932280.8796@pobox.suse.cz>

On Wed, 16 May 2012 09:36:51 +0200 (CEST)
Jiri Kosina <jkosina@suse.cz> wrote:

> There are several races in floppy driver between bottom half 
> (scheduled_work) and timers (fd_timeout, fd_timer). Due to slowness of the 
> actual floppy devices, those races are never (at least to my knowledge) 
> triggered on a bare floppy metal. However on virtualized (emulated) floppy 
> drives, which are of course magnitudes faster than the real ones, these 
> races trigger reliably. They usually exhibit themselves as NULL pointer 
> dereferences during DMA setup, such as
> 
> 	BUG: unable to handle kernel NULL pointer dereference at 0000000a
> 	[ ... snip ... ]
> 	EIP: 0060:[<c02053d5>] EFLAGS: 00010293 CPU: 0
> 	EAX: ffffe000 EBX: 0000000a ECX: 00000000 EDX: 0000000a
> 	ESI: c05d2718 EDI: 00000000 EBP: 00000000 ESP: f540fe44
> 	 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> 	Process swapper (pid: 0, ti=f540e000 task=c082d5a0 task.ti=c0826000)
> 	Stack:
> 	 ffffe000 00001ffc 00000000 00000000 00000000 c05d2718 c0708b40 f540fe80
> 	 c020470f c05d2718 c0708b40 00000000 f540fe80 0000000a f540fee4 00000000
> 	 c0708b40 f540fee4 00000000 00000000 c020526b 00000000 c05d2718 c0708b40
> 	Call Trace:
> 	 [<c020470f>] dump_trace+0xaf/0x110
> 	 [<c020526b>] show_trace_log_lvl+0x4b/0x60
> 	 [<c0205298>] show_trace+0x18/0x20
> 	 [<c05c5811>] dump_stack+0x6d/0x72
> 	 [<c0248527>] warn_slowpath_common+0x77/0xb0
> 	 [<c02485f3>] warn_slowpath_fmt+0x33/0x40
> 	 [<f7ec593c>] setup_DMA+0x14c/0x210 [floppy]
> 	 [<f7ecaa95>] setup_rw_floppy+0x105/0x190 [floppy]
> 	 [<c0256d08>] run_timer_softirq+0x168/0x2a0
> 	 [<c024e762>] __do_softirq+0xc2/0x1c0
> 	 [<c02042ed>] do_softirq+0x7d/0xb0
> 	 [<f54d8a00>] 0xf54d89ff
> 
> but other instances can be easily seen as well. This can be observed at 
> least under VMWare, VirtualBox and KVM.
> 
> This patch converts all the timers and bottom halfs to be processed in a 
> single workqueue. This aproach has been already discussed back in 2010 and 
> Acked by Linus [1], but it then never made it to the tree.
> 
> This all is based on original idea and code of Stephen Hemminger.  I have 
> ported original Stepen's code to the current state of the floppy driver, 
> and performed quite some testing (on real hardware), which didn't reveal 
> any issues (this includes not only writing and reading data, but also 
> formatting (unfortunately I didn't find any Double-Density disks any 
> more)). Ability to handle errors properly (supplying known bad floppies) 
> has also been verified.
> 
> [1] http://kerneltrap.org/mailarchive/linux-kernel/2010/6/11/4582092
> 
> Based-on-patch-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Thanks, the hold up for me was always finding real working floppy
drive to test.


  reply	other threads:[~2012-05-16 14:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-16  7:36 [PATCH] floppy: convert to delayed work and single-thread wq Jiri Kosina
2012-05-16 14:57 ` Stephen Hemminger [this message]
2012-05-16 15:25 ` Linus Torvalds
2012-05-16 17:01 ` Tejun Heo
2012-05-16 19:37   ` Jiri Kosina
2012-05-16 19:43     ` Linus Torvalds
2012-05-16 19:47     ` Linus Torvalds
2012-05-16 19:51       ` Jiri Kosina
2012-05-16 19:53       ` Tejun Heo
2012-05-16 19:57         ` Jiri Kosina
2012-05-16 20:01           ` Tejun Heo
2012-05-16 20:24             ` Jiri Kosina
2012-05-16 20:29               ` Tejun Heo
2012-05-16 20:42                 ` Linus Torvalds
2012-05-17 14:55                   ` Tejun Heo
2012-05-17 15:03                     ` Linus Torvalds
2012-05-17 15:06                     ` Jiri Kosina
2012-05-17 15:09                       ` Tejun Heo
2012-05-17 15:46                         ` Jiri Kosina
2012-05-17 16:02                           ` Tejun Heo
2012-05-16 20:44                 ` Jiri Kosina
2012-05-17 15:06                   ` Tejun Heo
2012-05-17  7:56                 ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120516075717.18402f00@nehalam.linuxnetplumber.net \
    --to=shemminger@vyatta.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.