Re: IDE locking problem

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2003-08-05 at 02:28, Bartlomiej Zolnierkiewicz wrote:
> On 3 Aug 2003, Benjamin Herrenschmidt wrote:
> 
> > And there's more to it... ide_unregister() doesn't copy hwif->hold from
> > old to new hwif causing my hotswap bay to lose it's iops on next plug,
> > it doesn't call unregister_device() for neither hwif->gendev nor
> > drive[n]->gendev, causing the device model list to be corrupted after
> > an unregister, ...
> 
> What is a goal of calling init_hwif_data() in ide_unregister()?
> I guess it is used mainly to clear hwif->io_ports and hwif->irq.
> Therefore even if you are using hwif->hold flag io_ports will be set to
> default values, so how do you later find your hwif?

What is the goal ? good question ;) I'd be happy with removing most
of the junk in init_hwif_data, but we need to go a bit further there
for 2.7, maybe we should discuss that one irc one of these days ;)
We probably want to remove the static array of hwifs and change that
into pointers, hwif themselves beeing fully initialized 'offline' by
the host driver, then handed out to the ide layer...

In the meantime, the current code works because init_hwif_data()
calls ide_init_hwif_ports() which is an arch hook, which will fill
the proper io base, so the hwif can still be found. Since the IOps
themselves are saved/restored in ide_unregister, we end up with
proper IO base and proper IOps still there.
In fact, I suspect the only remaining useful thing done by
init_hwif_date() in there is to clear the drive structures.
 
> Hmmm... what about not copying anything and calling init_hwif_data()
> only if !hwif->hold?

We may probably still want to clear the drive array and maybe a
the present flag, no ?

Also, look at my patch, we also _NEED_ to add some proper
device_unregister calls to ide_unregister() or this function will
leave dangling entries in the device list, and since those have the
same restrictions as the new blk_cleanup_queue(), we really need to
do that without the lock held.

I'd suggest merging my patch for now, it won't make things much
worse than what they are today regarding racyness of IDE registration
and unregistration, we an look into sanitizing this as a 2.7 goal.

Ben.