"ide=reverse" do we still need this?

"ide=reverse" do we still need this?

[Greg KH]

Hi,

I'm reworking the pci device list logic (we currently keep all PCI
devices in 2 lists, which isn't the nicest, we should be able to get
away with only 1 list.)

The only bother I've found so far is the pci_get_device_reverse()
function, it's used in 2 places, IDE and the calgary driver.

I'm curious if we really still support the ide=reverse option?  It's a
config option that I don't think the distros still enable (SuSE does
not).  Is this still needed these days?

In digging, we changed this option in 2.2.x from being called
"pci=reverse" and no one else seems to miss it.

Any thoughts?

thanks,

greg k-h

pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Tue, Feb 12, 2008 at 04:15:06PM -0800, Greg KH wrote:
> Hi,
> 
> I'm reworking the pci device list logic (we currently keep all PCI
> devices in 2 lists, which isn't the nicest, we should be able to get
> away with only 1 list.)
> 
> The only bother I've found so far is the pci_get_device_reverse()
> function, it's used in 2 places, IDE and the calgary driver.

Why does the calgary driver need this?  Can we just use pci_get_device()
instead?  Why do you need to walk the device list backwards?  Do you get
false positives going forward?

thanks,

greg k-h

Re: "ide=reverse" do we still need this?

[Rene Herman]

On 13-02-08 01:15, Greg KH wrote:

> I'm reworking the pci device list logic (we currently keep all PCI
> devices in 2 lists, which isn't the nicest, we should be able to get
> away with only 1 list.)
> 
> The only bother I've found so far is the pci_get_device_reverse()
> function, it's used in 2 places, IDE and the calgary driver.
> 
> I'm curious if we really still support the ide=reverse option?  It's a
> config option that I don't think the distros still enable (SuSE does
> not).  Is this still needed these days?
> 
> In digging, we changed this option in 2.2.x from being called
> "pci=reverse" and no one else seems to miss it.
> 
> Any thoughts?

While details escape me somewhat again at the monment, a few months ago I 
was playing around with a PCI Promise IDE controller and needed ide=reverse 
to save me from having to switch disks around to still have a bootable system.

Or some such. Not too clear anymore, but I remember it saved the day.

Rene.

Re: pci_get_device_reverse(), why does Calgary need this?

[Alan Cox]

> Why does the calgary driver need this?  Can we just use pci_get_device()
> instead?  Why do you need to walk the device list backwards?  Do you get
> false positives going forward?

It doesn't look to be performance critical so the driver can
pci_get_device until the end and use the final hit anyway. IDE reverse is
more problematic but nobody seems to use it.

Re: "ide=reverse" do we still need this?

[Ken Moffat]

On Tue, Feb 12, 2008 at 04:15:07PM -0800, Greg KH wrote:
> 
> I'm curious if we really still support the ide=reverse option?  It's a
> config option that I don't think the distros still enable (SuSE does
> not).  Is this still needed these days?
> 
 My "server" has a consumer-grade desktop amd64 mobo, with all that
implies about cheap hardware and strange/misleading bios options.
It also has an add-in dual IDE card with the main data on raid1.
It's set to ide=reverse, without that it doesn't boot (the add-ins
are IDE, system drive is SATA, so I guess it probably tries to boot
from the DVD - it's been a long time since it bit me and I don't
remember the full details.

 That was how it was set for 2.6.18.6, and how it now boots from
2.6.22.18.  I think at one time the order of the interfaces might
have been different.  Certainly, I carry forward a fallback without
ide=reverse in lilo.conf, just in case the disks move on my next
kernel upgrade.

 What a distro selects should cover most of that distro's users, but
that is not anywhere near providing 100% coverage for *all* the
hardware out there.  Also, you can force your users to e.g. mount by
label.  So far, that hasn't been forced on me, and I really hate
having to reboot that box :)

Ken
-- 
das eine Mal als Tragödie, das andere Mal als Farce

Re: "ide=reverse" do we still need this?

[Greg KH]

On Wed, Feb 13, 2008 at 02:43:29AM +0000, Ken Moffat wrote:
> On Tue, Feb 12, 2008 at 04:15:07PM -0800, Greg KH wrote:
> > 
> > I'm curious if we really still support the ide=reverse option?  It's a
> > config option that I don't think the distros still enable (SuSE does
> > not).  Is this still needed these days?
> > 
>  My "server" has a consumer-grade desktop amd64 mobo, with all that
> implies about cheap hardware and strange/misleading bios options.
> It also has an add-in dual IDE card with the main data on raid1.
> It's set to ide=reverse, without that it doesn't boot (the add-ins
> are IDE, system drive is SATA, so I guess it probably tries to boot
> from the DVD - it's been a long time since it bit me and I don't
> remember the full details.
> 
>  That was how it was set for 2.6.18.6, and how it now boots from
> 2.6.22.18.  I think at one time the order of the interfaces might
> have been different.  Certainly, I carry forward a fallback without
> ide=reverse in lilo.conf, just in case the disks move on my next
> kernel upgrade.

Can't you just boot with /dev/disk/by-id/ and an initramfs to not have
to worry about such a thing in the future?

Have you tried the PATA drivers instead of IDE to see if this solves the
"moves around" issue?  If they work, then you would not need the command
line option at all.

thanks,

greg k-h

Re: "ide=reverse" do we still need this?

[Greg KH]

On Wed, Feb 13, 2008 at 02:41:07AM +0100, Rene Herman wrote:
> On 13-02-08 01:15, Greg KH wrote:
>
>> I'm reworking the pci device list logic (we currently keep all PCI
>> devices in 2 lists, which isn't the nicest, we should be able to get
>> away with only 1 list.)
>> The only bother I've found so far is the pci_get_device_reverse()
>> function, it's used in 2 places, IDE and the calgary driver.
>> I'm curious if we really still support the ide=reverse option?  It's a
>> config option that I don't think the distros still enable (SuSE does
>> not).  Is this still needed these days?
>> In digging, we changed this option in 2.2.x from being called
>> "pci=reverse" and no one else seems to miss it.
>> Any thoughts?
>
> While details escape me somewhat again at the monment, a few months ago I 
> was playing around with a PCI Promise IDE controller and needed ide=reverse 
> to save me from having to switch disks around to still have a bootable 
> system.
>
> Or some such. Not too clear anymore, but I remember it saved the day.

You couldn't just change the boot disk in grub?

Or use an initramfs and /dev/disk/by-id/ to keep any future moves
stable?

thanks,

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > Why does the calgary driver need this?  Can we just use pci_get_device()
> > instead?  Why do you need to walk the device list backwards?  Do you get
> > false positives going forward?
> 
> It doesn't look to be performance critical so the driver can
> pci_get_device until the end and use the final hit anyway.

