Messages in this thread Patch in this message | | | Date | Thu, 03 Apr 2003 16:36:35 +0800 | From | "rain.wang" <> | Subject | PATCH RFC :ide_do_reset() fix for 2.5.66 |
| |
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 --- /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); | |