Re: [PATCH linux-2.6-block:post-2.6.15 08/10] blk: update IDE to use new blk_ordered

[Bartlomiej Zolnierkiewicz]

thinko

On 11/22/05, Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> wrote:

> > > > >> static void ide_cacheflush_p(ide_drive_t *drive)
> > > > >>@@ -1034,6 +993,8 @@ static int ide_disk_remove(struct device
> > > > >>        struct ide_disk_obj *idkp = drive->driver_data;
> > > > >>        struct gendisk *g = idkp->disk;
> > > > >>
> > > > >>+       blk_queue_ordered(drive->queue, QUEUE_ORDERED_NONE, NULL, 0);
> > > > >>+
> > > > >
> > > > >
> > > > > Shouldn't this be done in ide_disk_release()?
> > > >
> > > > Hmmm... The thing is that, AFAIK, requests are not supposed to be issued
> > > > after ->remove is called (->remove is called only on module unload
> > > > unless hardware is hot-unplugged and HL driver cannot be unloaded while
> > > > it's still opened).  I think that's why both sd and ide-disk issue the
> > > > last cache flush in ->remove callbacks but not in ->release.
> > >
> > > Are you sure?  I think that only calling del_gendisk() assures you
> > > that there won't be outstanding fs requests?
> > >
> > > I have also noticed bug in ide_disk_remove() - ide_cacheflush_p()
> > > should be called after del_gendisk() - I will fix it later.
> > >
> > > BTW Nowadays you can dynamically dettach/attach driver from/to
> > > device using sysfs interface.
> >
> > I agree that it should go into ->release, but I am still a bit scared
> > about issuing commands in ->release (it might access some data
> > structure which might be gone by then).  Also, the correct order seems
> > to be 'turning off ordered' and then 'perform the last cache flush'.
> > So, how about adding blk_queue_ordered right above the last
> > ide_cacheflush_p() now and move both to ->release in a separate patch
> > for both IDE and SCSI?
>
> Not needed, when ide-disk is fixed to call del_gendisk() after
> ide_cacheflush_p(), we can add blk_queue_orderer() before

ide_cacheflush_p() after del_gendisk()

> the latter and then everything should be OK.

blk_queue_ordererd() before ide_cache_flush_p()

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