That would make more sense.

> IDE reverse is more problematic but nobody seems to use it.

I've seen two posters say they use it.  I'm wondering what it is really
solving if they use it, and why if it's really needed, scsi never had to
implement such a hack...

thanks,

greg k-h

Re: [discuss] "ide=reverse" do we still need this?

[Dirk GOUDERS]

Hi,

> I'm reworking the pci device list logic (we currently keep all PCI
> devices in 2 lists, which isn't the nicest, we should be able to get
> away with only 1 list.)
> 
> The only bother I've found so far is the pci_get_device_reverse()
> function, it's used in 2 places, IDE and the calgary driver.
> 
> I'm curious if we really still support the ide=reverse option?  It's a
> config option that I don't think the distros still enable (SuSE does
> not).  Is this still needed these days?
> 
> In digging, we changed this option in 2.2.x from being called
> "pci=reverse" and no one else seems to miss it.
> 
> Any thoughts?

I remember vaguely that some years ago, we set up a box with four IDE
disks as a RAID set.  For that purpose, we added a PCI ATA100 controller
so that each disk could act as a primary IDE device and we were only able
to boot the system with the option ide=reverse.
That box has been replaced by some other so I cannot verify it but as
far as I remember it was a problem with disk numbering between BIOS,
bootloader and/or kernel.  Also, at that time we used lilo and I am not
sure if grub would have done better.

Dirk

Re: [discuss] "ide=reverse" do we still need this?

[Greg KH]

On Wed, Feb 13, 2008 at 08:54:55AM +0100, Dirk GOUDERS wrote:
> Hi,
> 
> > I'm reworking the pci device list logic (we currently keep all PCI
> > devices in 2 lists, which isn't the nicest, we should be able to get
> > away with only 1 list.)
> > 
> > The only bother I've found so far is the pci_get_device_reverse()
> > function, it's used in 2 places, IDE and the calgary driver.
> > 
> > I'm curious if we really still support the ide=reverse option?  It's a
> > config option that I don't think the distros still enable (SuSE does
> > not).  Is this still needed these days?
> > 
> > In digging, we changed this option in 2.2.x from being called
> > "pci=reverse" and no one else seems to miss it.
> > 
> > Any thoughts?
> 
> I remember vaguely that some years ago, we set up a box with four IDE
> disks as a RAID set.  For that purpose, we added a PCI ATA100 controller
> so that each disk could act as a primary IDE device and we were only able
> to boot the system with the option ide=reverse.
> That box has been replaced by some other so I cannot verify it but as
> far as I remember it was a problem with disk numbering between BIOS,
> bootloader and/or kernel.  Also, at that time we used lilo and I am not
> sure if grub would have done better.

Hm, so, to summarize:
  - you needed this option many years ago to get a box to work properly
  - you don't need this today

So, if the option went away, you would not be inconvenienced?

thanks,

greg k-h

Re: [discuss] "ide=reverse" do we still need this?

[Dirk GOUDERS]

> Hm, so, to summarize:
>   - you needed this option many years ago to get a box to work properly
>   - you don't need this today

I would summarize:

    - ide=reverse solved certain problems and I am not sure if there are
      users who still need this option

> So, if the option went away, you would not be inconvenienced?

I remember that the disks of the old box are still in a drawer and today,
I will try to reanimate the system and see if grub's ability to map
drives helps to boot the system without ide=reverse.  I'll report later.

Dirk

Re: pci_get_device_reverse(), why does Calgary need this?

[Muli Ben-Yehuda]

On Tue, Feb 12, 2008 at 04:16:38PM -0800, Greg KH wrote:

> Why does the calgary driver need this?  Can we just use
> pci_get_device() instead?  Why do you need to walk the device list
> backwards?  Do you get false positives going forward?

It's not strictly needed, we used it for symmetry. Feel free to nuke
it and walk the list forward.

Cheers,
Muli

Re: "ide=reverse" do we still need this?

[Rene Herman]

On 13-02-08 05:44, Greg KH wrote:

>> While details escape me somewhat again at the monment, a few months ago
>> I was playing around with a PCI Promise IDE controller and needed
>> ide=reverse to save me from having to switch disks around to still have
>> a bootable system.
>> 
>> Or some such. Not too clear anymore, but I remember it saved the day.
> 
> You couldn't just change the boot disk in grub?
> 
> Or use an initramfs and /dev/disk/by-id/ to keep any future moves stable?

No. The thing is that you need these kinds of hacks while messing with old 
systems, building and stripping them, often in recovery type of situations.

As said (same as the other person I saw reacting) details of what was most 
decidedly needed last time around escape me at the moment, but ide=reverse 
is the kind of hack that saves one hours of unscrewing computer cases and 
switching disks around while building stuff, making quick tests, doing 
recovery...

If it must go for the greater architectural good, so be it, but it's the 
type of thing that's used specifically in the situations where you don't 
have stable, well arranged (or known!) setups to begin with.

Rene.

Re: "ide=reverse" do we still need this?

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

On Wed, 2008-02-13 at 13:06 +0100, Rene Herman wrote:
> On 13-02-08 05:44, Greg KH wrote:
> 
> >> While details escape me somewhat again at the monment, a few months ago
> >> I was playing around with a PCI Promise IDE controller and needed
> >> ide=reverse to save me from having to switch disks around to still have
> >> a bootable system.
> >> 
> >> Or some such. Not too clear anymore, but I remember it saved the day.
> > 
> > You couldn't just change the boot disk in grub?
> > 
> > Or use an initramfs and /dev/disk/by-id/ to keep any future moves stable?
> 
> No. The thing is that you need these kinds of hacks while messing with old 
> systems, building and stripping them, often in recovery type of situations.
> 
> As said (same as the other person I saw reacting) details of what was most 
> decidedly needed last time around escape me at the moment, but ide=reverse 
> is the kind of hack that saves one hours of unscrewing computer cases and 
> switching disks around while building stuff, making quick tests, doing 
> recovery...
> 
> If it must go for the greater architectural good, so be it, but it's the 
> type of thing that's used specifically in the situations where you don't 
> have stable, well arranged (or known!) setups to begin with.

I might be off the deep end, but isn't this what
Documentation/feature-removal-schedule.txt is for?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: pci_get_device_reverse(), why does Calgary need this?

[Bartlomiej Zolnierkiewicz]

On Wednesday 13 February 2008, Greg KH wrote:
> On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > false positives going forward?
> > 
> > It doesn't look to be performance critical so the driver can
> > pci_get_device until the end and use the final hit anyway.
> 
> That would make more sense.
> 
> > IDE reverse is more problematic but nobody seems to use it.
> 
> I've seen two posters say they use it.  I'm wondering what it is really
> solving if they use it, and why if it's really needed, scsi never had to
> implement such a hack...

