Due to recent changes to usb_reset_device, the following hang occurs:
events/0 D 0000000000000000 0 6 2
ffff880037477cc0 0000000000000046 ffff880037477c50 ffffffff80237434
ffffffff80574c80 00000001000a015c 0000000000000286 ffff8800374757d0
ffff88002a31c860 ffff880037475a00 0000000036779140 ffff880037475a00
Call Trace:
[<ffffffff80237434>] try_to_del_timer_sync+0x52/0x5b
[<ffffffff8026f86c>] dma_pool_free+0x1a7/0x1ec
[<ffffffffa02a928a>] ub_disconnect+0x8e/0x1ad [ub]
[<ffffffff802407c9>] autoremove_wake_function+0x0/0x2e
[<ffffffff80378959>] usb_unbind_interface+0x5c/0xb7
[<ffffffff8036ab70>] __device_release_driver+0x95/0xbd
[<ffffffff8036ac70>] device_release_driver+0x21/0x2d
[<ffffffff803789f8>] usb_driver_release_interface+0x44/0x83
[<ffffffff80378ab9>] usb_forced_unbind_intf+0x17/0x1d
[<ffffffff80371ba4>] usb_reset_device+0x7d/0x114
[<ffffffffa02aaffd>] ub_reset_task+0x0/0x293 [ub]
[<ffffffffa02ab1c1>] ub_reset_task+0x1c4/0x293 [ub]
[<ffffffff8033dd1e>] flush_to_ldisc+0x0/0x1cd
[<ffffffffa02aaffd>] ub_reset_task+0x0/0x293 [ub]
[<ffffffff8023d302>] run_workqueue+0x87/0x114
[<ffffffff8023d467>] worker_thread+0xd8/0xe7
[<ffffffff802407c9>] autoremove_wake_function+0x0/0x2e
[<ffffffff8023d38f>] worker_thread+0x0/0xe7
[<ffffffff802404c1>] kthread+0x47/0x73
[<ffffffff8022c8dd>] schedule_tail+0x27/0x60
[<ffffffff8020c249>] child_rip+0xa/0x11
[<ffffffff8024047a>] kthread+0x0/0x73
[<ffffffff8020c23f>] child_rip+0x0/0x11
This is because usb_reset_device now unbinds, and that calls disconnect,
which in case of ub waits until the reset completes... which deadlocks.
Worse, this deadlocks keventd and this takes whole box down.
I'm going to fix this properly later, but let's unbreak the driver
quickly for non-composite devices at least.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
To keep the size of changesets sane we split the switch by drivers;
to keep the damn thing bisectable we do the following:
1) rename the affected methods, add ones with correct
prototypes, make (few) callers handle both. That's this changeset.
2) for each driver convert to new methods. *ALL* drivers
are converted in this series.
3) kill the old (renamed) methods.
Note that it _is_ a flagday; all in-tree drivers are converted and by the
end of this series no trace of old methods remain. The only reason why
we do that this way is to keep the damn thing bisectable and allow per-driver
debugging if anything goes wrong.
New methods:
open(bdev, mode)
release(disk, mode)
ioctl(bdev, mode, cmd, arg) /* Called without BKL */
compat_ioctl(bdev, mode, cmd, arg)
locked_ioctl(bdev, mode, cmd, arg) /* Called with BKL, legacy */
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
I hoped to continue to ignore this problem or use libusual, but these
days it's simpler to work around than to deal with it. Let's attempt to
use bad residue devices and hope that upper level integrity checks catch
any problems (e.g. please use sha1sum on your backups).
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
The wodim says:
"close track/session scsi sendcmd: cmd timeout after 5.000 (480) s"
This happened because we ignored the supplied timeout and used 5s.
It's not completely correct to apply a timeout meant for the complete
command to any single URB, but we don't have many URBs per command, so
this is simple and works.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
We can save some atomic ops in the IO path, if we clearly define
the rules of how to modify the queue flags.
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
When __blk_end_request returns nonzero, it means that the request was
not completely processed and some BIOs are still attached. Since we
have dequeued it by that time, it means leaking requests and hanging
processes, which is why BUG() was in there. In ub this happens if
a packet request ends normally, but with residue (e.g. when scsi_id
issues INQUIRY).
The fix is to make sure that arguments passed to __blk_end_request
are correct: the full request length and not just transferred length.
The transferred length is indicated to applications by adjusting
rq->data_len with old, unchanged code outside of this patch.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Cc: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Cc: Greg KH <greg@kroah.com>
Cc: Boaz Harrosh <bharrosh@panasas.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
These zeroings were taken from usb-storage long time ago. I examined
the submission paths and usb_fill_bulk_urb and found them unnecessary.
Signed-off-by: Pete Zaitcev <zaitcev@yahoo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch converts ub to use blk_end_request interfaces.
Related 'uptodate' arguments are converted to 'error'.
Cc: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
Most drivers need to set length and offset as well, so may as well fold
those three lines into one.
Add sg_assign_page() for those two locations that only needed to set
the page, where the offset/length is set outside of the function context.
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
Some of the code has been gradually transitioned to using the proper
struct request_queue, but there's lots left. So do a full sweet of
the kernel and get rid of this typedef and replace its uses with
the proper type.
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
bsg uses scsi_cmd_ioctl() for some SCSI/sg ioctl
commands. scsi_cmd_ioctl() gets a request queue from a gendisk
arguement. This prevents bsg being bound to SCSI devices that don't
have a gendisk (like OSD). This adds a request_queue argument to
scsi_cmd_ioctl(). The SCSI/sg ioctl commands doesn't use a gendisk so
it's safe for any SCSI devices to use scsi_cmd_ioctl().
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
The usb-storage switched to binding to first endpoint recently. Apparently,
there are devices out there with extra endpoints. It is perfectly legal.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Maintain a per-CPU global "struct pt_regs *" variable which can be used instead
of passing regs around manually through all ~1800 interrupt handlers in the
Linux kernel.
The regs pointer is used in few places, but it potentially costs both stack
space and code to pass it around. On the FRV arch, removing the regs parameter
from all the genirq function results in a 20% speed up of the IRQ exit path
(ie: from leaving timer_interrupt() to leaving do_IRQ()).
Where appropriate, an arch may override the generic storage facility and do
something different with the variable. On FRV, for instance, the address is
maintained in GR28 at all times inside the kernel as part of general exception
handling.
Having looked over the code, it appears that the parameter may be handed down
through up to twenty or so layers of functions. Consider a USB character
device attached to a USB hub, attached to a USB controller that posts its
interrupts through a cascaded auxiliary interrupt controller. A character
device driver may want to pass regs to the sysrq handler through the input
layer which adds another few layers of parameter passing.
I've build this code with allyesconfig for x86_64 and i386. I've runtested the
main part of the code on FRV and i386, though I can't test most of the drivers.
I've also done partial conversion for powerpc and MIPS - these at least compile
with minimal configurations.
This will affect all archs. Mostly the changes should be relatively easy.
Take do_IRQ(), store the regs pointer at the beginning, saving the old one:
struct pt_regs *old_regs = set_irq_regs(regs);
And put the old one back at the end:
set_irq_regs(old_regs);
Don't pass regs through to generic_handle_irq() or __do_IRQ().
In timer_interrupt(), this sort of change will be necessary:
- update_process_times(user_mode(regs));
- profile_tick(CPU_PROFILING, regs);
+ update_process_times(user_mode(get_irq_regs()));
+ profile_tick(CPU_PROFILING);
I'd like to move update_process_times()'s use of get_irq_regs() into itself,
except that i386, alone of the archs, uses something other than user_mode().
Some notes on the interrupt handling in the drivers:
(*) input_dev() is now gone entirely. The regs pointer is no longer stored in
the input_dev struct.
(*) finish_unlinks() in drivers/usb/host/ohci-q.c needs checking. It does
something different depending on whether it's been supplied with a regs
pointer or not.
(*) Various IRQ handler function pointers have been moved to type
irq_handler_t.
Signed-Off-By: David Howells <dhowells@redhat.com>
(cherry picked from 1b16e7ac850969f38b375e511e3fa2f474a33867 commit)
The command "cdrecord dev=/dev/uba x.iso" prints nasty garbage if a blank
is not in the drive. This happens because drivers have to set req->errors
separately from just returning zero uptodate with end_that_request_first,
end_that_request_last. These functions only set error in BIO, but sg_io()
ignores it.
Since we're on it, let cdrecord access a device when ->changed is set.
It's useful if someone wants to know device capabilities without burning
anything.
Signed-Off-By: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Remove some silly messages and cast in stone "temporary" messages which
we keep around. Also, I am hesitant to remove the initialization retries
without having the hardware to test (anyone who was at KS04 has a spare?)
Signed-off-by: Pete Zaitcev <zaitcev@yahoo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
<zaitcev> I am taling about this: "if (disk->flags & GENHD_FL_UP) del_gendisk(disk);"
<zaitcev> If del_gendisk() undoes add_disk() like viro just said, why is it conditional?
<viro> huh?
<viro> add_disk() sets the damn flag
<zaitcev> So, I should not need to check ever
<viro> so the above is "if I've called add_disk(), call gendisk()"
<viro> which might be what you want, of course
<viro> but usually you know if you'd done add_disk() on that puppy anyway
In ub, nobody upstream should ever see half-constructed disks before
they were passed to add_disk. To that end, only add the struct lun to
the list on the path of no return. With that fix in place, we do
not need to test GENHD_FL_UP.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Remove the check for NULL which makes no sense. Suggested by Al.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
In kernel 2.6.16, if a mounted storage device is removed, an oops happens
because ub supplies an interface device (and kobject) to the block layer,
but neglects to pin it. And apparently, the block layer expects its users
to pin device structures.
The code in ub was broken this way for years. But the bug was exposed only
by 2.6.16 when it started to call block_uevent on close, which traverses
device structures (kobjects actually).
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Matt mentioned that a very old ZIP-100 actually does need this, but I am
yet to see anyone who actually has one still working and uses ub with it.
He/she must be a retrocomputing geek, who can easily bias it to usb-storage
with libusual, if needed. Meanwhile, common folks have trouble with poorly
designed USB keys and some el-cheapo European music players. I think we
better drop this for now.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Remove the "diag" file from the sysfs. The usbmon is good enough these days
so I do not need this feature anymore. Also, sysfs is a pain. Al Viro caught
a race in this, which I thought too bothersome to fix.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
The first_open was long overdue for removal, but I wanted to keep this
separate for other changes in case of regressions.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
For crying out loud, they have devices which do not like port resets.
So, do what usb-storage does and try both bulk and port resets.
We start with a port reset (which usb-storage does at the end of transport),
then do a Bulk reset, then a port reset again. This seems to work for me.
The code is getting dirtier and dirtier here, but I swear that I'll
do something about it (see those two new XXX). Honest.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
If SCSI commands are submitted while other commands are still processed,
the dispatch loop turns, and we stop the work_timer. Then, if URB fails
to complete, ub hangs until the device is unplugged.
This does not happen often, becase we only allow one SCSI command per
block device, but does happen (on multi-LUN devices, for example).
The fix is to stop timer only when we actually going to change the state.
The nicest code would be to have the timer stopped in URB callback, but
this is impossible, because it can be called from inside a timer, through
the urb_unlink. Then we get BUG in timer.c:cascade(). So, we do it a
little dirtier.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
The blk_cleanup_queue does not necesserily destroy the queue. When we
destroy the corresponding ub_dev, it may leave the queue spinlock pointer
dangling.
This patch moves spinlocks from ub_dev to static memory. The locking
scheme is not changed. These spinlocks are still separate from the ub_lock.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
add @uptodate argument to end_that_request_last() and @error
to rq_end_io_fn(). there's no generic way to pass error code
to request completion function, making generic error handling
of non-fs request difficult (rq->errors is driver-specific and
each driver uses it differently). this patch adds @uptodate
to end_that_request_last() and @error to rq_end_io_fn().
for fs requests, this doesn't really matter, so just using the
same uptodate argument used in the last call to
end_that_request_first() should suffice. imho, this can also
help the generic command-carrying request jens is working on.
Signed-off-by: tejun heo <htejun@gmail.com>
Signed-Off-By: Jens Axboe <axboe@suse.de>
Implement command retries and resets in ub. It is advantageous for users
to know if their devices are getting bad. However, failing every I/O
is not practical if you have a external USB enclosure with a hard drive.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch adds a shim driver libusual, which routes devices between
usb-storage and ub according to the common table, based on unusual_devs.h.
The help and example syntax is in Kconfig.
Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
When building on a 64-bit platform, gcc produces a warning
"cast of a pointer to an integer of a different size".
The scatterlist.offset on the LHS is unsigned int, so I used
that originally.
Signed-off-by: Pete Zaitcev <zaitcev@yahoo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/block/ub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
This code appears to be more trouble than it's worth, considering that
no normal users reload drivers. So, we comment it for now. It is not
removed outright for the benefit of hackers (that is, myself).
Signed-off-by: Pete Zaitcev <zaitcev@yahoo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
This patch fixes a few problems with ub and cleans up a couple of things:
- Bump UB_MAX_REQ_SG, this allows to burn CDs
- Drop initialization of urb.transfer_flags,
now that URB_UNLINK_ASYNC is gone
- Add forgotten processing of stalls at GetMaxLUN
- Remove a few more P3-tagged printks whose time has come
- Correct comment about ZIP-100
Signed-off-by: Pete Zaitcev <zaitcev@yahoo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
drivers/block/ub.c | 53 +++++++++++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 26 deletions(-)
29 July 2005, Cambridge, MA:
This afternoon Alan Stern submitted a patch to remove the URB_ASYNC_UNLINK
flag from the Linux kernel. Mr. Stern explained, "This flag is a relic
from an earlier, less-well-designed system. For over a year it hasn't
been used for anything other than printing warning messages."
An anonymous spokesman for the Linux kernel development community
commented, "This is exactly the sort of thing we see happening all the
time. As the kernel evolves, support for old techniques and old code can
be jettisoned and replaced by newer, better approaches. Proprietary
operating systems do not have the freedom or flexibility to change so
quickly."
Mr. Stern, a staff member at Harvard University's Rowland Institute who
works on Linux only as a hobby, noted that the patch (labelled as548) did
not update two files, keyspan.c and option.c, in the USB drivers' "serial"
subdirectory. "Those files need more extensive changes," he remarked.
"They examine the status field of several URBs at times when they're not
supposed to. That will need to be fixed before the URB_ASYNC_UNLINK flag
is removed."
Greg Kroah-Hartman, the kernel maintainer responsible for overseeing all
of Linux's USB drivers, did not respond to our inquiries or return our
calls. His only comment was "Applied, thanks."
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
