lkml.org 
[lkml]   [2010]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [REGRESSION] cdrom drive doesn't detect removal
From
Date
On Tue, 2010-09-14 at 10:07 +0200, Kay Sievers wrote: 
> On Tue, Sep 14, 2010 at 09:39, Tejun Heo <tj@kernel.org> wrote:
> > On 09/14/2010 03:27 AM, Maxim Levitsky wrote:
> >>> However 2.6.36 doesn't detect that removal.
> >>> According to udevadm monitor --property no uevents are send on removal.
> >> Correction, this is regression between 2.6.34 and 2.6.35. This shows how
> >> much I use cd these days...
> >>
> >> I bisected it down to this:
> >>
> >> 6b4517a7913a09d3259bb1d21c9cb300f12294bd is the first bad commit
> >> commit 6b4517a7913a09d3259bb1d21c9cb300f12294bd
> >> Author: Tejun Heo <tj@kernel.org>
> >> Date: Wed Apr 7 18:53:59 2010 +0900
> >>
> >> block: implement bd_claiming and claiming block
> >
> > Hmmm... weird. This commit does change the open behavior but
> > shouldn't change the end result. Can someone please enlighten me how
> > udevadm is interacting with the device at system call level?
>
> Not at all. Udev does not really touch it.
>
> The cdrom drive isn't unlocked on usual systems, udisks polls the
> cdrom drive periodically just like HAL did. The only difference is
> that with the polling, the sr driver detects a media change and sends
> a uevent to udev, instead of HAL looking at the result of the open().
>
> Are we sure, that there is something that still polls the drive?
> Udisks is only auto-started, when the desktop calls into some D-Bus
> methods, unlike HAL where it was an init script.


Note that on current vanilla head, I applied the attached manual revert
patch, and problem disappears (and probably introduces the same bug that
patch supposed to fix).


Best regards,
Maxim Levitsky
commit 6256435b81d05c98be64fbeea57776520961a10c
Author: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Tue Sep 14 23:47:14 2010 +0200

block_dev: revert cdrom breakage

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 50e8c85..f1095f3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -839,14 +839,15 @@ static void bd_finish_claiming(struct block_device *bdev,
int bd_claim(struct block_device *bdev, void *holder)
{
struct block_device *whole = bdev->bd_contains;
- int res;
+ int res = -EBUSY;

might_sleep();

spin_lock(&bdev_lock);
- res = bd_prepare_to_claim(bdev, whole, holder);
- if (res == 0)
+ if (bd_may_claim(bdev, whole, holder)) {
+ res = 0;
__bd_claim(bdev, whole, holder);
+ }
spin_unlock(&bdev_lock);

return res;
@@ -1462,7 +1463,6 @@ EXPORT_SYMBOL(blkdev_get);

static int blkdev_open(struct inode * inode, struct file * filp)
{
- struct block_device *whole = NULL;
struct block_device *bdev;
int res;

@@ -1485,24 +1485,24 @@ static int blkdev_open(struct inode * inode, struct file * filp)
if (bdev == NULL)
return -ENOMEM;

+ filp->f_mapping = bdev->bd_inode->i_mapping;
+
+ res = blkdev_get(bdev, filp->f_mode);
+ if (res)
+ return res;
+
if (filp->f_mode & FMODE_EXCL) {
- whole = bd_start_claiming(bdev, filp);
- if (IS_ERR(whole)) {
- bdput(bdev);
- return PTR_ERR(whole);
- }
+ res = bd_claim(bdev, filp);
+ if (res)
+ goto out_blkdev_put;
}

- filp->f_mapping = bdev->bd_inode->i_mapping;
+ return 0;

- res = blkdev_get(bdev, filp->f_mode);
+ out_blkdev_put:
+ blkdev_put(bdev, filp->f_mode);
+ return res;

- if (whole) {
- if (res == 0)
- bd_finish_claiming(bdev, whole, filp);
- else
- bd_abort_claiming(whole, filp);
- }

return res;
}
@@ -1712,34 +1712,30 @@ EXPORT_SYMBOL(lookup_bdev);
*/
struct block_device *open_bdev_exclusive(const char *path, fmode_t mode, void *holder)
{
- struct block_device *bdev, *whole;
- int error;
+ struct block_device *bdev;
+ int error = 0;

bdev = lookup_bdev(path);
if (IS_ERR(bdev))
return bdev;

- whole = bd_start_claiming(bdev, holder);
- if (IS_ERR(whole)) {
- bdput(bdev);
- return whole;
- }
-
error = blkdev_get(bdev, mode);
if (error)
- goto out_abort_claiming;
+ return ERR_PTR(error);

error = -EACCES;
if ((mode & FMODE_WRITE) && bdev_read_only(bdev))
goto out_blkdev_put;
+ error = bd_claim(bdev, holder);
+ if (error)
+ goto blkdev_put;
+

bd_finish_claiming(bdev, whole, holder);
return bdev;

out_blkdev_put:
blkdev_put(bdev, mode);
-out_abort_claiming:
- bd_abort_claiming(whole, holder);
return ERR_PTR(error);
}
\
 
 \ /
  Last update: 2010-09-15 01:41    [W:0.086 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site