From: Anton Vorontsov <cbou@mail.ru>
To: kernel-discuss@handhelds.org
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] genhd fix or ide workaround -- choose one
Date: Thu, 19 Oct 2006 02:15:06 +0400 [thread overview]
Message-ID: <20061018221506.GA4187@localhost> (raw)
[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]
Hi all,
I've caught deadlock inside IDE layer using IDE-CS: after accessing
to IDE disk placed in PCMCIA (CF card really), it will never probe
again after pulling it from PCMCIA/CF.
The kernel is stuck at
drivers/ide/ide.c:ide_unregister():604:
602 spin_unlock_irq(&ide_lock);
603 device_unregister(&drive->gendev);
604 wait_for_completion(&drive->gendev_rel_comp);
605 spin_lock_irq(&ide_lock);
ide_unregister() assumes that device_unregister() will call
ide-probe.c:drive_release_dev():
1282 static void drive_release_dev (struct device *dev)
1283 {
.... [...]
1302 complete(&drive->gendev_rel_comp);
1303 }
1311 static void init_gendisk (ide_hwif_t *hwif)
1312 {
....
1323 drive->gendev.release = drive_release_dev;
....
1333 }
But release() function will not called because drive->gendev is still
referenced inside genhd layer.
I'm attaching two patches: the fix and the workaround. I assume that
first is a candidate to -mm, and workaround is a temporary solution.
There are two set of patches: for the Linus's git tree and for the
handhelds.org's 2.6.17-hh1 (prefixed with hh-).
Note: this is cross-posting, thus please keep To: and CC: headers, to
keep us all in sync.
Thanks,
-- Anton (irc: bd2)
[-- Attachment #2: genhd-fix.patch --]
[-- Type: text/plain, Size: 1524 bytes --]
From: Anton Vorontsov <cbou@mail.ru>
No need to get_device(driverfs_dev), because it's already gotten by
add_disk(). And no function will call put_device() second time.
This is the source of deadlock[1] in IDE layer. SCSI workaround it by
calling put_device() themself after put_disk(), thus it should reflect
this change by removing put_device().
[1] deadlock in ide_unregister()
at wait_for_completion(&drive->gendev_rel_comp) after
device_unregister(&drive->gendev). That happens by reason of
ide-probe.c:drive_release_dev() not called because of driverfs_dev
(which is drive->gendev) still referenced by add_disk().
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 6fb4b61..8371adf 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -378,7 +378,7 @@ static char *make_block_name(struct gend
static int disk_sysfs_symlinks(struct gendisk *disk)
{
- struct device *target = get_device(disk->driverfs_dev);
+ struct device *target = disk->driverfs_dev;
int err;
char *disk_name = NULL;
@@ -414,9 +414,8 @@ err_out_dev_link:
sysfs_remove_link(&disk->kobj, "device");
err_out_disk_name:
kfree(disk_name);
-err_out:
- put_device(target);
}
+err_out:
return err;
}
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 84ff203..03dca98 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1760,7 +1760,6 @@ static void scsi_disk_release(struct cla
disk->private_data = NULL;
put_disk(disk);
- put_device(&sdkp->device->sdev_gendev);
kfree(sdkp);
}
[-- Attachment #3: ide-genhd-workaround.patch --]
[-- Type: text/plain, Size: 1501 bytes --]
From: Anton Vorontsov <cbou@mail.ru>
Because add_disk() getting driverfs_dev twice but putting it only once,
ide-{disk,cd,floppy} should put_device() themselfs. This is workaround
for the bug found in genhd, which causes deadlock in ide_unregister()
at wait_for_completion(&drive->gendev_rel_comp) after
device_unregister(&drive->gendev). That happens by reason of
ide-probe.c:drive_release_dev() not called because of driverfs_dev
(which is drive->gendev) still referenced by add_disk().
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index bddfebd..59121a3 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -3307,6 +3307,7 @@ static void ide_cd_release(struct kref *
blk_queue_prep_rq(drive->queue, NULL);
g->private_data = NULL;
put_disk(g);
+ put_device(&drive->gendev);
kfree(info);
}
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index 0a05a37..d5d02a1 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -1020,6 +1020,7 @@ static void ide_disk_release(struct kref
drive->driver_data = NULL;
g->private_data = NULL;
put_disk(g);
+ put_device(&drive->gendev);
kfree(idkp);
}
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 8ccee9c..7a60d8c 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -1887,6 +1887,7 @@ static void ide_floppy_release(struct kr
drive->driver_data = NULL;
g->private_data = NULL;
put_disk(g);
+ put_device(&drive->gendev);
kfree(floppy);
}
[-- Attachment #4: hh-genhd-fix.patch --]
[-- Type: text/plain, Size: 1759 bytes --]
From: Anton Vorontsov <cbou@mail.ru>
No need to get_device(driverfs_dev), because it's already gotten by
add_disk(). And no function will call put_device() second time.
This is the source of deadlock[1] in IDE layer. SCSI workaround it by
calling put_device() themself after put_disk(), thus it should reflect
this change by removing put_device().
[1] deadlock in ide_unregister()
at wait_for_completion(&drive->gendev_rel_comp) after
device_unregister(&drive->gendev). That happens by reason of
ide-probe.c:drive_release_dev() not called because of driverfs_dev
(which is drive->gendev) still referenced by add_disk().
Index: fs/partitions/check.c
===================================================================
RCS file: /cvs/linux/kernel26/fs/partitions/check.c,v
retrieving revision 1.17
diff -u -p -b -B -r1.17 check.c
--- fs/partitions/check.c 23 Aug 2006 18:40:39 -0000 1.17
+++ fs/partitions/check.c 18 Oct 2006 21:13:26 -0000
@@ -389,7 +389,7 @@ static char *make_block_name(struct gend
static void disk_sysfs_symlinks(struct gendisk *disk)
{
- struct device *target = get_device(disk->driverfs_dev);
+ struct device *target = disk->driverfs_dev;
if (target) {
char *disk_name = make_block_name(disk);
sysfs_create_link(&disk->kobj,&target->kobj,"device");
Index: drivers/scsi/sd.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/scsi/sd.c,v
retrieving revision 1.18
diff -u -p -b -B -r1.18 sd.c
--- drivers/scsi/sd.c 23 Aug 2006 18:38:06 -0000 1.18
+++ drivers/scsi/sd.c 18 Oct 2006 21:14:54 -0000
@@ -1756,7 +1756,6 @@ static void scsi_disk_release(struct cla
disk->private_data = NULL;
put_disk(disk);
- put_device(&sdkp->device->sdev_gendev);
kfree(sdkp);
}
[-- Attachment #5: hh-ide-genhd-workaround.patch --]
[-- Type: text/plain, Size: 2034 bytes --]
From: Anton Vorontsov <cbou@mail.ru>
Because add_disk() getting driverfs_dev twice but putting it only once,
ide-{disk,cd,floppy} should put_device() themselfs. This is workaround
for the bug found in genhd, which causes deadlock in ide_unregister()
at wait_for_completion(&drive->gendev_rel_comp) after
device_unregister(&drive->gendev). That happens by reason of
ide-probe.c:drive_release_dev() not called because of driverfs_dev
(which is drive->gendev) still referenced by add_disk().
Index: drivers/ide/ide-cd.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/ide/ide-cd.c,v
retrieving revision 1.21
diff -u -p -b -B -r1.21 ide-cd.c
--- drivers/ide/ide-cd.c 23 Aug 2006 18:35:05 -0000 1.21
+++ drivers/ide/ide-cd.c 18 Oct 2006 21:18:52 -0000
@@ -3240,6 +3240,7 @@ static void ide_cd_release(struct kref *
blk_queue_prep_rq(drive->queue, NULL);
g->private_data = NULL;
put_disk(g);
+ put_device(&drive->gendev);
kfree(info);
}
Index: drivers/ide/ide-disk.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/ide/ide-disk.c,v
retrieving revision 1.19
diff -u -p -b -B -r1.19 ide-disk.c
--- drivers/ide/ide-disk.c 23 Aug 2006 18:35:05 -0000 1.19
+++ drivers/ide/ide-disk.c 18 Oct 2006 21:18:52 -0000
@@ -1021,6 +1021,7 @@ static void ide_disk_release(struct kref
drive->devfs_name[0] = '\0';
g->private_data = NULL;
put_disk(g);
+ put_device(&drive->gendev);
kfree(idkp);
}
Index: drivers/ide/ide-floppy.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/ide/ide-floppy.c,v
retrieving revision 1.16
diff -u -p -b -B -r1.16 ide-floppy.c
--- drivers/ide/ide-floppy.c 23 Aug 2006 18:35:05 -0000 1.16
+++ drivers/ide/ide-floppy.c 18 Oct 2006 21:18:52 -0000
@@ -1893,6 +1893,7 @@ static void ide_floppy_release(struct kr
drive->driver_data = NULL;
g->private_data = NULL;
put_disk(g);
+ put_device(&drive->gendev);
kfree(floppy);
}
next reply other threads:[~2006-10-18 22:14 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-10-18 22:15 Anton Vorontsov [this message]
2006-10-19 12:05 ` [PATCH] genhd fix or ide workaround -- choose one Alan Cox
2006-10-19 12:25 ` Anton Vorontsov
2006-10-19 13:08 ` Alan Cox
2006-10-19 13:33 ` Anton Vorontsov
2006-10-19 14:16 ` Anton Vorontsov
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=20061018221506.GA4187@localhost \
--to=cbou@mail.ru \
--cc=kernel-discuss@handhelds.org \
--cc=linux-kernel@vger.kernel.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.