It is no longer solving anything, just adds more pain. ;)

[ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
  became popular).  Some "off-board" controllers integrated on motherboards
  used to appear before "on-board" IDE on PCI bus so this option was meant
  to preserve the legacy ordering. ]

Since it is valid only when "Probe IDE PCI devices in the PCI bus order
(DEPRECATED)" config option is used it is already on its way out (though
marking it as obsoleted would make it more explicit).

I think that removing "ide=reverse" in 2.6.26 would be OK...

Thanks,
Bart

Re: "ide=reverse" do we still need this?

[Rene Herman]

On 13-02-08 13:16, Michael Ellerman wrote:

> On Wed, 2008-02-13 at 13:06 +0100, Rene Herman wrote:
>> On 13-02-08 05:44, Greg KH wrote:
>>
>>>> While details escape me somewhat again at the monment, a few months ago
>>>> I was playing around with a PCI Promise IDE controller and needed
>>>> ide=reverse to save me from having to switch disks around to still have
>>>> a bootable system.
>>>>
>>>> Or some such. Not too clear anymore, but I remember it saved the day.
>>> You couldn't just change the boot disk in grub?
>>>
>>> Or use an initramfs and /dev/disk/by-id/ to keep any future moves stable?
>> No. The thing is that you need these kinds of hacks while messing with old 
>> systems, building and stripping them, often in recovery type of situations.
>>
>> As said (same as the other person I saw reacting) details of what was most 
>> decidedly needed last time around escape me at the moment, but ide=reverse 
>> is the kind of hack that saves one hours of unscrewing computer cases and 
>> switching disks around while building stuff, making quick tests, doing 
>> recovery...
>>
>> If it must go for the greater architectural good, so be it, but it's the 
>> type of thing that's used specifically in the situations where you don't 
>> have stable, well arranged (or known!) setups to begin with.
> 
> I might be off the deep end, but isn't this what
> Documentation/feature-removal-schedule.txt is for?

Documentation/feature-removal-schedule.txt is for asking/discussing whether 
or not features should be removed? No, I don't think so. It seems to be a 
schedule of when to remove features.

Rene.

Re: "ide=reverse" do we still need this?

[Rene Herman]

On 13-02-08 13:06, Rene Herman wrote:
> On 13-02-08 05:44, Greg KH wrote:
> 
>>> While details escape me somewhat again at the monment, a few months ago
>>> I was playing around with a PCI Promise IDE controller and needed
>>> ide=reverse to save me from having to switch disks around to still have
>>> a bootable system.
>>>
>>> Or some such. Not too clear anymore, but I remember it saved the day.
>>
>> You couldn't just change the boot disk in grub?
>>
>> Or use an initramfs and /dev/disk/by-id/ to keep any future moves stable?
> 
> No. The thing is that you need these kinds of hacks while messing with 
> old systems, building and stripping them, often in recovery type of 
> situations.
> 
> As said (same as the other person I saw reacting) details of what was 
> most decidedly needed last time around escape me at the moment, but 
> ide=reverse is the kind of hack that saves one hours of unscrewing 
> computer cases and switching disks around while building stuff, making 
> quick tests, doing recovery...
> 
> If it must go for the greater architectural good, so be it, but it's the 
> type of thing that's used specifically in the situations where you don't 
> have stable, well arranged (or known!) setups to begin with.

Allow me to add that the demise of drivers/ide itself is an argument for 
just shooting the thing if it helps clean up the API. Next year when I'm 
messing with that Promise controller again, the machine might very well be 
running a kernel using PATA instead of IDE anyway...

Rene.

Re: "ide=reverse" do we still need this?

[Ken Moffat]

On Tue, Feb 12, 2008 at 08:43:04PM -0800, Greg KH wrote:
> 
> Can't you just boot with /dev/disk/by-id/ and an initramfs to not have
> to worry about such a thing in the future?
> 
 Can comebody remind me what the initramfs is for in that situation,
please ?  From the little I've noticed, I thought the /dev/disk/by-id
part went into fstab ?  At the moment, I just build the things I
think I need in to the kernel on that box, without modules.

 Anyway, I'll try to find time to read my notes to see if I can
identify what happened/when, and to take the box down again so I
can try to confirm exactly what the problem is, if it still exists.
I certainly won't be taking it down until I've written my weekly
backups to tape at the weekend, so maybe not before next week.

> Have you tried the PATA drivers instead of IDE to see if this solves the
> "moves around" issue?  If they work, then you would not need the command
> line option at all.

 My previous kernel was 2.18-series, I think at that time they were
still under development.  This box handles the backups for my
various desktop boxes, which is why I'm very conservative about
changing it.  I guess the drivers are stable now, I'll maybe give
that a go (depending on time).

Ken
-- 
das eine Mal als Tragödie, das andere Mal als Farce

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday 13 February 2008, Greg KH wrote:
> > On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > > false positives going forward?
> > > 
> > > It doesn't look to be performance critical so the driver can
> > > pci_get_device until the end and use the final hit anyway.
> > 
> > That would make more sense.
> > 
> > > IDE reverse is more problematic but nobody seems to use it.
> > 
> > I've seen two posters say they use it.  I'm wondering what it is really
> > solving if they use it, and why if it's really needed, scsi never had to
> > implement such a hack...
> 
> It is no longer solving anything, just adds more pain. ;)
> 
> [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
>   became popular).  Some "off-board" controllers integrated on motherboards
>   used to appear before "on-board" IDE on PCI bus so this option was meant
>   to preserve the legacy ordering. ]
> 
> Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> (DEPRECATED)" config option is used it is already on its way out (though
> marking it as obsoleted would make it more explicit).
> 
> I think that removing "ide=reverse" in 2.6.26 would be OK...

Great, thanks for your blessing.  I'll make up a patch and send it to
you for approval.

thanks,

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 11:32:26AM +0200, Muli Ben-Yehuda wrote:
> On Tue, Feb 12, 2008 at 04:16:38PM -0800, Greg KH wrote:
> 
> > Why does the calgary driver need this?  Can we just use
> > pci_get_device() instead?  Why do you need to walk the device list
> > backwards?  Do you get false positives going forward?
> 
> It's not strictly needed, we used it for symmetry.

symetry for what?  Ah, unwinding from an error condition, that makes
sense.

Is there some reason you aren't using the "real" PCI driver api here and
registering a pci driver for these devices?  That would take the whole
"loop over all pci devices" logic out of the code entirely.

> Feel free to nuke it and walk the list forward.

So something like the patch below would be fine?

thanks,

greg k-h

