mirror of https://lore.kernel.org/lkml/
 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome