From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: linux-ide@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Mariusz Kozlowski <m.kozlowski@tuxland.pl>,
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
Borislav Petkov <petkovbb@gmail.com>
Subject: [PATCH] ide: fix regression caused by ide_device_{get,put}() addition (take 2)
Date: Sun, 3 Aug 2008 16:48:57 +0200 [thread overview]
Message-ID: <200808031648.57513.bzolnier@gmail.com> (raw)
Ben, please use this version instead
(together with ide_host_alloc_all() fix)
for media-bay testing.
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: fix regression caused by ide_device_{get,put}() addition (take 2)
On Monday 28 July 2008, Benjamin Herrenschmidt wrote:
[...]
> Vector: 300 (Data Access) at [c58b7b80]
> pc: c014f264: elv_may_queue+0x10/0x44
> lr: c0152750: get_request+0x2c/0x2c0
> sp: c58b7c30
> msr: 1032
> dar: c
> dsisr: 40000000
> current = 0xc58aaae0
> pid = 854, comm = media-bay
> enter ? for help
> mon> t
> [c58b7c40] c0152750 get_request+0x2c/0x2c0
> [c58b7c70] c0152a08 get_request_wait+0x24/0xec
> [c58b7cc0] c0225674 ide_cd_queue_pc+0x58/0x1a0
> [c58b7d40] c022672c ide_cdrom_packet+0x9c/0xdc
> [c58b7d70] c0261810 cdrom_get_disc_info+0x60/0xd0
> [c58b7dc0] c026208c cdrom_mrw_exit+0x1c/0x11c
> [c58b7e30] c0260f7c unregister_cdrom+0x84/0xe8
> [c58b7e50] c022395c ide_cd_release+0x80/0x84
> [c58b7e70] c0163650 kref_put+0x54/0x6c
> [c58b7e80] c0223884 ide_cd_put+0x40/0x5c
> [c58b7ea0] c0211100 generic_ide_remove+0x28/0x3c
> [c58b7eb0] c01e9d34 __device_release_driver+0x78/0xb4
> [c58b7ec0] c01e9e44 device_release_driver+0x28/0x44
> [c58b7ee0] c01e8f7c bus_remove_device+0xac/0xd8
> [c58b7f00] c01e7424 device_del+0x104/0x198
> [c58b7f20] c01e74d0 device_unregister+0x18/0x30
> [c58b7f40] c02121c4 __ide_port_unregister_devices+0x6c/0x88
> [c58b7f60] c0212398 ide_port_unregister_devices+0x38/0x80
> [c58b7f80] c0208ca4 media_bay_step+0x1cc/0x5c0
> [c58b7fb0] c0209124 media_bay_task+0x8c/0xcc
> [c58b7fd0] c00485c0 kthread+0x48/0x84
> [c58b7ff0] c0011b20 kernel_thread+0x44/0x60
The guilty commit turned out to be 08da591e14cf87247ec09b17c350235157a92fc3
("ide: add ide_device_{get,put}() helpers"). ide_device_put() is called
before kref_put() in ide_cd_put() so IDE device is already gone by the time
ide_cd_release() is reached.
Fix it by calling ide_device_get() before kref_get() and ide_device_put()
after kref_put() in all affected device drivers.
v2:
Brown paper bag time. In v1 cd->drive was referenced after dropping last
reference on cd object (which could result in OOPS in ide_device_put() as
reported/debugged by Mariusz Kozlowski). Fix it by caching cd->drive in
the local variable (fix other device drivers too).
Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reported-by: Mariusz Kozlowski <m.kozlowski@tuxland.pl>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
replacement patch
drivers/ide/ide-cd.c | 12 +++++++-----
drivers/ide/ide-disk.c | 11 ++++++-----
drivers/ide/ide-floppy.c | 11 ++++++-----
drivers/ide/ide-tape.c | 11 ++++++-----
drivers/scsi/ide-scsi.c | 11 ++++++-----
5 files changed, 31 insertions(+), 25 deletions(-)
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -66,11 +66,11 @@ static struct cdrom_info *ide_cd_get(str
mutex_lock(&idecd_ref_mutex);
cd = ide_cd_g(disk);
if (cd) {
- kref_get(&cd->kref);
- if (ide_device_get(cd->drive)) {
- kref_put(&cd->kref, ide_cd_release);
+ if (ide_device_get(cd->drive))
cd = NULL;
- }
+ else
+ kref_get(&cd->kref);
+
}
mutex_unlock(&idecd_ref_mutex);
return cd;
@@ -78,9 +78,11 @@ static struct cdrom_info *ide_cd_get(str
static void ide_cd_put(struct cdrom_info *cd)
{
+ ide_drive_t *drive = cd->drive;
+
mutex_lock(&idecd_ref_mutex);
- ide_device_put(cd->drive);
kref_put(&cd->kref, ide_cd_release);
+ ide_device_put(drive);
mutex_unlock(&idecd_ref_mutex);
}
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -63,11 +63,10 @@ static struct ide_disk_obj *ide_disk_get
mutex_lock(&idedisk_ref_mutex);
idkp = ide_disk_g(disk);
if (idkp) {
- kref_get(&idkp->kref);
- if (ide_device_get(idkp->drive)) {
- kref_put(&idkp->kref, ide_disk_release);
+ if (ide_device_get(idkp->drive))
idkp = NULL;
- }
+ else
+ kref_get(&idkp->kref);
}
mutex_unlock(&idedisk_ref_mutex);
return idkp;
@@ -75,9 +74,11 @@ static struct ide_disk_obj *ide_disk_get
static void ide_disk_put(struct ide_disk_obj *idkp)
{
+ ide_drive_t *drive = idkp->drive;
+
mutex_lock(&idedisk_ref_mutex);
- ide_device_put(idkp->drive);
kref_put(&idkp->kref, ide_disk_release);
+ ide_device_put(drive);
mutex_unlock(&idedisk_ref_mutex);
}
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -168,11 +168,10 @@ static struct ide_floppy_obj *ide_floppy
mutex_lock(&idefloppy_ref_mutex);
floppy = ide_floppy_g(disk);
if (floppy) {
- kref_get(&floppy->kref);
- if (ide_device_get(floppy->drive)) {
- kref_put(&floppy->kref, idefloppy_cleanup_obj);
+ if (ide_device_get(floppy->drive))
floppy = NULL;
- }
+ else
+ kref_get(&floppy->kref);
}
mutex_unlock(&idefloppy_ref_mutex);
return floppy;
@@ -180,9 +179,11 @@ static struct ide_floppy_obj *ide_floppy
static void ide_floppy_put(struct ide_floppy_obj *floppy)
{
+ ide_drive_t *drive = floppy->drive;
+
mutex_lock(&idefloppy_ref_mutex);
- ide_device_put(floppy->drive);
kref_put(&floppy->kref, idefloppy_cleanup_obj);
+ ide_device_put(drive);
mutex_unlock(&idefloppy_ref_mutex);
}
Index: b/drivers/ide/ide-tape.c
===================================================================
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -331,11 +331,10 @@ static struct ide_tape_obj *ide_tape_get
mutex_lock(&idetape_ref_mutex);
tape = ide_tape_g(disk);
if (tape) {
- kref_get(&tape->kref);
- if (ide_device_get(tape->drive)) {
- kref_put(&tape->kref, ide_tape_release);
+ if (ide_device_get(tape->drive))
tape = NULL;
- }
+ else
+ kref_get(&tape->kref);
}
mutex_unlock(&idetape_ref_mutex);
return tape;
@@ -343,9 +342,11 @@ static struct ide_tape_obj *ide_tape_get
static void ide_tape_put(struct ide_tape_obj *tape)
{
+ ide_drive_t *drive = tape->drive;
+
mutex_lock(&idetape_ref_mutex);
- ide_device_put(tape->drive);
kref_put(&tape->kref, ide_tape_release);
+ ide_device_put(drive);
mutex_unlock(&idetape_ref_mutex);
}
Index: b/drivers/scsi/ide-scsi.c
===================================================================
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -101,11 +101,10 @@ static struct ide_scsi_obj *ide_scsi_get
mutex_lock(&idescsi_ref_mutex);
scsi = ide_scsi_g(disk);
if (scsi) {
- scsi_host_get(scsi->host);
- if (ide_device_get(scsi->drive)) {
- scsi_host_put(scsi->host);
+ if (ide_device_get(scsi->drive))
scsi = NULL;
- }
+ else
+ scsi_host_get(scsi->host);
}
mutex_unlock(&idescsi_ref_mutex);
return scsi;
@@ -113,9 +112,11 @@ static struct ide_scsi_obj *ide_scsi_get
static void ide_scsi_put(struct ide_scsi_obj *scsi)
{
+ ide_drive_t *drive = scsi->drive;
+
mutex_lock(&idescsi_ref_mutex);
- ide_device_put(scsi->drive);
scsi_host_put(scsi->host);
+ ide_device_put(drive);
mutex_unlock(&idescsi_ref_mutex);
}
next reply other threads:[~2008-08-03 14:52 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-03 14:48 Bartlomiej Zolnierkiewicz [this message]
2008-08-04 5:41 ` [PATCH] ide: fix regression caused by ide_device_{get,put}() addition (take 2) Benjamin Herrenschmidt
2008-08-04 6:03 ` Benjamin Herrenschmidt
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=200808031648.57513.bzolnier@gmail.com \
--to=bzolnier@gmail.com \
--cc=benh@kernel.crashing.org \
--cc=fujita.tomonori@lab.ntt.co.jp \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.kozlowski@tuxland.pl \
--cc=petkovbb@gmail.com \
/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.