---
 arch/x86/kernel/pci-calgary_64.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -1232,8 +1232,7 @@ static int __init calgary_init(void)
 
 error:
 	do {
-		dev = pci_get_device_reverse(PCI_VENDOR_ID_IBM,
-					     PCI_ANY_ID, dev);
+		dev = pci_get_device(PCI_VENDOR_ID_IBM, PCI_ANY_ID, dev);
 		if (!dev)
 			break;
 		if (!is_cal_pci_dev(dev->device))

Re: pci_get_device_reverse(), why does Calgary need this?

[Muli Ben-Yehuda]

On Wed, Feb 13, 2008 at 09:32:03AM -0800, Greg KH wrote:

> Is there some reason you aren't using the "real" PCI driver api here
> and registering a pci driver for these devices?  That would take the
> whole "loop over all pci devices" logic out of the code entirely.

I recall we had a reason, but I no longer recall what it was. Some
reason the "real" PCI driver API didn't fit at the time. If someone
were to whip up a working patch, I'd happily apply it.

> > Feel free to nuke it and walk the list forward.
> 
> So something like the patch below would be fine?

Yep, looks good. 

Acked-by: Muli Ben-Yehuda <muli@il.ibm.com>

Cheers,
Muli

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 07:47:11PM +0200, Muli Ben-Yehuda wrote:
> On Wed, Feb 13, 2008 at 09:32:03AM -0800, Greg KH wrote:
> 
> > Is there some reason you aren't using the "real" PCI driver api here
> > and registering a pci driver for these devices?  That would take the
> > whole "loop over all pci devices" logic out of the code entirely.
> 
> I recall we had a reason, but I no longer recall what it was. Some
> reason the "real" PCI driver API didn't fit at the time. If someone
> were to whip up a working patch, I'd happily apply it.

"at the time"?  It's been in place since the 2.2 days :)

Is the problem that other drivers also want access to this PCI device
for some reason?

I'll whip up a patch for you to test with in a bit...

> > > Feel free to nuke it and walk the list forward.
> > 
> > So something like the patch below would be fine?
> 
> Yep, looks good. 
> 
> Acked-by: Muli Ben-Yehuda <muli@il.ibm.com>

thanks for reviewing this.

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Wednesday 13 February 2008, Greg KH wrote:
> > > On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > > > false positives going forward?
> > > > 
> > > > It doesn't look to be performance critical so the driver can
> > > > pci_get_device until the end and use the final hit anyway.
> > > 
> > > That would make more sense.
> > > 
> > > > IDE reverse is more problematic but nobody seems to use it.
> > > 
> > > I've seen two posters say they use it.  I'm wondering what it is really
> > > solving if they use it, and why if it's really needed, scsi never had to
> > > implement such a hack...
> > 
> > It is no longer solving anything, just adds more pain. ;)
> > 
> > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> >   became popular).  Some "off-board" controllers integrated on motherboards
> >   used to appear before "on-board" IDE on PCI bus so this option was meant
> >   to preserve the legacy ordering. ]
> > 
> > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > (DEPRECATED)" config option is used it is already on its way out (though
> > marking it as obsoleted would make it more explicit).
> > 
> > I think that removing "ide=reverse" in 2.6.26 would be OK...
> 
> Great, thanks for your blessing.  I'll make up a patch and send it to
> you for approval.

How does the patch below look?  I didn't want to remove the whole config
option, as there is more to the logic than just the "reverse order"
stuff there.

If you don't mind, can I take this through the PCI tree so as to allow
the removal of this pci function afterwards?

thanks,

greg k-h

--------
From: Greg Kroah-Hartman <gregkh@suse.de>
Subject: IDE: remove ide=reverse IDE core

This option is obsolete and can be removed safely.

It allows us to remove the pci_get_device_reverse() function from the
PCI core.


Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/ide/ide-scan-pci.c |    9 ++-------
 drivers/ide/ide.c          |   12 ------------
 include/linux/ide.h        |    1 -
 3 files changed, 2 insertions(+), 20 deletions(-)

--- a/drivers/ide/ide-scan-pci.c
+++ b/drivers/ide/ide-scan-pci.c
@@ -88,13 +88,8 @@ static int __init ide_scan_pcibus(void)
 	struct list_head *l, *n;
 
 	pre_init = 0;
-	if (!ide_scan_direction)
-		while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)))
-			ide_scan_pcidev(dev);
-	else
-		while ((dev = pci_get_device_reverse(PCI_ANY_ID, PCI_ANY_ID,
-						     dev)))
-			ide_scan_pcidev(dev);
+	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)))
+		ide_scan_pcidev(dev);
 
 	/*
 	 *	Hand the drivers over to the PCI layer now we
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -92,10 +92,6 @@ static int system_bus_speed;	/* holds wh
 DEFINE_MUTEX(ide_cfg_mtx);
  __cacheline_aligned_in_smp DEFINE_SPINLOCK(ide_lock);
 
-#ifdef CONFIG_IDEPCI_PCIBUS_ORDER
-int ide_scan_direction; /* THIS was formerly 2.2.x pci=reverse */
-#endif
-
 int noautodma = 0;
 
 #ifdef CONFIG_BLK_DEV_IDEACPI
@@ -1227,14 +1223,6 @@ static int __init ide_setup(char *s)
 		goto obsolete_option;
 	}
 
-#ifdef CONFIG_IDEPCI_PCIBUS_ORDER
-	if (!strcmp(s, "ide=reverse")) {
-		ide_scan_direction = 1;
-		printk(" : Enabled support for IDE inverse scan order.\n");
-		return 1;
-	}
-#endif
-
 #ifdef CONFIG_BLK_DEV_IDEACPI
 	if (!strcmp(s, "ide=noacpi")) {
 		//printk(" : Disable IDE ACPI support.\n");
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -988,7 +988,6 @@ extern void do_ide_request(struct reques
 void ide_init_disk(struct gendisk *, ide_drive_t *);
 
 #ifdef CONFIG_IDEPCI_PCIBUS_ORDER
-extern int ide_scan_direction;
 extern int __ide_pci_register_driver(struct pci_driver *driver, struct module *owner, const char *mod_name);
 #define ide_pci_register_driver(d) __ide_pci_register_driver(d, THIS_MODULE, KBUILD_MODNAME)
 #else

Re: [discuss] "ide=reverse" do we still need this?

[Dirk GOUDERS]

> Hm, so, to summarize:
>   - you needed this option many years ago to get a box to work properly
>   - you don't need this today
> 
> So, if the option went away, you would not be inconvenienced?

After having reanimated the old system and after comments of other
persons I would not be inconvenienced if the option went away.

The system indeed did not boot correctly without that option, because
the disks appeared in a wrong order.  On the other hand, I was able to
(re)install bootloaders (grub as well as lilo) and after that did not
need the option any more.  Unfortunately, after that I was not able to
reproduce the initial situation where the option was needed.

Dirk

Re: [discuss] "ide=reverse" do we still need this?

[Greg KH]

On Wed, Feb 13, 2008 at 09:00:15PM +0100, Dirk GOUDERS wrote:
> 
> > Hm, so, to summarize:
> >   - you needed this option many years ago to get a box to work properly
> >   - you don't need this today
> > 
> > So, if the option went away, you would not be inconvenienced?
> 
> After having reanimated the old system and after comments of other
> persons I would not be inconvenienced if the option went away.
> 
> The system indeed did not boot correctly without that option, because
> the disks appeared in a wrong order.  On the other hand, I was able to
> (re)install bootloaders (grub as well as lilo) and after that did not
> need the option any more.  Unfortunately, after that I was not able to
> reproduce the initial situation where the option was needed.

Great, thanks a lot for testing this, and letting us know.

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Bartlomiej Zolnierkiewicz]

On Wednesday 13 February 2008, Greg KH wrote:
> On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > On Wednesday 13 February 2008, Greg KH wrote:
> > > > On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > > > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > > > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > > > > false positives going forward?
> > > > > 
> > > > > It doesn't look to be performance critical so the driver can
> > > > > pci_get_device until the end and use the final hit anyway.
> > > > 
> > > > That would make more sense.
> > > > 
> > > > > IDE reverse is more problematic but nobody seems to use it.
> > > > 
> > > > I've seen two posters say they use it.  I'm wondering what it is really
> > > > solving if they use it, and why if it's really needed, scsi never had to
> > > > implement such a hack...
> > > 
> > > It is no longer solving anything, just adds more pain. ;)
> > > 
> > > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> > >   became popular).  Some "off-board" controllers integrated on motherboards
> > >   used to appear before "on-board" IDE on PCI bus so this option was meant
> > >   to preserve the legacy ordering. ]
> > > 
> > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > > (DEPRECATED)" config option is used it is already on its way out (though
> > > marking it as obsoleted would make it more explicit).
> > > 
> > > I think that removing "ide=reverse" in 2.6.26 would be OK...
> > 
> > Great, thanks for your blessing.  I'll make up a patch and send it to
> > you for approval.
> 
> How does the patch below look?  I didn't want to remove the whole config
> option, as there is more to the logic than just the "reverse order"
> stuff there.

looks fine,

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

> If you don't mind, can I take this through the PCI tree so as to allow
> the removal of this pci function afterwards?

[...]

great, could you also:
- rebase it on top of the patch below
- forward the patch below to Linus for 2.6.25

?

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: mark "ide=reverse" option as obsolete

- it is valid only if "Probe IDE PCI devices in the PCI bus order
  (DEPRECATED)" config option is used

- Greg needs to remove pci_get_device_reverse() for PCI core changes

Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -1038,7 +1038,7 @@ static int __init ide_setup(char *s)
 	if (!strcmp(s, "ide=reverse")) {
 		ide_scan_direction = 1;
 		printk(" : Enabled support for IDE inverse scan order.\n");
-		return 1;
+		goto obsolete_option;
 	}
 #endif
 

Re: "ide=reverse" do we still need this?

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

On Wed, 2008-02-13 at 13:46 +0100, Rene Herman wrote:
> On 13-02-08 13:16, Michael Ellerman wrote:
> 
> > On Wed, 2008-02-13 at 13:06 +0100, Rene Herman wrote:
> >> On 13-02-08 05:44, Greg KH wrote:
> >>
> >>>> While details escape me somewhat again at the monment, a few months ago
> >>>> I was playing around with a PCI Promise IDE controller and needed
> >>>> ide=reverse to save me from having to switch disks around to still have
> >>>> a bootable system.
> >>>>
> >>>> Or some such. Not too clear anymore, but I remember it saved the day.
> >>> You couldn't just change the boot disk in grub?
> >>>
> >>> Or use an initramfs and /dev/disk/by-id/ to keep any future moves stable?
> >> No. The thing is that you need these kinds of hacks while messing with old 
> >> systems, building and stripping them, often in recovery type of situations.
> >>
> >> As said (same as the other person I saw reacting) details of what was most 
> >> decidedly needed last time around escape me at the moment, but ide=reverse 
> >> is the kind of hack that saves one hours of unscrewing computer cases and 
> >> switching disks around while building stuff, making quick tests, doing 
> >> recovery...
> >>
> >> If it must go for the greater architectural good, so be it, but it's the 
> >> type of thing that's used specifically in the situations where you don't 
> >> have stable, well arranged (or known!) setups to begin with.
> > 
> > I might be off the deep end, but isn't this what
> > Documentation/feature-removal-schedule.txt is for?
> 
> Documentation/feature-removal-schedule.txt is for asking/discussing whether 
> or not features should be removed? No, I don't think so. It seems to be a 
> schedule of when to remove features.

Well it's sort of both I think. It's a schedule, but if enough people
complain that something's being removed then it can be reconsidered
before it's removed.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 11:20:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday 13 February 2008, Greg KH wrote:
> > On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> > > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > On Wednesday 13 February 2008, Greg KH wrote:
> > > > > On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > > > > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > > > > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > > > > > false positives going forward?
> > > > > > 
> > > > > > It doesn't look to be performance critical so the driver can
> > > > > > pci_get_device until the end and use the final hit anyway.
> > > > > 
> > > > > That would make more sense.
> > > > > 
> > > > > > IDE reverse is more problematic but nobody seems to use it.
> > > > > 
> > > > > I've seen two posters say they use it.  I'm wondering what it is really
> > > > > solving if they use it, and why if it's really needed, scsi never had to
> > > > > implement such a hack...
> > > > 
> > > > It is no longer solving anything, just adds more pain. ;)
> > > > 
> > > > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> > > >   became popular).  Some "off-board" controllers integrated on motherboards
> > > >   used to appear before "on-board" IDE on PCI bus so this option was meant
> > > >   to preserve the legacy ordering. ]
> > > > 
> > > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > > > (DEPRECATED)" config option is used it is already on its way out (though
> > > > marking it as obsoleted would make it more explicit).
> > > > 
> > > > I think that removing "ide=reverse" in 2.6.26 would be OK...
> > > 
> > > Great, thanks for your blessing.  I'll make up a patch and send it to
> > > you for approval.
> > 
> > How does the patch below look?  I didn't want to remove the whole config
> > option, as there is more to the logic than just the "reverse order"
> > stuff there.
> 
> looks fine,
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Thanks.

