Bug 13029
Summary: | media revalidation bug on USB card reader | ||
---|---|---|---|
Product: | IO/Storage | Reporter: | David Zeuthen (zeuthen) |
Component: | Block Layer | Assignee: | Jens Axboe (axboe) |
Status: | CLOSED CODE_FIX | ||
Severity: | normal | CC: | alan, bugzilla, florian, kay, martin.pitt, tj |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 2.6.38 | Subsystem: | |
Regression: | No | Bisected commit-id: | |
Attachments: |
cdrom_open-always-check-events.patch
blkdev-get-rescan.patch |
Description
David Zeuthen
2009-04-06 23:04:03 UTC
Touch intentionally avoids blocking or testing things like media state (note its use of O_NONBLOCK) Hi Alan, (In reply to comment #1) > Touch intentionally avoids blocking or testing things like media state (note > its use of O_NONBLOCK) I'm not sure what you are saying here as comment 0 shows that it is the touch call (with O_NONBLOCK) that actually makes the kernel remove sdb1 while earlier calls open() calls (without O_NONBLOCK) does not trigger this (but makes the kernel return ENOMEDIUM). What baffles me is that in order for the kernel to return ENOMEDIUM it must know there is no medium. Yet it refuses to remove the partitions on the medium until O_NONBLOCK is used. How do you explain that not being buggy behavior? Of course we can easily work around this in user space by just essentially running 'touch /dev/sdb' when we get ENOMEDIUM but at the end of the day I still think that is a work around. ah Ok I misunderstood the original report With O_NDELAY we go direct to the device level status Without it we go via open_for_data() which checks the drive_status and does other stuff as well so its definitely not sane unless something else is holding the partition open and this is a timing funny. Can you rebuild drivers/cdrom/cdrom.c with the ERRLOGMASK define near the top set to log everything as a first step, then see what that shows up. (In reply to comment #3) > Can you rebuild drivers/cdrom/cdrom.c with the ERRLOGMASK define near the top > set to log everything as a first step, then see what that shows up. Sure, but the bug only happens with a USB 4in1 card reader driven by usb-storage so maybe I need to tweak another define than in cdrom.c? The sd case is a bit trickier but if you stick a few printk() calls into sd.c : sd_open/sd_close and sd_revalidate_disk so you can see the events occuring maybe that will produce a clue. I just stumbled over this as well. We currently try to remove the polling in udisks to make use of the new in-kernel media polling facility (https://lkml.org/lkml/2010/12/8/299). My SD card reader (which is actually a Sony PRS e-book reader) behaves exactly like David's. When I eject the card from the reader, no uevents happen until I poke the drive with an open(): $ sudo head -c 1 /dev/sdd head: cannot open `/dev/sdd' for reading: No medium found KERNEL[1302016466.795990] change /devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.5/1-1.5.3/1-1.5.3.3/1-1.5.3.3:1.0/host19/target19:0:0/19:0:0:2/block/sdd (block) UDEV_LOG=3 ACTION=change DISK_MEDIA_CHANGE=1 Note there is no removal event for /dev/sdd1, and the device is still around: $ ls /dev/sdd* /dev/sdd /dev/sdd1 * The current udisks poller works around this by doing the poke again with O_NONBLOCK; when doing this, sdd1 actually goes away: $ sudo python -c 'import os; os.open("/dev/sdd", os.O_RDONLY|os.O_NONBLOCK)' KERNEL[1302017728.645882] remove /devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.5/1-1.5.3/1-1.5.3.3/1-1.5.3.3:1.0/host21/target21:0:0/21:0:0:2/block/sdd/sdd1 (block) ACTION=remove DEVTYPE=partition This behaves exactly the same when enabling the in-kernel polling in /sys/block/sdd/events_poll_msecs; when doing that, the "head" command (which is by and large doing open() without O_NONBLOCK here) does not need to be run, but the last python O_NONBLOCK poke is still required. Could the in-kernel poller do the equivalent of this when a DISK_MEDIA_CHANGE uevent is generated, i. e. take into account that partitions might have been removed/added? I. e. the equivalent of what opening the block device with O_NONBLOCK does? If this could be fixed in the in-kernel poller, we could remove the userspace workaround to open(O_NONBLOCK) the device on a DISK_MEDIA_CHANGE event. Tejun, the issue with the in-kernel polling and media re-validation seems like an old bug -- would be still nice to fix that, so userspace does not need to work around that weird behavior. Any ideas? Created attachment 53632 [details]
cdrom_open-always-check-events.patch
Can you please verify the attached patch fixes the problem? Thanks.
Created attachment 53652 [details]
blkdev-get-rescan.patch
For some reason I thought this report was about cdrom. We actually have two layers of problems here. For cdrom, the event check itself doesn't happen if medimum is not there. For other devices, that part happens but partition rescan doesn't. This patch should fix the latter. The patch is only compile tested. Please test whether it fixes the problem.
Thanks.
On Wed, Apr 6, 2011 at 13:48, <bugzilla-daemon@bugzilla.kernel.org> wrote: > --- Comment #9 from Tejun Heo <tj@kernel.org> 2011-04-06 11:48:53 --- > Created an attachment (id=53652) > --> (https://bugzilla.kernel.org/attachment.cgi?id=53652) > blkdev-get-rescan.patch > > For some reason I thought this report was about cdrom. We actually have two > layers of problems here. For cdrom, the event check itself doesn't happen if > medimum is not there. For other devices, that part happens but partition > rescan doesn't. This patch should fix the latter. The patch is only compile > tested. Please test whether it fixes the problem. That fixes the issue for me, and works without weird added additional open(O_NONBLOCK) workarounds. Thanks! On Wed, Apr 6, 2011 at 14:10, <bugzilla-daemon@bugzilla.kernel.org> wrote: > That fixes the issue for me, and works without weird added additional > open(O_NONBLOCK) workarounds. Feel free to add: Tested-By: Kay Sievers <kay.sievers@vrfy.org> to all 3 patches. I'm running them all here with the in-development version of udisks, that switched completely from userspace to kernel polling. A patch referencing this bug report has been merged in v2.6.39: commit 02e352287a40bd456eb78df705bf888bc3161d3f Author: Tejun Heo <tj@kernel.org> Date: Fri Apr 29 10:15:20 2011 +0200 block: rescan partitions on invalidated devices on -ENOMEDIA too So I close this bug as fixed assuming it fixes this also for David, the original reporter. A patch referencing this bug report has been merged in v3.0-rc1: commit 1196f8b814f32cd04df334abf47648c2a9fd8324 Author: Tejun Heo <tj@kernel.org> Date: Thu Apr 21 20:54:45 2011 +0200 block: rescan partitions on invalidated devices on -ENOMEDIA too |