[RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Bartlomiej Zolnierkiewicz]

Hi,

Some background first: we need ide-default driver (set as a device driver
for all driver-less ide devices) mainly because we allow changing devices
settings through /proc/ide/hdX/settings and some of them (current_speed,
pio_mode) are processed via request queue (we are currently preallocating
gendisk and queue structs for all possible ide devices).  The next problem
is that ide-default doesn't register itself with ide and driverfs.
If it does it will "steal" devices meaned to be used by other drivers.

If we want dynamic hwifs/devices, moving gendisks/queues allocation
to device drivers and ide integration with driverfs we need to:

(a) kill /proc/ide/hdX/settings for driver-less devices and kill ide-default

or

(b) add much more shit to ide-default and deal with driver ordering madness
    when integrating with driverfs

Any important reasons why we cant chose solution (a)?

--bartlomiej

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Alan Cox]

On Iau, 2003-08-28 at 15:46, Bartlomiej Zolnierkiewicz wrote:
> Some background first: we need ide-default driver (set as a device driver
> for all driver-less ide devices) mainly because we allow changing devices
> settings through /proc/ide/hdX/settings and some of them (current_speed,
> pio_mode) are processed via request queue (we are currently preallocating
> gendisk and queue structs for all possible ide devices).  The next problem
> is that ide-default doesn't register itself with ide and driverfs.
> If it does it will "steal" devices meaned to be used by other drivers.

Its also used to avoid special cases elsewhere.

> If we want dynamic hwifs/devices, moving gendisks/queues allocation
> to device drivers and ide integration with driverfs we need to:
> 
> (a) kill /proc/ide/hdX/settings for driver-less devices and kill ide-default

ide_default avoids a ton of driver specific special case code outside
of /proc/ide/foo/settings too. It isnt that simple, and I added it
originally to fix hundreds of weird little bugs and races. You also need
it for handling hotplug of devices.

I don't however think it needs to be any brighter than it is now. Driver
ordering isnt important, Linus was pretty emphatic that he a) didn't
care and b) wouldnt take patches to do any kind of rigid ordering when I
asked him (and for hotplug its pretty obvious why)

As far as I can see you either

1. Set up the queues and /proc when you create a hwif

or

2. Provide a generic function for each driver to call that does this
and/or undoes it. Since each driver needs the same code (default
included).

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Bartlomiej Zolnierkiewicz]

On Thursday 28 of August 2003 17:13, Alan Cox wrote:
> On Iau, 2003-08-28 at 15:46, Bartlomiej Zolnierkiewicz wrote:
> > Some background first: we need ide-default driver (set as a device driver
> > for all driver-less ide devices) mainly because we allow changing devices
> > settings through /proc/ide/hdX/settings and some of them (current_speed,
> > pio_mode) are processed via request queue (we are currently preallocating
> > gendisk and queue structs for all possible ide devices).  The next
> > problem is that ide-default doesn't register itself with ide and
> > driverfs. If it does it will "steal" devices meaned to be used by other
> > drivers.
>
> Its also used to avoid special cases elsewhere.

/proc/ide/hdX/settings is the source (directly/indirectly)
for 95% of these special cases.

> > If we want dynamic hwifs/devices, moving gendisks/queues allocation
> > to device drivers and ide integration with driverfs we need to:
> >
> > (a) kill /proc/ide/hdX/settings for driver-less devices and kill
> > ide-default
>
> ide_default avoids a ton of driver specific special case code outside
> of /proc/ide/foo/settings too. It isnt that simple, and I added it

It is simple.  No settings - you dont hit these places.

> originally to fix hundreds of weird little bugs and races. You also need
> it for handling hotplug of devices.

Why for hotplug?  We should move all code physically touching
devices to use request queue (REQ_SPECIAL), then you can simply
mark device as unplugged when its gone and check this flag inside
ide_do_request().

When its plugged again its reprobed (you dont need driver for this)
and if its the same device you just clear unplugged flag.

Does it make sense?

With this scheme you also shouldn't need hwif->unplugged_ops hack.
You mark all drives belonging to hwif as unplugged and you are happy.

> I don't however think it needs to be any brighter than it is now. Driver
> ordering isnt important, Linus was pretty emphatic that he a) didn't

Ordering is important when converting ide drivers to driverfs.
Then i will need to register ide-default with driverfs and it will
be "stealing" devices.

> care and b) wouldnt take patches to do any kind of rigid ordering when I
> asked him (and for hotplug its pretty obvious why)
>
> As far as I can see you either
>
> 1. Set up the queues and /proc when you create a hwif
>
> or
>
> 2. Provide a generic function for each driver to call that does this
> and/or undoes it. Since each driver needs the same code (default
> included).

2. is a proper solution and its not a problem.

Problem is when integrating ide with driverfs.
Then you need to register/unregister ide-default as driverfs driver
and now it can "steal" devices, ie. you have cd drive owned by ide-default,
later you load ide-cdrom driver and your cd drive needs to be unregistered
from ide-default driver first before it can be registered with ide-cdrom
driver - you need to add code to do this or device will be "stealed".
Its not very hard to do but it adds complexity.

--bartlomiej

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Alan Cox]

On Iau, 2003-08-28 at 16:47, Bartlomiej Zolnierkiewicz wrote:
> > Its also used to avoid special cases elsewhere.
> 
> /proc/ide/hdX/settings is the source (directly/indirectly)
> for 95% of these special cases.

/proc/ide/foo/identify is another (and tools rely on that working
without ide-cd/ide-disk etc loaded btw) [HP monitoring tools for one]

> Why for hotplug?  We should move all code physically touching
> devices to use request queue (REQ_SPECIAL), then you can simply
> mark device as unplugged when its gone and check this flag inside
> ide_do_request().

Because you need to manipulate drives not attached to a driver
currently. I guess you could go through hoops to avoid it, but the
old IDE driver was just full of bugs that ide_default removed,
and it removed rather more code than it added.

You also need to be able to open the device to talk to the empty
slot about reprobing it, changing the bus state or even figuring
what driver is missing.

> With this scheme you also shouldn't need hwif->unplugged_ops hack.
> You mark all drives belonging to hwif as unplugged and you are happy.

You need the unplugged_ops for controller unplug, I'm more worried about
disk unplug (which I have working now). For controller unplug you either
have to know that ide_unregister does not ever return until the device
is truely idle - including interrupts, proc access currently running
etc. If so then notionally you can handle it without the unplugged ops
although they certainly make it safer.

I don't know what the constraints on PCI hotplug are but I don't believe
anything will run off and reuse the resources after an unplug event,
although your accesses might generate machine checks (non fatal) on IA64
etc.

> Problem is when integrating ide with driverfs.
> Then you need to register/unregister ide-default as driverfs driver
> and now it can "steal" devices, ie. you have cd drive owned by ide-default,
> later you load ide-cdrom driver and your cd drive needs to be unregistered
> from ide-default driver first before it can be registered with ide-cdrom
> driver - you need to add code to do this or device will be "stealed".
> Its not very hard to do but it adds complexity.

My own feeling is that its a lot easier than trying to catch all the 
corner cases ide_default fixed, especially when you do drive level
hotplug. If the IDE code was glorious and elegant, refcounted and with
a clear life cycle of objects I'd agree with you 8)

Alan

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Bartlomiej Zolnierkiewicz]

On Thursday 28 of August 2003 20:06, Alan Cox wrote:
> On Iau, 2003-08-28 at 16:47, Bartlomiej Zolnierkiewicz wrote:
> > > Its also used to avoid special cases elsewhere.
> >
> > /proc/ide/hdX/settings is the source (directly/indirectly)
> > for 95% of these special cases.
>
> /proc/ide/foo/identify is another (and tools rely on that working
> without ide-cd/ide-disk etc loaded btw) [HP monitoring tools for one]

Eeeh... easy to workaround: no settings -> drive->id doesnt change.
There is minor problem here: no driver -> driver -> no driver.

> > Why for hotplug?  We should move all code physically touching
> > devices to use request queue (REQ_SPECIAL), then you can simply
> > mark device as unplugged when its gone and check this flag inside
> > ide_do_request().
>
> Because you need to manipulate drives not attached to a driver
> currently. I guess you could go through hoops to avoid it, but the
> old IDE driver was just full of bugs that ide_default removed,
> and it removed rather more code than it added.

Heh.. bugs yes, code not :).

> You also need to be able to open the device to talk to the empty

You can't open devices owned by ide-default in 2.6 because ide-default,
probably because it doesnt call add_disk() for them.

> slot about reprobing it, changing the bus state or even figuring
> what driver is missing.

I think scsi does it without default driver.  procfs or sysfs

> > With this scheme you also shouldn't need hwif->unplugged_ops hack.
> > You mark all drives belonging to hwif as unplugged and you are happy.
>
> You need the unplugged_ops for controller unplug, I'm more worried about
> disk unplug (which I have working now). For controller unplug you either

So you can unplug disk in the middle of the transfer,
replug and transfer is continued?

> have to know that ide_unregister does not ever return until the device
> is truely idle - including interrupts, proc access currently running
> etc. If so then notionally you can handle it without the unplugged ops
> although they certainly make it safer.
>
> I don't know what the constraints on PCI hotplug are but I don't believe
> anything will run off and reuse the resources after an unplug event,
> although your accesses might generate machine checks (non fatal) on IA64
> etc.
>
> > Problem is when integrating ide with driverfs.
> > Then you need to register/unregister ide-default as driverfs driver
> > and now it can "steal" devices, ie. you have cd drive owned by
> > ide-default, later you load ide-cdrom driver and your cd drive needs to
> > be unregistered from ide-default driver first before it can be registered
> > with ide-cdrom driver - you need to add code to do this or device will be
> > "stealed". Its not very hard to do but it adds complexity.
>
> My own feeling is that its a lot easier than trying to catch all the
> corner cases ide_default fixed, especially when you do drive level
> hotplug. If the IDE code was glorious and elegant, refcounted and with
> a clear life cycle of objects I'd agree with you 8)

Eeehh... okay I will try "plan b". :-\

--bartlomiej

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Alan Cox]

On Iau, 2003-08-28 at 19:39, Bartlomiej Zolnierkiewicz wrote:
> > Because you need to manipulate drives not attached to a driver
> > currently. I guess you could go through hoops to avoid it, but the
> > old IDE driver was just full of bugs that ide_default removed,
> > and it removed rather more code than it added.
> 
> Heh.. bugs yes, code not :).

It removed code. I counted both lines and size when deciding to take
that path 8)

> > You also need to be able to open the device to talk to the empty
> 
> You can't open devices owned by ide-default in 2.6 because ide-default,
> probably because it doesnt call add_disk() for them.

ide_default in 2.6 has no open/close method. In the latest 2.4-ac (bits
I sent to Marcelo too) it has one because it now needs it.

> I think scsi does it without default driver.  procfs or sysfs

The scsi scheme is somewhat broken, it also fails with hotplug because
if I ask for a reprobe of "0 0 0" and we change controllers - we've just
rescanned the wrong bus. Its only a small race but its real.

> > You need the unplugged_ops for controller unplug, I'm more worried about
> > disk unplug (which I have working now). For controller unplug you either
> 
> So you can unplug disk in the middle of the transfer,
> replug and transfer is continued?

I can do

	umount /dev/hda
	hdparm -b0 /dev/hda
        hdparm -b1 /dev/hda
        mount /dev/hda

and mount a different disk/cd/dvd. Its very nice on the thinkpads

In the 2.4 case its a bit messier than I suspect you can do in 2.6
because I can't rely on sane behaviour if I remove queues etc, instead
I keep the queue around so that to the block layer nothing disappears
and is freed up causing race issues but to the IDE layer the right stuff
happens (I think ;))

Alan

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Erik Andersen]

On Thu Aug 28, 2003 at 05:47:11PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Problem is when integrating ide with driverfs.
> Then you need to register/unregister ide-default as driverfs driver
> and now it can "steal" devices, ie. you have cd drive owned by ide-default,
> later you load ide-cdrom driver and your cd drive needs to be unregistered
> from ide-default driver first before it can be registered with ide-cdrom
> driver - you need to add code to do this or device will be "stealed".
> Its not very hard to do but it adds complexity.

Make an interface whereby ide-cdrom (and ide-disk, etc) can walk
the list of all unclaimed devices (i.e. devices owned by
ide-default), check they are of the correct type, and claim them
(thereby removing them from the ide-default driver).  When
unregistering, reverse the process and give the device back to
ide-default...  i.e. make ide-default a holding pen for unclaimed
devices,

 -Erik

--
Erik B. Andersen             http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

Re: [RFC] /proc/ide/hdx/settings with ide-default pseudo-driver is a 2.6/2.7 show-stopper

[Alan Cox]

On Gwe, 2003-08-29 at 02:19, Erik Andersen wrote:
> Make an interface whereby ide-cdrom (and ide-disk, etc) can walk
> the list of all unclaimed devices (i.e. devices owned by
> ide-default), check they are of the correct type, and claim them
> (thereby removing them from the ide-default driver).  When
> unregistering, reverse the process and give the device back to
> ide-default...  i.e. make ide-default a holding pen for unclaimed
> devices,

Thats how the current setup works (at least in 2.4). ide-default is
the driver for the old cases where driver = NULL, eliminating the
special cases and bugs.