> > If you don't mind, can I take this through the PCI tree so as to allow
> > the removal of this pci function afterwards?
> 
> [...]
> 
> great, could you also:
> - rebase it on top of the patch below
> - forward the patch below to Linus for 2.6.25

Sure, you want this to go in for .25, but not the one I just posted
removing this option, correct?  That should wait for .26?

thanks,

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Bartlomiej Zolnierkiewicz]

On Thursday 14 February 2008, Greg KH wrote:
> On Wed, Feb 13, 2008 at 11:20:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Wednesday 13 February 2008, Greg KH wrote:
> > > On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> > > > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > > On Wednesday 13 February 2008, Greg KH wrote:
> > > > > > On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > > > > > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > > > > > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > > > > > > false positives going forward?
> > > > > > > 
> > > > > > > It doesn't look to be performance critical so the driver can
> > > > > > > pci_get_device until the end and use the final hit anyway.
> > > > > > 
> > > > > > That would make more sense.
> > > > > > 
> > > > > > > IDE reverse is more problematic but nobody seems to use it.
> > > > > > 
> > > > > > I've seen two posters say they use it.  I'm wondering what it is really
> > > > > > solving if they use it, and why if it's really needed, scsi never had to
> > > > > > implement such a hack...
> > > > > 
> > > > > It is no longer solving anything, just adds more pain. ;)
> > > > > 
> > > > > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> > > > >   became popular).  Some "off-board" controllers integrated on motherboards
> > > > >   used to appear before "on-board" IDE on PCI bus so this option was meant
> > > > >   to preserve the legacy ordering. ]
> > > > > 
> > > > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > > > > (DEPRECATED)" config option is used it is already on its way out (though
> > > > > marking it as obsoleted would make it more explicit).
> > > > > 
> > > > > I think that removing "ide=reverse" in 2.6.26 would be OK...
> > > > 
> > > > Great, thanks for your blessing.  I'll make up a patch and send it to
> > > > you for approval.
> > > 
> > > How does the patch below look?  I didn't want to remove the whole config
> > > option, as there is more to the logic than just the "reverse order"
> > > stuff there.
> > 
> > looks fine,
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> Thanks.
> 
> > > If you don't mind, can I take this through the PCI tree so as to allow
> > > the removal of this pci function afterwards?
> > 
> > [...]
> > 
> > great, could you also:
> > - rebase it on top of the patch below
> > - forward the patch below to Linus for 2.6.25
> 
> Sure, you want this to go in for .25, but not the one I just posted
> removing this option, correct?  That should wait for .26?

Yes, lets give people the final call before actually removing this option
(unless of course there is some urgent reason for removing it in .25).

Thanks,
Bart

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Thu, Feb 14, 2008 at 01:02:55AM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 14 February 2008, Greg KH wrote:
> > On Wed, Feb 13, 2008 at 11:20:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > On Wednesday 13 February 2008, Greg KH wrote:
> > > > On Wed, Feb 13, 2008 at 09:28:24AM -0800, Greg KH wrote:
> > > > > On Wed, Feb 13, 2008 at 01:34:12PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > > > On Wednesday 13 February 2008, Greg KH wrote:
> > > > > > > On Wed, Feb 13, 2008 at 02:17:37AM +0000, Alan Cox wrote:
> > > > > > > > > Why does the calgary driver need this?  Can we just use pci_get_device()
> > > > > > > > > instead?  Why do you need to walk the device list backwards?  Do you get
> > > > > > > > > false positives going forward?
> > > > > > > > 
> > > > > > > > It doesn't look to be performance critical so the driver can
> > > > > > > > pci_get_device until the end and use the final hit anyway.
> > > > > > > 
> > > > > > > That would make more sense.
> > > > > > > 
> > > > > > > > IDE reverse is more problematic but nobody seems to use it.
> > > > > > > 
> > > > > > > I've seen two posters say they use it.  I'm wondering what it is really
> > > > > > > solving if they use it, and why if it's really needed, scsi never had to
> > > > > > > implement such a hack...
> > > > > > 
> > > > > > It is no longer solving anything, just adds more pain. ;)
> > > > > > 
> > > > > > [ The option comes from 2.2.x (so long before LABEL=/ and /dev/disk/by-id/
> > > > > >   became popular).  Some "off-board" controllers integrated on motherboards
> > > > > >   used to appear before "on-board" IDE on PCI bus so this option was meant
> > > > > >   to preserve the legacy ordering. ]
> > > > > > 
> > > > > > Since it is valid only when "Probe IDE PCI devices in the PCI bus order
> > > > > > (DEPRECATED)" config option is used it is already on its way out (though
> > > > > > marking it as obsoleted would make it more explicit).
> > > > > > 
> > > > > > I think that removing "ide=reverse" in 2.6.26 would be OK...
> > > > > 
> > > > > Great, thanks for your blessing.  I'll make up a patch and send it to
> > > > > you for approval.
> > > > 
> > > > How does the patch below look?  I didn't want to remove the whole config
> > > > option, as there is more to the logic than just the "reverse order"
> > > > stuff there.
> > > 
> > > looks fine,
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > 
> > Thanks.
> > 
> > > > If you don't mind, can I take this through the PCI tree so as to allow
> > > > the removal of this pci function afterwards?
> > > 
> > > [...]
> > > 
> > > great, could you also:
> > > - rebase it on top of the patch below
> > > - forward the patch below to Linus for 2.6.25
> > 
> > Sure, you want this to go in for .25, but not the one I just posted
> > removing this option, correct?  That should wait for .26?
> 
> Yes, lets give people the final call before actually removing this option
> (unless of course there is some urgent reason for removing it in .25).

No, no rush from me, I'll queue this up and send it to Linus in my next
batch.

thanks,

greg k-h

Re: [discuss] pci_get_device_reverse(), why does Calgary need this?

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

Greg KH <greg@kroah.com> writes:

> How does the patch below look?  I didn't want to remove the whole config
> option, as there is more to the logic than just the "reverse order"
> stuff there.

I think you miss Documentation - it's mentioned in ide.txt and
kernel-parameters.txt,

Andreas
-- 
 Andreas Jaeger, Director Platform / openSUSE, aj@suse.de
  SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
   Maxfeldstr. 5, 90409 Nürnberg, Germany
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 193 bytes --]

Re: [discuss] pci_get_device_reverse(), why does Calgary need this?

[Bartlomiej Zolnierkiewicz]

On Thursday 14 February 2008, Andreas Jaeger wrote:
> Greg KH <greg@kroah.com> writes:
> 
> > How does the patch below look?  I didn't want to remove the whole config
> > option, as there is more to the logic than just the "reverse order"
> > stuff there.
> 
> I think you miss Documentation - it's mentioned in ide.txt and
> kernel-parameters.txt,

