PATCH RFC :ide_do_reset() fix for 2.5.66

rain.wang (rain.wang@mic.com.tw)
Thu, 03 Apr 2003 16:36:35 +0800


This is a multi-part message in MIME format.
--------------71983BEDD2F9CD3F3D50627B
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Jens Axboe wrote:

> On Thu, Apr 03 2003, rain.wang wrote:
> > Hi Alan,
> > I found just changing ide_do_reset() to wait till completion can
> > handle the handler race. can this be enough?
>
> This is buggy for a number of reasons. Firstly, how do you make sure
> that someone else doesn't race with your hwif_data manipulation? This
> looks very suspect. By far the worst problem is that you are assuming
> that ide_do_reset() can sleep, when in fact it cannot (just follow the
> various paths into ide_do_request()). You even grab the ide_lock _and_
> disable interrupts yourself prior calling wait_for_completion(), this is
> incredibly broken.
>
> --
> Jens Axboe

Hi,
Thank you, I'm too young. I should have put this in RFC.
please help me to replace using 'hwif-data', I mainly mean
to let drive reset wait for its clean up complete.

regards
rain.w

--------------71983BEDD2F9CD3F3D50627B
Content-Type: text/plain; charset=us-ascii;
name="ide-iops.c.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="ide-iops.c.diff"

--- /usr/src/linux/drivers/ide/ide-iops.c Thu Apr 3 14:13:51 2003
+++ ide-iops.c Thu Apr 3 16:24:31 2003
@@ -1107,6 +1107,10 @@
}
/* done polling */
hwgroup->poll_timeout = 0;
+
+ /* tell ide_do_reset it complete */
+ complete((struct completion *)hwif->hwif_data);
+
return ide_stopped;
}

@@ -1171,6 +1175,10 @@
}
}
hwgroup->poll_timeout = 0; /* done polling */
+
+ /* tell ide_do_reset it complete */
+ complete((struct completion *)hwif->hwif_data);
+
return ide_stopped;
}

@@ -1307,7 +1315,25 @@

ide_startstop_t ide_do_reset (ide_drive_t *drive)
{
- return do_reset1(drive, 0);
+ /*
+ * Waiting for completion needed.
+ */
+ ide_hwif_t *hwif;
+ void *old_data;
+ DECLARE_COMPLETION(wait);
+
+ hwif = HWIF(drive);
+
+ old_data = hwif->hwif_data;
+ hwif->hwif_data = &wait;
+
+ (void) do_reset1(drive, 0);
+
+ wait_for_completion(&wait);
+
+ hwif->hwif_data = old_data;
+
+ return ide_stopped;
}

EXPORT_SYMBOL(ide_do_reset);

--------------71983BEDD2F9CD3F3D50627B--

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/