+ drivers/ide/Kconfig

Greg, please update the patch (and add my S-o-B while at it).

Thanks,
Bart

Re: pci_get_device_reverse(), why does Calgary need this?

[Sergei Shtylyov]

> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide: mark "ide=reverse" option as obsolete

> - it is valid only if "Probe IDE PCI devices in the PCI bus order
>   (DEPRECATED)" config option is used

> - Greg needs to remove pci_get_device_reverse() for PCI core changes

> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

MBR, Sergei

Re: "ide=reverse" do we still need this?

[Bill Davidsen]

Greg KH wrote:
> On Wed, Feb 13, 2008 at 02:41:07AM +0100, Rene Herman wrote:
>> On 13-02-08 01:15, Greg KH wrote:
>>
>>> I'm reworking the pci device list logic (we currently keep all PCI
>>> devices in 2 lists, which isn't the nicest, we should be able to get
>>> away with only 1 list.)
>>> The only bother I've found so far is the pci_get_device_reverse()
>>> function, it's used in 2 places, IDE and the calgary driver.
>>> I'm curious if we really still support the ide=reverse option?  It's a
>>> config option that I don't think the distros still enable (SuSE does
>>> not).  Is this still needed these days?
>>> In digging, we changed this option in 2.2.x from being called
>>> "pci=reverse" and no one else seems to miss it.
>>> Any thoughts?
>> While details escape me somewhat again at the monment, a few months ago I 
>> was playing around with a PCI Promise IDE controller and needed ide=reverse 
>> to save me from having to switch disks around to still have a bootable 
>> system.
>>
>> Or some such. Not too clear anymore, but I remember it saved the day.
> 
> You couldn't just change the boot disk in grub?
> 
> Or use an initramfs and /dev/disk/by-id/ to keep any future moves
> stable?
> 
There are any number of things you can do when the system is booted, but 
the only thing you can do when the system won't boot is use kernel boot 
options.

This is primarily useful for old systems running modern software, such 
as old PC redeployed to network, external device control, or system 
monitoring. Upgrading the BIOS is no longer going to happen, and 
upgrading the hardware isn't cost effective, but keeping old systems out 
of the landfill is ecologically and financially sound.

The option is a holdover from the past, but so arm some of my clients 
and their hardware. ;-)
And *my* hardware, I might add, I am as cheap as anyone.

-- 
Bill Davidsen <davidsen@tmr.com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Wed, Feb 13, 2008 at 10:14:59AM -0800, Greg KH wrote:
> On Wed, Feb 13, 2008 at 07:47:11PM +0200, Muli Ben-Yehuda wrote:
> > On Wed, Feb 13, 2008 at 09:32:03AM -0800, Greg KH wrote:
> > 
> > > Is there some reason you aren't using the "real" PCI driver api here
> > > and registering a pci driver for these devices?  That would take the
> > > whole "loop over all pci devices" logic out of the code entirely.
> > 
> > I recall we had a reason, but I no longer recall what it was. Some
> > reason the "real" PCI driver API didn't fit at the time. If someone
> > were to whip up a working patch, I'd happily apply it.
> 
> "at the time"?  It's been in place since the 2.2 days :)
> 
> Is the problem that other drivers also want access to this PCI device
> for some reason?
> 
> I'll whip up a patch for you to test with in a bit...

Hm, that's wierd.  I thought I got something, until I realized that you
are doing a lot of logic before you ever even determine that your
hardware is present in the system.  Why are you calling
calgary_locate_bbars() and doing all of that work?  Or am I mising
something in the code flow here?

Also, it looks like you use the pci_get_device() to find the pci device,
then you do somethings, and then drop the device, never to use it again.

So, a traditional "probe" might not work as well, but it could be used
if you want to.

thanks,

greg k-h

Re: [discuss] pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Thu, Feb 14, 2008 at 01:11:42PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 14 February 2008, Andreas Jaeger wrote:
> > Greg KH <greg@kroah.com> writes:
> > 
> > > How does the patch below look?  I didn't want to remove the whole config
> > > option, as there is more to the logic than just the "reverse order"
> > > stuff there.
> > 
> > I think you miss Documentation - it's mentioned in ide.txt and
> > kernel-parameters.txt,
> 
> + drivers/ide/Kconfig
> 
> Greg, please update the patch (and add my S-o-B while at it).

Will do.  Andreas, thanks for pointing that out.

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Muli Ben-Yehuda]

On Thu, Feb 14, 2008 at 11:17:03PM -0800, Greg KH wrote:

> Hm, that's wierd.  I thought I got something, until I realized that
> you are doing a lot of logic before you ever even determine that
> your hardware is present in the system.  Why are you calling
> calgary_locate_bbars() and doing all of that work?  Or am I mising
> something in the code flow here?

Ok, it's all starting to come back to me. See below.

> Also, it looks like you use the pci_get_device() to find the pci
> device, then you do somethings, and then drop the device, never to
> use it again.
> 
> So, a traditional "probe" might not work as well, but it could be
> used if you want to.

We have a two stage initialization process. First, we need to be
initialized very early in order to be able to allocate relatively
large contiguous chunks of RAM. That requires detecting whether we're
on a Calgary/CalIOC2 system very early (detect_calgary(), called from
pci_iommu_alloc()). Then we have "regular" PCI initialization at a
later stage (calgary_iommu_init(), called from pci_iommu_init()),
which also calls calgary_locate_bbars(). Unlike a regular PCI device
which has its BARs in the configuration space, we need to probe BIOS
provided tables to find where our BARs are before we do anything else
with the "device representation" of the IOMMU. Note that the same
two-stage initialization scheme is used by all other x86 IOMMUs, for
similar reasons.

Now, Calgary is an isolation-capable IOMMU which has per-root-bus (as
opposed to global or per-BDF) IO address space. The reason we want to
access the pci_dev of each Calgary bus is to hang off of it the IOMMU
tables for all devices under that bus so that we end up using the
right translation table when a device asks for a DMA mapping. Hence
the loop in calgary_init, which loops over all pci_dev's of Calgary
busses, raises the ref count for each pci_dev to protect against
hot-unplug, and hooks up the IOMMU table for everything under that bus
to the pci_dev for the bus. Unlike a regular driver, our main entry
points are via the DMA-API, which takes a device's pci_dev, not
Calgary's. We then walk up the bus hierarchy and find the Calgary
pci_dev that is an ancestor of this device, and that gives us the
right IOMMU table to us. Note that this has to occur *before* PCI
device initialization, since we want to turn translation on before any
device will try to DMA.

In conclusion, our usage doesn't seem lika a good fit for the probe
approach, although it could probably converted provided we got the
ordering right with regards to regular PCI device
initialization. Doesn't seem to be worth the effort.

Cheers,
Muli

Re: "ide=reverse" do we still need this?

[Matthew Wilcox]

On Thu, Feb 14, 2008 at 12:16:42PM -0500, Bill Davidsen wrote:
> There are any number of things you can do when the system is booted, but 
> the only thing you can do when the system won't boot is use kernel boot 
> options.

Greg's not removing your option to boot the system using an old kernel
to set up a new kernel properly ;-)

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote:
> In conclusion, our usage doesn't seem lika a good fit for the probe
> approach, although it could probably converted provided we got the
> ordering right with regards to regular PCI device
> initialization. Doesn't seem to be worth the effort.

After reading this, and looking at the code again, I agree.  Thanks for
the great explaination, I'll leave the code alone now :)

thanks,

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Sam Ravnborg]

On Fri, Feb 15, 2008 at 07:20:08AM -0800, Greg KH wrote:
> On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote:
> > In conclusion, our usage doesn't seem lika a good fit for the probe
> > approach, although it could probably converted provided we got the
> > ordering right with regards to regular PCI device
> > initialization. Doesn't seem to be worth the effort.
> 
> After reading this, and looking at the code again, I agree.  Thanks for
> the great explaination, I'll leave the code alone now :)

Copy the nice explanation from the mail into the code as a comment
somewhere?

	Sam

Re: pci_get_device_reverse(), why does Calgary need this?

[yong xue]

2008/2/15, Sam Ravnborg <sam@ravnborg.org>:
> On Fri, Feb 15, 2008 at 07:20:08AM -0800, Greg KH wrote:
>  > On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote:
>  > > In conclusion, our usage doesn't seem lika a good fit for the probe
>  > > approach, although it could probably converted provided we got the
>  > > ordering right with regards to regular PCI device
>  > > initialization. Doesn't seem to be worth the effort.
>  >
>  > After reading this, and looking at the code again, I agree.  Thanks for
>  > the great explaination, I'll leave the code alone now :)
>
>
> Copy the nice explanation from the mail into the code as a comment
>  somewhere?
>         Sam

I think  it is good  to do so.

xueyong
>
> -
>  To unsubscribe from this list: send the line "unsubscribe linux-ide" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Fri, Feb 15, 2008 at 04:31:40PM +0100, Sam Ravnborg wrote:
> On Fri, Feb 15, 2008 at 07:20:08AM -0800, Greg KH wrote:
> > On Fri, Feb 15, 2008 at 09:48:27AM +0200, Muli Ben-Yehuda wrote:
> > > In conclusion, our usage doesn't seem lika a good fit for the probe
> > > approach, although it could probably converted provided we got the
> > > ordering right with regards to regular PCI device
> > > initialization. Doesn't seem to be worth the effort.
> > 
> > After reading this, and looking at the code again, I agree.  Thanks for
> > the great explaination, I'll leave the code alone now :)
> 
> Copy the nice explanation from the mail into the code as a comment
> somewhere?

That would be nice.  Muli, want to make a patch for this?

thanks,

greg k-h

Re: pci_get_device_reverse(), why does Calgary need this?

[Muli Ben-Yehuda]

On Fri, Feb 15, 2008 at 10:28:22AM -0800, Greg KH wrote:

> That would be nice.  Muli, want to make a patch for this?

Sure, I'll put something together.

Cheers,
Muli

Re: "ide=reverse" do we still need this?

[Ken Moffat]

On Wed, Feb 13, 2008 at 03:32:49PM +0000, Ken Moffat wrote:
> On Tue, Feb 12, 2008 at 08:43:04PM -0800, Greg KH wrote:
> > 
> > Can't you just boot with /dev/disk/by-id/ and an initramfs to not have
> > to worry about such a thing in the future?
> > 

 Initramfs isn't something I've ever tried, so I'm not about to rush
into it on the server.  Maybe I'll try it on a desktop one day.
Anyway, I've now got it running without ide=reverse, details follow
for anybody else who gets a similar problem in the future.

>  Can comebody remind me what the initramfs is for in that situation,
> please ?  From the little I've noticed, I thought the /dev/disk/by-id
> part went into fstab ?  At the moment, I just build the things I
> think I need in to the kernel on that box, without modules.
> 
 And for the next person asking this, it seems to be so that you
can specify the root= parameter.

>  Anyway, I'll try to find time to read my notes to see if I can
> identify what happened/when, and to take the box down again so I
> can try to confirm exactly what the problem is, if it still exists.
> I certainly won't be taking it down until I've written my weekly
> backups to tape at the weekend, so maybe not before next week.
> 

 Turns out I was wrong about having a SATA disk for the system - I
used to, but then I needed to separate the backups into separate r/o
and r/w filesystems when nfs no longer let me export part of a ro fs
as rw.  In the change, my staging area for writing to tape or DVD
moved to the system disk, and the only big-enough disk I could get
locally was parallel ide.  So in that setup, ide on a card comes
first.
> > Have you tried the PATA drivers instead of IDE to see if this solves the
> > "moves around" issue?  If they work, then you would not need the command
> > line option at all.
> 

 My first thought was to try using libata for the drives on the
add-on card (sii0680), although it's marked as experimental.  Maybe
I picked the wrong driver, but they didn't show up.  Reverted to
previous config.

 Changed to mount-by-label so that I don't have to change fstab for
the old and new kernels.

 Moved the main drive and the CD to libata (sii still old IDE) -
specify sda instead of hda in root=, change system scripts referencing
drives by name (for SMART - system disk is again -d ata, the data raid
moved from hd{e,g} to hd{a,c}), now seems to be working but I expect
I'll notice a few things more in my scripts over the next days.

 I also discovered that lilo needed the real node specified in root=
to exist.  It complained, so I added a symlink to the hdaX node -
panic'd trying to load rootfs from 0307.  Reboot to old kernel, run mknod on /dev,
repeat, booted.

Ken
-- 
das eine Mal als Tragödie, das andere Mal als Farce

Re: [discuss] pci_get_device_reverse(), why does Calgary need this?

[Greg KH]

On Thu, Feb 14, 2008 at 01:11:42PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 14 February 2008, Andreas Jaeger wrote:
> > Greg KH <greg@kroah.com> writes:
> > 
> > > How does the patch below look?  I didn't want to remove the whole config
> > > option, as there is more to the logic than just the "reverse order"
> > > stuff there.
> > 
> > I think you miss Documentation - it's mentioned in ide.txt and
> > kernel-parameters.txt,
> 
> + drivers/ide/Kconfig
> 
> Greg, please update the patch (and add my S-o-B while at it).

Now done, thanks.

greg k-h