commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Sébastien Dugué]

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


  Hi Bjorn,

 your commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 broke the serial console
on my box. Adding 'legacy_serial.force=1' to my boot param as a workaround
solves the issue, but this may be hiding bugs in Linux PnP support or
in my firmware.

  The box is a dual HT Xeon running a vanilla 2.6.22 x86_64 kernel

  here is my .config:

CONFIG_PNP=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_PCI=y
CONFIG_SERIAL_8250_PNP=y
CONFIG_SERIAL_8250_NR_UARTS=4
CONFIG_SERIAL_8250_RUNTIME_UARTS=4
CONFIG_SERIAL_8250_EXTENDED=y
CONFIG_SERIAL_8250_SHARE_IRQ=y
CONFIG_SERIAL_8250_DETECT_IRQ=y

  lspci output attached.
 
  Any ideas to help me debug this?

  If you need more info (like DSDT dump), just ask.

  Thanks,

  Sébastien.



[-- Attachment #2: lspci.txt --]
[-- Type: text/plain, Size: 2176 bytes --]

00:00.0 Host bridge: Intel Corporation E7520 Memory Controller Hub (rev 0c)
00:00.1 Class ff00: Intel Corporation E7525/E7520 Error Reporting Registers (rev 0c)
00:01.0 System peripheral: Intel Corporation E7520 DMA Controller (rev 0c)
00:02.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A (rev 0c)
00:04.0 PCI bridge: Intel Corporation E7525/E7520 PCI Express Port B (rev 0c)
00:05.0 PCI bridge: Intel Corporation E7520 PCI Express Port B1 (rev 0c)
00:06.0 PCI bridge: Intel Corporation E7520 PCI Express Port C (rev 0c)
00:07.0 PCI bridge: Intel Corporation E7520 PCI Express Port C1 (rev 0c)
00:1c.0 PCI bridge: Intel Corporation 6300ESB 64-bit PCI-X Bridge (rev 02)
00:1d.0 USB Controller: Intel Corporation 6300ESB USB Universal Host Controller (rev 02)
00:1d.1 USB Controller: Intel Corporation 6300ESB USB Universal Host Controller (rev 02)
00:1d.4 System peripheral: Intel Corporation 6300ESB Watchdog Timer (rev 02)
00:1d.5 PIC: Intel Corporation 6300ESB I/O Advanced Programmable Interrupt Controller (rev 02)
00:1d.7 USB Controller: Intel Corporation 6300ESB USB2 Enhanced Host Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev 0a)
00:1f.0 ISA bridge: Intel Corporation 6300ESB LPC Interface Controller (rev 02)
00:1f.1 IDE interface: Intel Corporation 6300ESB PATA Storage Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 6300ESB SMBus Controller (rev 02)
01:00.0 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge A (rev 09)
01:00.2 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge B (rev 09)
02:01.0 Ethernet controller: Intel Corporation 82544EI Gigabit Ethernet Controller (Copper) (rev 02)
02:03.0 SCSI storage controller: Adaptec AIC-7902B U320 (rev 10)
02:03.1 SCSI storage controller: Adaptec AIC-7902B U320 (rev 10)
03:01.0 Ethernet controller: Intel Corporation 82544EI Gigabit Ethernet Controller (Copper) (rev 02)
08:01.0 VGA compatible controller: ATI Technologies Inc Radeon RV100 QY [Radeon 7000/VE]
08:02.0 Ethernet controller: Intel Corporation 82541GI Gigabit Ethernet Controller
08:03.0 Ethernet controller: Intel Corporation 82541GI Gigabit Ethernet Controller

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Tuesday 24 July 2007 08:28:05 am Sébastien Dugué wrote:
>  your commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 broke the serial console
> on my box. Adding 'legacy_serial.force=1' to my boot param as a workaround
> solves the issue, but this may be hiding bugs in Linux PnP support or
> in my firmware.

Thanks for your report.  We need to figure out why the 8250_pnp driver
didn't find your serial console device.  Can you confirm that you also
have CONFIG_ACPI and CONFIG_PNPACPI in your .config?

If you have those, and it still doesn't work, can you collect the DSDT
dump, the output of "grep . /sys/bus/pnp/devices/*/*", and the dmesg
from your "legacy_serial.force=1" boot?  Then we can tell which port
the blind probe finds and whether it's described somewhere by ACPI.

Thanks,
  Bjorn

>   The box is a dual HT Xeon running a vanilla 2.6.22 x86_64 kernel
> 
>   here is my .config:
> 
> CONFIG_PNP=y
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_8250_PCI=y
> CONFIG_SERIAL_8250_PNP=y
> CONFIG_SERIAL_8250_NR_UARTS=4
> CONFIG_SERIAL_8250_RUNTIME_UARTS=4
> CONFIG_SERIAL_8250_EXTENDED=y
> CONFIG_SERIAL_8250_SHARE_IRQ=y
> CONFIG_SERIAL_8250_DETECT_IRQ=y
> 
>   lspci output attached.
>  
>   Any ideas to help me debug this?
> 
>   If you need more info (like DSDT dump), just ask.
> 
>   Thanks,
> 
>   Sébastien.
> 
> 
> 

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Bjorn Helgaas wrote:
> On Tuesday 24 July 2007 08:28:05 am Sébastien Dugué wrote:
>>  your commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 broke the serial console
>> on my box. Adding 'legacy_serial.force=1' to my boot param as a workaround
>> solves the issue, but this may be hiding bugs in Linux PnP support or
>> in my firmware.
> 
> Thanks for your report.  We need to figure out why the 8250_pnp driver
> didn't find your serial console device.  Can you confirm that you also
> have CONFIG_ACPI and CONFIG_PNPACPI in your .config?
> 
> If you have those, and it still doesn't work, can you collect the DSDT
> dump, the output of "grep . /sys/bus/pnp/devices/*/*", and the dmesg
> from your "legacy_serial.force=1" boot?  Then we can tell which port
> the blind probe finds and whether it's described somewhere by ACPI.

hrm...

     x86, serial: convert legacy COM ports to platform devices

     Make x86 COM ports into platform devices and don't probe for them
     if we have PNP.

This seems like it will break decades-long-working stuff, in favor of 
breaking new ground in our favorite area, "trusting the BIOS."

It's just not worth it for serial ports, IMO.  Serial ports are 
something that just shouldn't break at this late stage in the game.  My 
new Intel platform boxes don't even have serial ports, so I question the 
value of messing with serial port probing even more... because... just 
wait a year, and your box won't have a serial port either!  :)

I certainly don't object to the use of platform devices (or isa_driver), 
but the probe change seems questionable.  That's sorta analagous to 
rewriting the floppy driver probe routine.  Sure you could do it... but 
why risk all that damage and go through debugging all over again?

It seems clear from this report that we cannot, should not, trust BIOS 
for something (a) so simple and (b) that has been working for over a decade.

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Andrew Morton]

On Tue, 24 Jul 2007 13:49:55 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> Bjorn Helgaas wrote:
> > On Tuesday 24 July 2007 08:28:05 am S__bastien Dugu__ wrote:
> >>  your commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 broke the serial console
> >> on my box. Adding 'legacy_serial.force=1' to my boot param as a workaround
> >> solves the issue, but this may be hiding bugs in Linux PnP support or
> >> in my firmware.
> > 
> > Thanks for your report.  We need to figure out why the 8250_pnp driver
> > didn't find your serial console device.  Can you confirm that you also
> > have CONFIG_ACPI and CONFIG_PNPACPI in your .config?
> > 
> > If you have those, and it still doesn't work, can you collect the DSDT
> > dump, the output of "grep . /sys/bus/pnp/devices/*/*", and the dmesg
> > from your "legacy_serial.force=1" boot?  Then we can tell which port
> > the blind probe finds and whether it's described somewhere by ACPI.
> 
> hrm...
> 
>      x86, serial: convert legacy COM ports to platform devices
> 
>      Make x86 COM ports into platform devices and don't probe for them
>      if we have PNP.
> 
> This seems like it will break decades-long-working stuff, in favor of 
> breaking new ground in our favorite area, "trusting the BIOS."
> 
> It's just not worth it for serial ports, IMO.  Serial ports are 
> something that just shouldn't break at this late stage in the game.  My 
> new Intel platform boxes don't even have serial ports, so I question the 
> value of messing with serial port probing even more... because... just 
> wait a year, and your box won't have a serial port either!  :)
> 
> I certainly don't object to the use of platform devices (or isa_driver), 
> but the probe change seems questionable.  That's sorta analagous to 
> rewriting the floppy driver probe routine.  Sure you could do it... but 
> why risk all that damage and go through debugging all over again?
> 
> It seems clear from this report that we cannot, should not, trust BIOS 
> for something (a) so simple and (b) that has been working for over a decade.
> 

Well I queued up a tentative revert patch but then I re-read the changlog:

    x86, serial: convert legacy COM ports to platform devices
    
    Make x86 COM ports into platform devices and don't probe for them
    if we have PNP.
    
    This prevents double discovery, where a device was found both by
    the legacy probe and by 8250_pnp, e.g.,
    
        serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
        00:02: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
    
    This also means IRDA devices without a UART PNP ID will no longer be
    claimed by the serial driver, which might require changes in IRDA
    drivers and administration.
    
    In addition to this patch, you may need to configure a setserial init
    script, e.g., /etc/init.d/setserial, so it doesn't poke legacy UART
    stuff back in.  On Debian, "dpkg-reconfigure setserial" with the "kernel"
    option does this.
    
    To force the old legacy probe behavior even when we have PNPBIOS or
    ACPI, load the new legacy_serial module (or build 8250 static) with
    the "legacy_serial.force" option.

So 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 fixed a bunch of longstanding
nasties while breaking Sebastien's machine (at least).

We of course want the best of both worlds here, so I'll keep the revert
patch in -mm for a while, see what happens.  Although I can't immediately
see any way in which this can be fixed...

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Andrew Morton wrote:
> Well I queued up a tentative revert patch but then I re-read the changlog:
> 
>     x86, serial: convert legacy COM ports to platform devices
>     
>     Make x86 COM ports into platform devices and don't probe for them
>     if we have PNP.
>     
>     This prevents double discovery, where a device was found both by
>     the legacy probe and by 8250_pnp, e.g.,
>     
>         serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
>         00:02: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A

If this actually causes a problem, then the drivers are buggy.  We have 
standard APIs for checking this stuff...

Turning off legacy serial probing is just a bandaid, the resource bug is 
still there [by definition, if double-probing problems exist].


>     This also means IRDA devices without a UART PNP ID will no longer be
>     claimed by the serial driver, which might require changes in IRDA
>     drivers and administration.

This was completely unaddressed, I presume?


>     In addition to this patch, you may need to configure a setserial init
>     script, e.g., /etc/init.d/setserial, so it doesn't poke legacy UART
>     stuff back in.  On Debian, "dpkg-reconfigure setserial" with the "kernel"
>     option does this.

So this patch breaks some userspace too?


>     To force the old legacy probe behavior even when we have PNPBIOS or
>     ACPI, load the new legacy_serial module (or build 8250 static) with
>     the "legacy_serial.force" option.
> 
> So 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 fixed a bunch of longstanding
> nasties while breaking Sebastien's machine (at least).
> 
> We of course want the best of both worlds here, so I'll keep the revert
> patch in -mm for a while, see what happens.  Although I can't immediately
> see any way in which this can be fixed...

I see VERY few positives, a whole lot of trouble, and a couple band-aids.

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Maciej W. Rozycki]

On Tue, 24 Jul 2007, Jeff Garzik wrote:

> It seems clear from this report that we cannot, should not, trust BIOS for
> something (a) so simple and (b) that has been working for over a decade.

 And (c) something BIOS writers have never ever in their most unlikely 
imagination expected to be trusted for.

  Maciej

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Tuesday 24 July 2007 12:17:36 pm Maciej W. Rozycki wrote:
> On Tue, 24 Jul 2007, Jeff Garzik wrote:
> 
> > It seems clear from this report that we cannot, should not, trust BIOS for
> > something (a) so simple and (b) that has been working for over a decade.
> 
>  And (c) something BIOS writers have never ever in their most unlikely 
> imagination expected to be trusted for.

I don't think it's quite so clear-cut.  It is true that "poke at 0x3e8,
and if it responds, assume it's a 16550 with IRQ 4" is simple.  But it
doesn't always work.  Google for "irda setserial" and you'll find many
cases where the serial driver's blind probe erroneously claims an IRDA
device.  The SIR mode of IRDA devices is basically 16550-compatible,
so this wouldn't be a big problem, except that the blind probe often
assumes the wrong IRQ.  So users have to use setserial to fix up the
incorrect assumptions made by the blind probe.

We haven't debugged the problem on Sebastien's machine yet.  I suspect
we'll find that his serial port *is* described by ACPI, but that there's
some little difference in the way Linux discovers those devices compared
to how Windows does it.  If we figure out how to use ACPI more like
Windows does, I think we'll fix several little issues, including the one
on Sebastien's machine.

We have a whole laundry list of minor issues because we either don't
listen to the BIOS at all, or we use it differently than Windows does.
Here are a few off the top of my head:

  - IRDA drivers have platform-specific code to "preconfigure" (discover
    and reprogram) bridges on the way to the IR device
  - Hardware sensor drivers conflict with ACPI embedded controller
    drivers, so every once in a while, they return bogus readings
  - PCMCIA devices grab resources already in use by a PNP device,
    causing the PNP device to stop working
  - Linux enumerates CPUs with the MADT; I think Windows uses the ACPI
    namespace.  Sometimes there are multiple MADTs, and sometimes Linux
    uses the wrong one.

If we keep papering over these problems by ignoring what ACPI is trying
to tell us, we're going to be adding machine-specific hacks forever.
Of course, there are ACPI bugs.  But Windows does rely on ACPI, and
Microsoft doesn't want to add those per-platform hacks any more than
we do.  So we might as well try to take advantage of the ACPI testing
they do.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Bjorn Helgaas wrote:
> On Tuesday 24 July 2007 12:17:36 pm Maciej W. Rozycki wrote:
>> On Tue, 24 Jul 2007, Jeff Garzik wrote:
>>
>>> It seems clear from this report that we cannot, should not, trust BIOS for
>>> something (a) so simple and (b) that has been working for over a decade.
>>  And (c) something BIOS writers have never ever in their most unlikely 
>> imagination expected to be trusted for.
> 
> I don't think it's quite so clear-cut.  It is true that "poke at 0x3e8,
> and if it responds, assume it's a 16550 with IRQ 4" is simple.  But it
> doesn't always work.  Google for "irda setserial" and you'll find many
> cases where the serial driver's blind probe erroneously claims an IRDA
> device.  The SIR mode of IRDA devices is basically 16550-compatible,
> so this wouldn't be a big problem, except that the blind probe often
> assumes the wrong IRQ.  So users have to use setserial to fix up the
> incorrect assumptions made by the blind probe.
> 
> We haven't debugged the problem on Sebastien's machine yet.  I suspect
> we'll find that his serial port *is* described by ACPI, but that there's
> some little difference in the way Linux discovers those devices compared
> to how Windows does it.  If we figure out how to use ACPI more like
> Windows does, I think we'll fix several little issues, including the one
> on Sebastien's machine.

You have not fixed the double-probe problem either.  That should have 
been fixed before 7e92b4fc34 was even considered for upstream.


> We have a whole laundry list of minor issues because we either don't
> listen to the BIOS at all, or we use it differently than Windows does.

Getting [back] to this thread, I know that most versions of Windows poke 
the serial port directly.  It's pretty obvious when running Windows in 
an emulator.


> Here are a few off the top of my head:
> 
>   - IRDA drivers have platform-specific code to "preconfigure" (discover
>     and reprogram) bridges on the way to the IR device
>   - Hardware sensor drivers conflict with ACPI embedded controller
>     drivers, so every once in a while, they return bogus readings

Driver bug, completely unrelated to not "listen[ing] to the BIOS"


>   - PCMCIA devices grab resources already in use by a PNP device,
>     causing the PNP device to stop working

ditto


>   - Linux enumerates CPUs with the MADT; I think Windows uses the ACPI
>     namespace.  Sometimes there are multiple MADTs, and sometimes Linux
>     uses the wrong one.

Color me skeptical.  I think we would have bug reports if we were really 
getting this wrong a lot of the time.


> If we keep papering over these problems by ignoring what ACPI is trying
> to tell us, we're going to be adding machine-specific hacks forever.
> Of course, there are ACPI bugs.  But Windows does rely on ACPI, and
> Microsoft doesn't want to add those per-platform hacks any more than
> we do.  So we might as well try to take advantage of the ACPI testing
> they do.

You seem to be missing that ignoring BIOS is often a VERY GOOD thing, 
that has served us well many many times in the past.

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/24/07, Jeff Garzik <jeff@garzik.org> wrote:
>
> You seem to be missing that ignoring BIOS is often a VERY GOOD thing,
> that has served us well many many times in the past.
>
I have a system that has the same problem, and it turns out that FW
missed PNP0501 is DSDT for uart. and add that it into DSDT works well.

Or we can add some DMI check up, to enable legacy_serial for old BIOS
before some date point?

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Tuesday 24 July 2007 02:33:05 pm Yinghai Lu wrote:
> I have a system that has the same problem, and it turns out that FW
> missed PNP0501 is DSDT for uart. and add that it into DSDT works well.

Is this FW that has been shipped?  Can you give any more details,
like DMI info and a copy of the DSDT?  We can't expect users to
upgrade their firmware or use a custom DSDT.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/24/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Tuesday 24 July 2007 02:33:05 pm Yinghai Lu wrote:
> > I have a system that has the same problem, and it turns out that FW
> > missed PNP0501 is DSDT for uart. and add that it into DSDT works well.
>
> Is this FW that has been shipped?  Can you give any more details,
> like DMI info and a copy of the DSDT?  We can't expect users to
> upgrade their firmware or use a custom DSDT.

The system is not shipped yet.
Normally PNP0501 is coming with superio section in DSDT. So i think
late BIOS if have acpi there, that should be there already.
Problem is that some new design may get rid of superio, but SB could
have extra uart for serial port. at that case BIOS may not have that
PNP0501...

I don't think revert is reasonable.

or we can make legacy_serial.force=1 is default at this point.

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Tuesday 24 July 2007 10:27:26 pm Yinghai Lu wrote:
> On 7/24/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Tuesday 24 July 2007 02:33:05 pm Yinghai Lu wrote:
> > > I have a system that has the same problem, and it turns out that FW
> > > missed PNP0501 is DSDT for uart. and add that it into DSDT works well.
> >
> > Is this FW that has been shipped?  Can you give any more details,
> > like DMI info and a copy of the DSDT?  We can't expect users to
> > upgrade their firmware or use a custom DSDT.
> 
> The system is not shipped yet.
> Normally PNP0501 is coming with superio section in DSDT. So i think
> late BIOS if have acpi there, that should be there already.
> Problem is that some new design may get rid of superio, but SB could
> have extra uart for serial port. at that case BIOS may not have that
> PNP0501...

If the system has a non-PCI UART that is usable by the OS, it
should be described by ACPI.  This is true regardless of whether
the UART is attached via superio or elsewhere.

So in this case, my patch just helped expose a defect in the
firmware, and it sounds like the firmware was fixed before
ever being shipped.

> or we can make legacy_serial.force=1 is default at this point.

At which point?  We already do the legacy probe if there are no
PNP devices.  Do you mean we should also do the legacy probe if
there are PNP devices, but no serial devices were found?  That
might be reasonable to do, but wouldn't have prevented the problem
on Sebastien's machine, because his machine *does* have PNP UARTs.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Tuesday 24 July 2007 10:27:26 pm Yinghai Lu wrote:
> > On 7/24/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > On Tuesday 24 July 2007 02:33:05 pm Yinghai Lu wrote:
> > > > I have a system that has the same problem, and it turns out that FW
> > > > missed PNP0501 is DSDT for uart. and add that it into DSDT works well.
> > >
> > > Is this FW that has been shipped?  Can you give any more details,
> > > like DMI info and a copy of the DSDT?  We can't expect users to
> > > upgrade their firmware or use a custom DSDT.
> >
> > The system is not shipped yet.
> > Normally PNP0501 is coming with superio section in DSDT. So i think
> > late BIOS if have acpi there, that should be there already.
> > Problem is that some new design may get rid of superio, but SB could
> > have extra uart for serial port. at that case BIOS may not have that
> > PNP0501...
>
> If the system has a non-PCI UART that is usable by the OS, it
> should be described by ACPI.  This is true regardless of whether
> the UART is attached via superio or elsewhere.
>
> So in this case, my patch just helped expose a defect in the
> firmware, and it sounds like the firmware was fixed before
> ever being shipped.

Yeah.

>
> > or we can make legacy_serial.force=1 is default at this point.
>
> At which point?  We already do the legacy probe if there are no
> PNP devices.  Do you mean we should also do the legacy probe if
> there are PNP devices, but no serial devices were found?  That
> might be reasonable to do, but wouldn't have prevented the problem
> on Sebastien's machine, because his machine *does* have PNP UARTs.

but still could have problem. if the system does not have any uart,
and So we can not find that in DSDT...and we add legacy again...that
is not right.

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Wednesday 25 July 2007 09:57:47 am Yinghai Lu wrote:
> On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Tuesday 24 July 2007 10:27:26 pm Yinghai Lu wrote:
> > > or we can make legacy_serial.force=1 is default at this point.
> >
> > At which point?  We already do the legacy probe if there are no
> > PNP devices.  Do you mean we should also do the legacy probe if
> > there are PNP devices, but no serial devices were found?  That
> > might be reasonable to do, but wouldn't have prevented the problem
> > on Sebastien's machine, because his machine *does* have PNP UARTs.
> 
> but still could have problem. if the system does not have any uart,
> and So we can not find that in DSDT...and we add legacy again...that
> is not right.

I'm sorry, I don't understand what you're saying.

The behavior with my patch is: If we have PNPBIOS or ACPI, and it
does not describe any UARTs, we assume the box has no UARTs.  If we
don't have PNPBIOS or ACPI, we blindly probe for legacy UARTs.

What behavior are you suggesting?

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Wednesday 25 July 2007 09:57:47 am Yinghai Lu wrote:
> > On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > On Tuesday 24 July 2007 10:27:26 pm Yinghai Lu wrote:
> > > > or we can make legacy_serial.force=1 is default at this point.
> > >
> > > At which point?  We already do the legacy probe if there are no
> > > PNP devices.  Do you mean we should also do the legacy probe if
> > > there are PNP devices, but no serial devices were found?  That
> > > might be reasonable to do, but wouldn't have prevented the problem
> > > on Sebastien's machine, because his machine *does* have PNP UARTs.
> >
> > but still could have problem. if the system does not have any uart,
> > and So we can not find that in DSDT...and we add legacy again...that
> > is not right.
>
> I'm sorry, I don't understand what you're saying.
>
> The behavior with my patch is: If we have PNPBIOS or ACPI, and it
> does not describe any UARTs, we assume the box has no UARTs.  If we
> don't have PNPBIOS or ACPI, we blindly probe for legacy UARTs.
>
> What behavior are you suggesting?

*** If we have PNPBIOS or ACPI, and it  does describe any UARTs, we
assume the box has no other legacy UARTs.
*** If we have PNPBIOS or ACPI, and it  does not describe any UARTs,
we blindly probe for legacy UARTs. ----------in case old box DSDT
doesn't have PNP0501 there in DSDT.
If we don't have PNPBIOS or ACPI, we blindly probe for legacy UARTs.

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Tuesday 24 July 2007 02:13:49 pm Jeff Garzik wrote:
> Bjorn Helgaas wrote:
> >   - Linux enumerates CPUs with the MADT; I think Windows uses the ACPI
> >     namespace.  Sometimes there are multiple MADTs, and sometimes Linux
> >     uses the wrong one.
> 
> Color me skeptical.  I think we would have bug reports if we were really 
> getting this wrong a lot of the time.

We do have bug reports, like http://bugzilla.kernel.org/show_bug.cgi?id=7465
Not very many, but enough that I'd like to get to the root cause.

> You seem to be missing that ignoring BIOS is often a VERY GOOD thing, 
> that has served us well many many times in the past.

I can see that I'm not going to win this argument :-)

But I would like to find and fix the problem with Sebastien's machine,
because the patch does fix real problems with IRDA and I think the fix
for Sebastien is likely to fix other PNP issues.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Bjorn Helgaas wrote:
> On Tuesday 24 July 2007 02:13:49 pm Jeff Garzik wrote:
>> Bjorn Helgaas wrote:
>>>   - Linux enumerates CPUs with the MADT; I think Windows uses the ACPI
>>>     namespace.  Sometimes there are multiple MADTs, and sometimes Linux
>>>     uses the wrong one.
>> Color me skeptical.  I think we would have bug reports if we were really 
>> getting this wrong a lot of the time.
> 
> We do have bug reports, like http://bugzilla.kernel.org/show_bug.cgi?id=7465
> Not very many, but enough that I'd like to get to the root cause.
> 
>> You seem to be missing that ignoring BIOS is often a VERY GOOD thing, 
>> that has served us well many many times in the past.
> 
> I can see that I'm not going to win this argument :-)
> 
> But I would like to find and fix the problem with Sebastien's machine,
> because the patch does fix real problems with IRDA and I think the fix
> for Sebastien is likely to fix other PNP issues.

Please go back and fix the original issue!

If you are having problems caused by double-probing, the fix is NOT to 
remove one the probes.  The fix is to ensure use of proper resource 
reservation, and in some cases, add co-driver-awareness.

Your entire justification for the patch has not been proven, since you 
demonstrably have not solved the original bug for which your patch was 
intended to solve.

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Tuesday 24 July 2007 02:40:42 pm Jeff Garzik wrote:
> Please go back and fix the original issue!
> 
> If you are having problems caused by double-probing, the fix is NOT to 
> remove one the probes.  The fix is to ensure use of proper resource 
> reservation, and in some cases, add co-driver-awareness.

Keith Owens did report an issue with the double probe, but I confess
I don't fully understand it.

The real problem was that the serial driver claimed a non-serial
device.  On many laptops, there's an IR device at 0x3e8, IRQ 3.
The serial driver claimed this as ttyS2, using IRQ 4.  So if you
wanted to use the IR device, you had to either:

  - use setserial to make the serial driver forget about ttyS2
    so an IR driver could claim it, or

  - use setserial to change the IRQ to 3 and just use the device
    in SIR mode, which is 16550-compatible so you can use the
    serial driver

I didn't express that very clearly in the changelog.

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Bjorn Helgaas wrote:
> Keith Owens did report an issue with the double probe, but I confess
> I don't fully understand it.

heh, ok...


> The real problem was that the serial driver claimed a non-serial
> device.  On many laptops, there's an IR device at 0x3e8, IRQ 3.
> The serial driver claimed this as ttyS2, using IRQ 4.  So if you
> wanted to use the IR device, you had to either:
> 
>   - use setserial to make the serial driver forget about ttyS2
>     so an IR driver could claim it, or
> 
>   - use setserial to change the IRQ to 3 and just use the device
>     in SIR mode, which is 16550-compatible so you can use the
>     serial driver
> 
> I didn't express that very clearly in the changelog.

That cannot be a justification for breaking serial port probe that has 
been working for 10+ years.

The VAST MAJORITY of computers, all told, have one or more serial ports 
in the expected places.  We must consider the majority first, then 
tackle edge cases like weirdo laptops that can be detected by other means.

Now that we have established the double-probe problem as potentially 
bogus, and not investigated at all, we are left with:

You changed legacy serial probing, affecting millions of computers, due 
to a few machines with unusual IR configurations.

At this point the "revert! revert!" submarine alarm is sounding quite 
loudly.  You can fix IR without breaking legacy serial.

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Alan Cox]

> That cannot be a justification for breaking serial port probe that has 
> been working for 10+ years.

Agree. With my "nearest thing we have to a serial maintainer" hat on
please revert this Andrew. Bjorn - lets discuss putting the right APIs in
place so you can busy out serial ports from other drivers when they are a
shared resource.

Alan

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Alan Cox]

On 25 Jul 2007 18:30:50 +0200
Andi Kleen <andi@firstfloor.org> wrote:

> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> 
> > > That cannot be a justification for breaking serial port probe that has 
> > > been working for 10+ years.
> > 
> > Agree. With my "nearest thing we have to a serial maintainer" hat on
> > please revert this Andrew. Bjorn - lets discuss putting the right APIs in
> > place so you can busy out serial ports from other drivers when they are a
> > shared resource.
> 
> One alternative would be to make the default behaviour dependent on 
> DMI year and acpi enabled. On modern systems with ACPI ACPI serial
> probing should work (modulo bugs which can be probably worked out) 
> Let's say >= 2003 ? 

That isn't the problem with the IR stuff. In many cases the user wants
the port to be showing up as a serial port for SIR usage. We need a clean
automatic SIR/FIR switching for them. It has nothing to do with whether
the ACPI data is trustworthy or not.

Alan

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Wednesday 25 July 2007 09:48:08 am Alan Cox wrote:
> That isn't the problem with the IR stuff. In many cases the user wants
> the port to be showing up as a serial port for SIR usage. We need a clean
> automatic SIR/FIR switching for them. It has nothing to do with whether
> the ACPI data is trustworthy or not.

If we were starting fresh, I think the best way would be for the IR
driver to claim the whole PNP device, which typically has two I/O port
ranges, like this:

  00:03 SMCf010 SMC Fast Infrared Port
    state = active
        io 0x3e8-0x3ef
        io 0x100-0x10f
        irq 3
        dma 3

There could be an ioctl or equivalent to request SIR mode, and the
IR driver could use serial8250_register_port() when entering SIR
mode and serial8250_unregister_port() when leaving.

The IR driver would be the clear owner of the entire device.  Of course,
this still only works if we get rid of the 8250 blind probe for 0x3e8
or figure out some API for the IR driver to tell the 8250 driver to get
its mitts off.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Bjorn Helgaas wrote:
> The IR driver would be the clear owner of the entire device.  Of course,
> this still only works if we get rid of the 8250 blind probe for 0x3e8
> or figure out some API for the IR driver to tell the 8250 driver to get
> its mitts off.


Load driver B before loading driver A.

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Alan Cox]

On Wed, 25 Jul 2007 12:47:14 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> Bjorn Helgaas wrote:
> > The IR driver would be the clear owner of the entire device.  Of course,
> > this still only works if we get rid of the 8250 blind probe for 0x3e8
> > or figure out some API for the IR driver to tell the 8250 driver to get
> > its mitts off.
> 
> 
> Load driver B before loading driver A.

Completely impractical. Devices may be hotplug, there may be combinations
of ordering required and since you have to build serial in for serial
console its a total non starter


Alan

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Alan Cox]

> The IR driver would be the clear owner of the entire device.  Of course,
> this still only works if we get rid of the 8250 blind probe for 0x3e8
> or figure out some API for the IR driver to tell the 8250 driver to get
> its mitts off.

As far as I can see all we need is something of the form


	serial8250_claim_uart(unsigned long ioaddr, int type)  

(type as we don't have iomap on most platforms yet)

	returning

		-EBUSY		"serial port is open"
		-ENOENT		"never heard of it"
		0		"ok"

and
	serial8250_unclaim_uart(unsigned long ioaddr, int type)

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Alan Cox]

>   - use setserial to make the serial driver forget about ttyS2
>     so an IR driver could claim it, or
> 
>   - use setserial to change the IRQ to 3 and just use the device
>     in SIR mode, which is 16550-compatible so you can use the
>     serial driver
> 
> I didn't express that very clearly in the changelog.

So the actual problem is quite different. Your IR driver for the port
should have an interface to tell the serial layer to make it unavailable.
End of problem and you can then use either service without setserial magic

Alan

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Sébastien Dugué]

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


  Hi Bjorn, 

  looks like the commit was dropped, nevertheless here is some more info
to try and understand what may be going on so it may benefit the posterity ;-)

On Tue, 24 Jul 2007 09:48:45 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Tuesday 24 July 2007 08:28:05 am Sébastien Dugué wrote:
> >  your commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26 broke the serial console
> > on my box. Adding 'legacy_serial.force=1' to my boot param as a workaround
> > solves the issue, but this may be hiding bugs in Linux PnP support or
> > in my firmware.
> 
> Thanks for your report.  We need to figure out why the 8250_pnp driver
> didn't find your serial console device.  Can you confirm that you also
> have CONFIG_ACPI and CONFIG_PNPACPI in your .config?

  Yep, both =y

CONFIG_ACPI=y
CONFIG_PNPACPI=y

> 
> If you have those, and it still doesn't work, can you collect the DSDT
> dump, the output of "grep . /sys/bus/pnp/devices/*/*", and the dmesg
> from your "legacy_serial.force=1" boot? 

  Please find those attached. I also include the output from dmidecode
just in case (I'm not sure it contains anything interesting in this case
though).

  dsdt.dsl -> DSDT dump
  dmi.txt  -> output from dmidecode
  pnp-info.txt -> output of the grep

  dmesg-ok.txt -> dmesg with 'legacy_serial.force=1' (CONFIG_PNP_DEBUG=y)
  dmesg-bad.txt -> dmesg without 'legacy_serial.force=1' (CONFIG_PNP_DEBUG=y)

> Then we can tell which port
> the blind probe finds and whether it's described somewhere by ACPI.
> 
> Thanks,
>   Bjorn

  Thanks,

  Sébastien.

> 
> >   The box is a dual HT Xeon running a vanilla 2.6.22 x86_64 kernel
> > 
> >   here is my .config:
> > 
> > CONFIG_PNP=y
> > CONFIG_SERIAL_8250=y
> > CONFIG_SERIAL_8250_CONSOLE=y
> > CONFIG_SERIAL_8250_PCI=y
> > CONFIG_SERIAL_8250_PNP=y
> > CONFIG_SERIAL_8250_NR_UARTS=4
> > CONFIG_SERIAL_8250_RUNTIME_UARTS=4
> > CONFIG_SERIAL_8250_EXTENDED=y
> > CONFIG_SERIAL_8250_SHARE_IRQ=y
> > CONFIG_SERIAL_8250_DETECT_IRQ=y
> > 
> >   lspci output attached.
> >  
> >   Any ideas to help me debug this?
> > 
> >   If you need more info (like DSDT dump), just ask.
> > 
> >   Thanks,
> > 
> >   Sébastien.
> > 
> > 
> > 
> 
> 

[-- Attachment #2: logs.tar.bz2 --]
[-- Type: application/x-bzip, Size: 20798 bytes --]

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Wednesday 25 July 2007 01:45:54 am Sébastien Dugué wrote:
>   looks like the commit was dropped, nevertheless here is some more info
> to try and understand what may be going on so it may benefit the posterity ;-)

Thank you very much.

Your machine does indeed describe both UARTs in ACPI, and the PNP probe
finds them correctly.  The only wrinkle is that the PNP probe names the
ports in the order they appear in the ACPI namespace, and your firmware
has them "backwards":

                    Device (COMB)
                    {
                        Name (_HID, EisaId ("PNP0501"))
                        Name (_DDN, "COM2")
                        Name (_UID, 0x02)

                    Device (COMA)
                    {
                        Name (_HID, EisaId ("PNP0501"))
                        Name (_DDN, "COM1")
                        Name (_UID, 0x01)

There's nothing illegal about this, but with the current PNPACPI,
it causes the names to be reversed.  The blind probe tries 0x3f8
first, and names that ttyS0.  But the PNP probe, using the namespace
order, finds the "COM2" port at 0x2f8 first:

  00:0b: ttyS0 at I/O 0x2f8 (irq = 3) is a 16550A
  00:0c: ttyS1 at I/O 0x3f8 (irq = 4) is a 16550A

So I think your serial console would work without "legacy_serial.force"
if you used "console=ttyS1" instead of "console=ttyS0".  But you
shouldn't have to do that, of course.

The _DDN is a "DOS device name", and the _UID is a "logical device ID
that does not change across reboots."  Both are optional, and PNPACPI
ignores them.  But maybe we could change PNPACPI to sort by them if
they are present.  I'll think about this a bit.

Thanks again,
  Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Sébastien Dugué]

  Hi Bjorn,

On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Wednesday 25 July 2007 01:45:54 am Sébastien Dugué wrote:
> >   looks like the commit was dropped, nevertheless here is some more info
> > to try and understand what may be going on so it may benefit the posterity ;-)
> 
> Thank you very much.
> 
> Your machine does indeed describe both UARTs in ACPI, and the PNP probe
> finds them correctly.  The only wrinkle is that the PNP probe names the
> ports in the order they appear in the ACPI namespace, and your firmware
> has them "backwards":
> 
>                     Device (COMB)
>                     {
>                         Name (_HID, EisaId ("PNP0501"))
>                         Name (_DDN, "COM2")
>                         Name (_UID, 0x02)
> 
>                     Device (COMA)
>                     {
>                         Name (_HID, EisaId ("PNP0501"))
>                         Name (_DDN, "COM1")
>                         Name (_UID, 0x01)


  Yes, that's what I thought was weird in the DSDT, but thought the PnP layer
would map those devices according to UID or at least DDN. It seems that's
not the case.

> 
> There's nothing illegal about this, but with the current PNPACPI,
> it causes the names to be reversed.  The blind probe tries 0x3f8
> first, and names that ttyS0.  But the PNP probe, using the namespace
> order, finds the "COM2" port at 0x2f8 first:
> 
>   00:0b: ttyS0 at I/O 0x2f8 (irq = 3) is a 16550A
>   00:0c: ttyS1 at I/O 0x3f8 (irq = 4) is a 16550A

  Oh I see, didn't notice this though.

> 
> So I think your serial console would work without "legacy_serial.force"
> if you used "console=ttyS1" instead of "console=ttyS0".  But you
> shouldn't have to do that, of course.

  Will try ttyS1 just to confirm...

  Waiting for reboot...

  OK boots fine with the serial console fully operational again.

> 
> The _DDN is a "DOS device name", and the _UID is a "logical device ID
> that does not change across reboots."  Both are optional, and PNPACPI
> ignores them.  But maybe we could change PNPACPI to sort by them if
> they are present.  I'll think about this a bit.

  That would be nice, but I wish you good luck with all those
crappy BIOSes out there.

  Thanks,

  Sébastien.

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Sébastien Dugué]

On Wed, 25 Jul 2007 15:32:53 +0200 Sébastien Dugué <sebastien.dugue@bull.net> wrote:

> 
>   Hi Bjorn,
> 
> On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> 
> > On Wednesday 25 July 2007 01:45:54 am Sébastien Dugué wrote:
> > >   looks like the commit was dropped, nevertheless here is some more info
> > > to try and understand what may be going on so it may benefit the posterity ;-)
> > 
> > Thank you very much.
> > 
> > Your machine does indeed describe both UARTs in ACPI, and the PNP probe
> > finds them correctly.  The only wrinkle is that the PNP probe names the
> > ports in the order they appear in the ACPI namespace, and your firmware
> > has them "backwards":
> > 
> >                     Device (COMB)
> >                     {
> >                         Name (_HID, EisaId ("PNP0501"))
> >                         Name (_DDN, "COM2")
> >                         Name (_UID, 0x02)
> > 
> >                     Device (COMA)
> >                     {
> >                         Name (_HID, EisaId ("PNP0501"))
> >                         Name (_DDN, "COM1")
> >                         Name (_UID, 0x01)
> 
> 
>   Yes, that's what I thought was weird in the DSDT, but thought the PnP layer
> would map those devices according to UID or at least DDN. It seems that's
> not the case.
> 
> > 
> > There's nothing illegal about this, but with the current PNPACPI,
> > it causes the names to be reversed.  The blind probe tries 0x3f8
> > first, and names that ttyS0.  But the PNP probe, using the namespace
> > order, finds the "COM2" port at 0x2f8 first:
> > 
> >   00:0b: ttyS0 at I/O 0x2f8 (irq = 3) is a 16550A
> >   00:0c: ttyS1 at I/O 0x3f8 (irq = 4) is a 16550A
> 
>   Oh I see, didn't notice this though.
> 
> > 
> > So I think your serial console would work without "legacy_serial.force"
> > if you used "console=ttyS1" instead of "console=ttyS0".  But you
> > shouldn't have to do that, of course.
> 
>   Will try ttyS1 just to confirm...
> 
>   Waiting for reboot...
> 
>   OK boots fine with the serial console fully operational again.

  But I also need to change my inittab. At least it confirms the
root cause of the problem.

  Sébastien.

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Wednesday 25 July 2007 07:32:53 am Sébastien Dugué wrote:
> On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> 
> > The _DDN is a "DOS device name", and the _UID is a "logical device ID
> > that does not change across reboots."  Both are optional, and PNPACPI
> > ignores them.  But maybe we could change PNPACPI to sort by them if
> > they are present.  I'll think about this a bit.
> 
>   That would be nice, but I wish you good luck with all those
> crappy BIOSes out there.

Yeah, it's an ugly world we live in.  Would you be able to try the
attached patch just for testing?  It should sort devices with the
same _HID by their _UID.  It doesn't have any effect on my systems,
because my devices are already ordered by _UID by default.  But I
think it should switch your COM1/COM2 ports back to the order you
expect.

Yinghai, you mentioned the same issue on boxes with multiple root
bridges.  Any chance you could try this out there as well?

Index: w/drivers/pnp/pnpacpi/core.c
===================================================================
--- w.orig/drivers/pnp/pnpacpi/core.c	2007-07-25 13:56:02.000000000 -0600
+++ w/drivers/pnp/pnpacpi/core.c	2007-07-25 16:25:27.000000000 -0600
@@ -224,18 +224,91 @@
 	return -EINVAL;
 }
 
-static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
+struct pnpacpi_device {
+	struct acpi_device	*device;
+	unsigned long		unique_id;
+	struct list_head	list;
+};
+
+static LIST_HEAD(pnpacpi_dev_list);
+
+static inline int hardware_id_match(struct acpi_device *dev1,
+				    struct acpi_device *dev2)
+{
+	return !strncmp(acpi_device_hid(dev1), acpi_device_hid(dev2),
+			sizeof(acpi_device_hid(dev1)));
+}
+
+static inline int precedes(struct pnpacpi_device *dev1,
+			   struct pnpacpi_device *dev2)
+{
+	return hardware_id_match(dev1->device, dev2->device) &&
+		dev1->unique_id < dev2->unique_id;
+}
+
+static acpi_status __init pnpacpi_find_device_handler(acpi_handle handle,
 	u32 lvl, void *context, void **rv)
 {
 	struct acpi_device *device;
+	struct pnpacpi_device *dev, *cur, *prev = NULL;
+	struct list_head *node;
+	acpi_status status;
+	unsigned long id;
 
-	if (!acpi_bus_get_device(handle, &device))
-		pnpacpi_add_device(device);
-	else
+	if (acpi_bus_get_device(handle, &device))
 		return AE_CTRL_DEPTH;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		pnp_err("Out of memory");
+		return AE_OK;
+	}
+
+	status = acpi_evaluate_integer(handle, METHOD_NAME__UID, NULL, &id);
+	if (ACPI_FAILURE(status))
+		id = 0;
+
+	INIT_LIST_HEAD(&dev->list);
+	dev->device = device;
+	dev->unique_id = id;
+
+	/*
+	 * If several devices have the same _HID, sort them by _UID so
+	 * device names are ordered by _UID.  Note that _UID can be
+	 * either an integer or a string; we only order the integer ones.
+	 */
+	if (list_empty(&pnpacpi_dev_list)) {
+		list_add(&dev->list, &pnpacpi_dev_list);
+		return AE_OK;
+	}
+
+	list_for_each(node, &pnpacpi_dev_list) {
+		cur = list_entry(node, struct pnpacpi_device, list);
+		if (precedes(dev, cur) ||
+		    (prev && hardware_id_match(prev->device, cur->device))) {
+			list_add_tail(&dev->list, node);
+			return AE_OK;
+		}
+		prev = cur;
+	}
+
+	list_add_tail(&dev->list, &pnpacpi_dev_list);
 	return AE_OK;
 }
 
+static void __init pnpacpi_add_devices(void)
+{
+	struct list_head *node, *next;
+	struct pnpacpi_device *dev;
+
+	list_for_each_safe(node, next, &pnpacpi_dev_list) {
+		dev = list_entry(node, struct pnpacpi_device, list);
+		pnpacpi_add_device(dev->device);
+		list_del(node);
+		kfree(dev);
+	}
+}
+
 static int __init acpi_pnp_match(struct device *dev, void *_pnp)
 {
 	struct acpi_device	*acpi = to_acpi_device(dev);
@@ -282,7 +355,8 @@
 	pnp_info("PnP ACPI init");
 	pnp_register_protocol(&pnpacpi_protocol);
 	register_acpi_bus_type(&acpi_pnp_bus);
-	acpi_get_devices(NULL, pnpacpi_add_device_handler, NULL, NULL);
+	acpi_get_devices(NULL, pnpacpi_find_device_handler, NULL, NULL);
+	pnpacpi_add_devices();
 	pnp_info("PnP ACPI: found %d devices", num);
 	unregister_acpi_bus_type(&acpi_pnp_bus);
 	pnp_platform_devices = 1;

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Wednesday 25 July 2007 07:32:53 am Sébastien Dugué wrote:
> > On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> >
> > > The _DDN is a "DOS device name", and the _UID is a "logical device ID
> > > that does not change across reboots."  Both are optional, and PNPACPI
> > > ignores them.  But maybe we could change PNPACPI to sort by them if
> > > they are present.  I'll think about this a bit.
> >
> >   That would be nice, but I wish you good luck with all those
> > crappy BIOSes out there.
>
> Yeah, it's an ugly world we live in.  Would you be able to try the
> attached patch just for testing?  It should sort devices with the
> same _HID by their _UID.  It doesn't have any effect on my systems,
> because my devices are already ordered by _UID by default.  But I
> think it should switch your COM1/COM2 ports back to the order you
> expect.
>
> Yinghai, you mentioned the same issue on boxes with multiple root
> bridges.  Any chance you could try this out there as well?
>
it doesn't solve pci_root_bus reverse problem.

is that too late for PNP0A03?

I wonder if we need to modify acpi_device_register to sort them.

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/25/07, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Wednesday 25 July 2007 07:32:53 am Sébastien Dugué wrote:
> > > On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > >
> > > > The _DDN is a "DOS device name", and the _UID is a "logical device ID
> > > > that does not change across reboots."  Both are optional, and PNPACPI
> > > > ignores them.  But maybe we could change PNPACPI to sort by them if
> > > > they are present.  I'll think about this a bit.
> > >
> > >   That would be nice, but I wish you good luck with all those
> > > crappy BIOSes out there.
> >
> > Yeah, it's an ugly world we live in.  Would you be able to try the
> > attached patch just for testing?  It should sort devices with the
> > same _HID by their _UID.  It doesn't have any effect on my systems,
> > because my devices are already ordered by _UID by default.  But I
> > think it should switch your COM1/COM2 ports back to the order you
> > expect.
> >
> > Yinghai, you mentioned the same issue on boxes with multiple root
> > bridges.  Any chance you could try this out there as well?
> >
> it doesn't solve pci_root_bus reverse problem.
>
> is that too late for PNP0A03?
>
> I wonder if we need to modify acpi_device_register to sort them.

local hack for pci_root reverse problem would be

 or in acpi_pci_register_driver, sort the acpi_pci_roots before using it.

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Shaohua Li]

On Wed, 2007-07-25 at 17:37 -0700, Yinghai Lu wrote:
> On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Wednesday 25 July 2007 07:32:53 am Sébastien Dugué wrote:
> > > On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > >
> > > > The _DDN is a "DOS device name", and the _UID is a "logical device ID
> > > > that does not change across reboots."  Both are optional, and PNPACPI
> > > > ignores them.  But maybe we could change PNPACPI to sort by them if
> > > > they are present.  I'll think about this a bit.
> > >
> > >   That would be nice, but I wish you good luck with all those
> > > crappy BIOSes out there.
> >
> > Yeah, it's an ugly world we live in.  Would you be able to try the
> > attached patch just for testing?  It should sort devices with the
> > same _HID by their _UID.  It doesn't have any effect on my systems,
> > because my devices are already ordered by _UID by default.  But I
> > think it should switch your COM1/COM2 ports back to the order you
> > expect.
> >
> > Yinghai, you mentioned the same issue on boxes with multiple root
> > bridges.  Any chance you could try this out there as well?
> >
> it doesn't solve pci_root_bus reverse problem.
> 
> is that too late for PNP0A03?
> 
> I wonder if we need to modify acpi_device_register to sort them.
The pci root driver is an acpi driver not a pnp driver, so Bjorn's patch
will not work. Maybe the ACPI core (ACPICA) should do the sort?

Thanks,
Shaohua

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Wednesday 25 July 2007 08:21:06 pm Shaohua Li wrote:
> On Wed, 2007-07-25 at 17:37 -0700, Yinghai Lu wrote:
> > On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > Yinghai, you mentioned the same issue on boxes with multiple root
> > > bridges.  Any chance you could try this out there as well?
> > >
> > it doesn't solve pci_root_bus reverse problem.
> > 
> > is that too late for PNP0A03?
> > 
> > I wonder if we need to modify acpi_device_register to sort them.
> The pci root driver is an acpi driver not a pnp driver, so Bjorn's patch
> will not work.

Right, I forgot that the PCI root driver is an ACPI driver.

This is a longer-term idea, but the way I'd like to solve this is
by converting ACPI drivers into PNP drivers.  Then the PNPACPI sort
would work for all of them.  PNP would provide a hook to retrieve
the ACPI handle corresponding to a PNP device.

> Maybe the ACPI core (ACPICA) should do the sort? 

I thought about that, but I didn't see a nice way to do it.  The
current namespace interfaces like acpi_get_devices() are walk-
oriented -- they call a callback function for every node that
meets some criteria.  Sorting requires some sort of buffer so
you can look at all the matching nodes before returning any of
them.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Wednesday 25 July 2007 08:21:06 pm Shaohua Li wrote:
> On Wed, 2007-07-25 at 17:37 -0700, Yinghai Lu wrote:
> > On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > On Wednesday 25 July 2007 07:32:53 am Sébastien Dugué wrote:
> > > > On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > >
> > > > > The _DDN is a "DOS device name", and the _UID is a "logical device ID
> > > > > that does not change across reboots."  Both are optional, and PNPACPI
> > > > > ignores them.  But maybe we could change PNPACPI to sort by them if
> > > > > they are present.  I'll think about this a bit.
> > > >
> > > >   That would be nice, but I wish you good luck with all those
> > > > crappy BIOSes out there.
> > >
> > > Yeah, it's an ugly world we live in.  Would you be able to try the
> > > attached patch just for testing?  It should sort devices with the
> > > same _HID by their _UID.  It doesn't have any effect on my systems,
> > > because my devices are already ordered by _UID by default.  But I
> > > think it should switch your COM1/COM2 ports back to the order you
> > > expect.
> > >
> > > Yinghai, you mentioned the same issue on boxes with multiple root
> > > bridges.  Any chance you could try this out there as well?
> > >
> > it doesn't solve pci_root_bus reverse problem.
> > 
> > is that too late for PNP0A03?
> > 
> > I wonder if we need to modify acpi_device_register to sort them.
> The pci root driver is an acpi driver not a pnp driver, so Bjorn's patch
> will not work. Maybe the ACPI core (ACPICA) should do the sort?

I talked to Adam about this yesterday.  He convinced me that we
shouldn't rely on device ordering to determine names.  For one
thing, that would make threaded probing difficult.  Better to
rely on a unique identifier, and let udev take care of user-space
persistent naming issues.

I don't think udev solves the problem for built-in drivers like
serial, where you need to be able to do "console=ttyS0" and have
it mean something predictable.  But possibly we could expose the
_DDN and/or _UID through PNP and have 8250_pnp give a hint about
what the ttyS device should be when it registers with 8250.

For example, 8250_pnp could have rules like "COM1 should always
be ttyS0" or "a port at 0x3f8 should always be ttyS0."

That doesn't help with Yinghai's PCI root ordering issue, of course.
But I hope that can be addressed with udev, because there's not so
much need for a persistent kernel name.  If that's not enough, can
you explain more about the problem?

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/27/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Wednesday 25 July 2007 08:21:06 pm Shaohua Li wrote:
> That doesn't help with Yinghai's PCI root ordering issue, of course.

I could
sort the acpi_pci_roots before using it, in acpi_pci_register_driver()

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Friday 27 July 2007 12:38:56 pm Yinghai Lu wrote:
> On 7/27/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Wednesday 25 July 2007 08:21:06 pm Shaohua Li wrote:
> > That doesn't help with Yinghai's PCI root ordering issue, of course.
> 
> I could
> sort the acpi_pci_roots before using it, in acpi_pci_register_driver()

acpi_pci_register_driver() is only used by hotplug.  acpi_pci_root_add()
does the pci_acpi_scan_root(), so sorting in acpi_pci_register_driver()
won't affect that.

Can you give a more detailed example?  Maybe that would help me understand
the problem you're seeing.

And why is udev not sufficient to give the NICs persistent names?

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/27/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Friday 27 July 2007 12:38:56 pm Yinghai Lu wrote:
> > On 7/27/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > On Wednesday 25 July 2007 08:21:06 pm Shaohua Li wrote:
> > > That doesn't help with Yinghai's PCI root ordering issue, of course.
> >
> > I could
> > sort the acpi_pci_roots before using it, in acpi_pci_register_driver()
>
> acpi_pci_register_driver() is only used by hotplug.  acpi_pci_root_add()
> does the pci_acpi_scan_root(), so sorting in acpi_pci_register_driver()
> won't affect that.

Not sure. will test that later.

>
> Can you give a more detailed example?  Maybe that would help me understand
> the problem you're seeing.
>
> And why is udev not sufficient to give the NICs persistent names?

yes. NIC on node1 become the eth0 instead of eth2.

YH

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Bjorn Helgaas]

On Friday 27 July 2007 02:35:30 pm Yinghai Lu wrote:
> On 7/27/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > Can you give a more detailed example?  Maybe that would help me understand
> > the problem you're seeing.
> >
> > And why is udev not sufficient to give the NICs persistent names?
> 
> yes. NIC on node1 become the eth0 instead of eth2.

We aren't doing threaded probes or anything yet, so on a given
machine, device names should be deterministic.

Of course, if you reconfigure the machine, the driver may find
devices in a different order and name them something different.
I don't see a way to avoid that.

And couldn't we use udev to associate a fixed name with a MAC
address?  Then the user could use the same persistent name,
regardless of the order in which the driver found the devices.

Bjorn

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Jeff Garzik]

Bjorn Helgaas wrote:
> And couldn't we use udev to associate a fixed name with a MAC
> address?  Then the user could use the same persistent name,
> regardless of the order in which the driver found the devices.


I don't know about udev, but people are definitely using fixed names 
based on MAC address for ethernet devices already:  nameif(8) and 
/etc/mactab, iftab(5) and ifrename(8).

	Jeff

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Kay Sievers]

On 7/27/07, Jeff Garzik <jeff@garzik.org> wrote:
> Bjorn Helgaas wrote:
> > And couldn't we use udev to associate a fixed name with a MAC
> > address?  Then the user could use the same persistent name,
> > regardless of the order in which the driver found the devices.
>
>
> I don't know about udev, but people are definitely using fixed names
> based on MAC address for ethernet devices already:  nameif(8) and
> /etc/mactab, iftab(5) and ifrename(8).

That's all obsolete with udev, it creates persistent name entries on
first device discovery, and keeps them forever.
You can edit the automatically created/updated udev rules, if you want
different names, but by default there is no intervention needed to
have stable network device names out of the box, regardless of any
kernel module loading/linking order or whatever.

Kay

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Kyle Moffett]

On Jul 27, 2007, at 17:05:30, Jeff Garzik wrote:
> Bjorn Helgaas wrote:
>> And couldn't we use udev to associate a fixed name with a MAC  
>> address?  Then the user could use the same persistent name,  
>> regardless of the order in which the driver found the devices.
>
> I don't know about udev, but people are definitely using fixed  
> names based on MAC address for ethernet devices already:  nameif(8)  
> and /etc/mactab, iftab(5) and ifrename(8).

I was doing that for a while, but now Debian and RedHat and most  
other modern distros have a udev rules file called something like: / 
etc/udev/rules.d/z30-persistent-net-generator.rules  Under Debian it  
generates a file /etc/udev/rules.d/z25-persistent-net.rules which  
automatically renames net devices based on their mac address.  The  
persistent-net-generator takes the current name and MAC and stuffs  
them in an appropriate file.  So really once you've booted with the  
network card in, all you have to do is modify the persistent- 
net.rules file to change the name of your NIC and rerun udevtrigger.   
I've gotten into the habit of using descriptive names lately (7 chars  
is the most that ifconfig will show without truncating, and it seems  
to be unable to properly sort/display/operate-on longer ones).

For example, my Debian firewall box has:
   world: Connection to the cable modem
   switch: Connection to my VLAN-capable switch
   main0: "Primary" VLAN on my switch.
   main1: A local gigabit switch for connections to a couple other  
systems
   main: A bridge of main0 and main1 which is used as the primary LAN  
interface
   hbeat: Dedicated NIC for Linux-HA (HeartBeat)
   debian: A Debian netinst net-boot VLAN with support for PXE and  
OpenFirmware
   radius: A VLAN used by my wireless AP for radius traffic
   wifi: A VLAN used for private wireless traffic

Most of those interfaces are virtual and created by custom "/etc/ 
network/if-pre-up.d/*" scripts, but you can see the "world",  
"switch", "main1", and "hbeat" interfaces are actual physical  
interfaces.  Such naming (with appropriate comments in /etc/network/ 
interfaces) vastly eases the management of a box with a shitton of  
interfaces.  The fact that ifconfig has some outstanding bugs with  
big interface names is mostly irrelevant since I use the iproute2  
tool ("ip") to do almost all administration.

Cheers,
Kyle Moffett

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Russell King]

On Fri, Jul 27, 2007 at 12:16:59PM -0600, Bjorn Helgaas wrote:
> For example, 8250_pnp could have rules like "COM1 should always
> be ttyS0" or "a port at 0x3f8 should always be ttyS0."

In which case register via the legacy ports first, and then register
PNP ports.  If the PNP ports correspond with legacy ports, they will
re-use those slots.

So, if we register 0x3f8 first (which ends up as ttyS0), followed by
0x2f8 (ttyS1) and then PNP tries to register a port at 0x2f8 then
0x3f8, you'll still end up with 0x3f8 being ttyS0 and 0x2f8 as ttyS1.

> That doesn't help with Yinghai's PCI root ordering issue, of course.
> But I hope that can be addressed with udev, because there's not so
> much need for a persistent kernel name.  If that's not enough, can
> you explain more about the problem?

However, aren't PNP serial ports attached to separate PNP devices?  If
so, udev can work it out already - they just need to look at the
backing device associated with the serial port.  For legacy ports that'll
be serial8250.0.  For PNP, it'll be some PNP device.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Sébastien Dugué]

On Wed, 25 Jul 2007 16:41:23 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Wednesday 25 July 2007 07:32:53 am Sébastien Dugué wrote:
> > On Wed, 25 Jul 2007 07:16:44 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > 
> > > The _DDN is a "DOS device name", and the _UID is a "logical device ID
> > > that does not change across reboots."  Both are optional, and PNPACPI
> > > ignores them.  But maybe we could change PNPACPI to sort by them if
> > > they are present.  I'll think about this a bit.
> > 
> >   That would be nice, but I wish you good luck with all those
> > crappy BIOSes out there.
> 
> Yeah, it's an ugly world we live in.  Would you be able to try the
> attached patch just for testing?  It should sort devices with the
> same _HID by their _UID.  It doesn't have any effect on my systems,
> because my devices are already ordered by _UID by default.  But I
> think it should switch your COM1/COM2 ports back to the order you
> expect.
> 

  Works well, thanks Bjorn.

  Sébastien.

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[Yinghai Lu]

On 7/25/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> The _DDN is a "DOS device name", and the _UID is a "logical device ID
> that does not change across reboots."  Both are optional, and PNPACPI
> ignores them.  But maybe we could change PNPACPI to sort by them if
> they are present.  I'll think about this a bit.
>
this happen with muliti peer root buses too.
some DSDT put bus 0x80 before  0x0. So the bus 0x80 is scanned by the
kernel at first. so some device such as nic number is reversed too...

need to make some change to acpica...

YH

Linus 2.6.23-rc1

[Linus Torvalds]

Ok, right on time, two weeks afetr 2.6.22, there's a 2.6.23-rc1 out there.

And it has a *ton* of changes as usual for the merge window, way too much 
for me to be able to post even just the shortlog or diffstat on the 
mailing list (but I had many people who wanted to full logs to stay 
around, so you'll continue to see those being uploaded to kernel.org).

Lots of architecture updates (for just about all of them - x86[-64], arm, 
alpha, mips, ia64, powerpc, s390, sh, sparc, um..), lots of driver updates 
(again, all over - usb, net, dvb, ide, sata, scsi, isdn, infiniband, 
firewire, i2c, you name it).

Filesystems, VM, networking, ACPI, it's all there. And virtualization all 
over the place (kvm, lguest, Xen).

Notable new things might be the merge of the cfs scheduler, and the UIO 
driver infrastructure might interest some people.

Oh, and I personally like how "sendfile" is now totally gone internally, 
and the kernel now ends up doing all that with splice insted. Good 
riddance, although we'll obvously end up supporting the old user level 
interfaces for a long time.

So give it all a good whacking, and report back about all the neat new 
features!

		Linus

2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Alexey Dobriyan]

Managed to hit BUG_ON() in kmap_atomic_prot() three times while doing
nothing unusual for this box (two times it was under X, so I can't
guarantee, one time while trying to reproduce via ./configure in gdb
tarball)

Box has 2.5G of RAM. 2.6.22 was OK.

[dives into framebuffer console setup for complete oops]


CONFIG_X86_32=y
CONFIG_GENERIC_TIME=y
CONFIG_GENERIC_CMOS_UPDATE=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_SEMAPHORE_SLEEPERS=y
CONFIG_X86=y
CONFIG_MMU=y
CONFIG_ZONE_DMA=y
CONFIG_QUICKLIST=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_IOMAP=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_DMI=y
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_EXPERIMENTAL=y
CONFIG_BROKEN_ON_SMP=y
CONFIG_LOCK_KERNEL=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_LOCALVERSION=""
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=15
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_SYSCTL=y
CONFIG_EMBEDDED=y
CONFIG_KALLSYMS=y
CONFIG_KALLSYMS_ALL=y
CONFIG_HOTPLUG=y
CONFIG_PRINTK=y
CONFIG_BUG=y
CONFIG_ELF_CORE=y
CONFIG_BASE_FULL=y
CONFIG_FUTEX=y
CONFIG_SHMEM=y
CONFIG_SLAB=y
CONFIG_RT_MUTEXES=y
CONFIG_BASE_SMALL=0
CONFIG_BLOCK=y
CONFIG_IOSCHED_NOOP=y
CONFIG_IOSCHED_CFQ=y
CONFIG_DEFAULT_CFQ=y
CONFIG_DEFAULT_IOSCHED="cfq"
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ=y
CONFIG_X86_PC=y
CONFIG_MPENTIUM4=y
CONFIG_X86_CMPXCHG=y
CONFIG_X86_L1_CACHE_SHIFT=7
CONFIG_X86_XADD=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_X86_WP_WORKS_OK=y
CONFIG_X86_INVLPG=y
CONFIG_X86_BSWAP=y
CONFIG_X86_POPAD_OK=y
CONFIG_X86_GOOD_APIC=y
CONFIG_X86_INTEL_USERCOPY=y
CONFIG_X86_USE_PPRO_CHECKSUM=y
CONFIG_X86_TSC=y
CONFIG_X86_CMOV=y
CONFIG_X86_MINIMUM_CPU_FAMILY=4
CONFIG_PREEMPT=y
CONFIG_PREEMPT_BKL=y
CONFIG_X86_MCE=y
CONFIG_VM86=y
CONFIG_HIGHMEM4G=y
CONFIG_VMSPLIT_3G=y
CONFIG_PAGE_OFFSET=0xC0000000
CONFIG_HIGHMEM=y
CONFIG_ARCH_FLATMEM_ENABLE=y
CONFIG_ARCH_SPARSEMEM_ENABLE=y
CONFIG_ARCH_SELECT_MEMORY_MODEL=y
CONFIG_ARCH_POPULATES_NODE_MAP=y
CONFIG_SELECT_MEMORY_MODEL=y
CONFIG_FLATMEM_MANUAL=y
CONFIG_FLATMEM=y
CONFIG_FLAT_NODE_MEM_MAP=y
CONFIG_SPARSEMEM_STATIC=y
CONFIG_SPLIT_PTLOCK_CPUS=4
CONFIG_ZONE_DMA_FLAG=1
CONFIG_BOUNCE=y
CONFIG_NR_QUICK=1
CONFIG_VIRT_TO_BUS=y
CONFIG_MTRR=y
CONFIG_HZ_250=y
CONFIG_HZ=250
CONFIG_PHYSICAL_START=0x100000
CONFIG_PHYSICAL_ALIGN=0x100000
CONFIG_ARCH_ENABLE_MEMORY_HOTPLUG=y
CONFIG_PCI=y
CONFIG_PCI_GOANY=y
CONFIG_PCI_BIOS=y
CONFIG_PCI_DIRECT=y
CONFIG_ISA_DMA_API=y
CONFIG_BINFMT_ELF=y
CONFIG_NET=y
CONFIG_UNIX=y
CONFIG_INET=y
CONFIG_IP_FIB_HASH=y
CONFIG_TCP_CONG_CUBIC=y
CONFIG_DEFAULT_TCP_CONG="cubic"
CONFIG_STANDALONE=y
CONFIG_PREVENT_FIRMWARE_BUILD=y
CONFIG_BLK_DEV=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_MISC_DEVICES=y
CONFIG_IDE=y
CONFIG_IDE_MAX_HWIFS=4
CONFIG_BLK_DEV_IDE=y
CONFIG_BLK_DEV_IDEDISK=y
CONFIG_IDEDISK_MULTI_MODE=y
CONFIG_BLK_DEV_IDECD=y
CONFIG_BLK_DEV_IDEPCI=y
CONFIG_IDEPCI_SHARE_IRQ=y
CONFIG_IDEPCI_PCIBUS_ORDER=y
CONFIG_BLK_DEV_IDEDMA_PCI=y
CONFIG_BLK_DEV_PIIX=y
CONFIG_BLK_DEV_IDEDMA=y
CONFIG_SCSI=y
CONFIG_SCSI_DMA=y
CONFIG_BLK_DEV_SD=y
CONFIG_SCSI_LOWLEVEL=y
CONFIG_ATA=y
CONFIG_ATA_PIIX=y
CONFIG_NETDEVICES=y
CONFIG_NET_ETHERNET=y
CONFIG_MII=y
CONFIG_NET_PCI=y
CONFIG_8139TOO=y
CONFIG_8139TOO_PIO=y
CONFIG_INPUT=y
CONFIG_INPUT_MOUSEDEV=y
CONFIG_INPUT_MOUSEDEV_SCREEN_X=1024
CONFIG_INPUT_MOUSEDEV_SCREEN_Y=768
CONFIG_INPUT_KEYBOARD=y
CONFIG_KEYBOARD_ATKBD=y
CONFIG_INPUT_MOUSE=y
CONFIG_MOUSE_PS2=y
CONFIG_SERIO=y
CONFIG_SERIO_I8042=y
CONFIG_SERIO_LIBPS2=y
CONFIG_VT=y
CONFIG_VT_CONSOLE=y
CONFIG_HW_CONSOLE=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_UNIX98_PTYS=y
CONFIG_RTC=y
CONFIG_AGP=y
CONFIG_AGP_INTEL=y
CONFIG_DRM=y
CONFIG_DRM_RADEON=y
CONFIG_DEVPORT=y
CONFIG_I2C=y
CONFIG_I2C_BOARDINFO=y
CONFIG_I2C_ALGOBIT=y
CONFIG_FB=y
CONFIG_FB_DDC=y
CONFIG_FB_CFB_FILLRECT=y
CONFIG_FB_CFB_COPYAREA=y
CONFIG_FB_CFB_IMAGEBLIT=y
CONFIG_FB_DEFERRED_IO=y
CONFIG_FB_MODE_HELPERS=y
CONFIG_FB_RADEON=y
CONFIG_FB_RADEON_I2C=y
CONFIG_VGA_CONSOLE=y
CONFIG_DUMMY_CONSOLE=y
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_TIMER=y
CONFIG_SND_PCM=y
CONFIG_SND_OSSEMUL=y
CONFIG_SND_MIXER_OSS=y
CONFIG_SND_PCM_OSS=y
CONFIG_SND_AC97_CODEC=y
CONFIG_SND_INTEL8X0=y
CONFIG_AC97_BUS=y
CONFIG_HID_SUPPORT=y
CONFIG_USB_SUPPORT=y
CONFIG_USB_ARCH_HAS_HCD=y
CONFIG_USB_ARCH_HAS_OHCI=y
CONFIG_USB_ARCH_HAS_EHCI=y
CONFIG_USB=y
CONFIG_USB_DEVICEFS=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_UHCI_HCD=y
CONFIG_USB_PRINTER=y
CONFIG_USB_STORAGE=y
CONFIG_EXT2_FS=y
CONFIG_EXT3_FS=y
CONFIG_JBD=y
CONFIG_REISERFS_FS=y
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
CONFIG_FAT_FS=y
CONFIG_VFAT_FS=y
CONFIG_FAT_DEFAULT_CODEPAGE=866
CONFIG_FAT_DEFAULT_IOCHARSET="koi8-r"
CONFIG_PROC_FS=y
CONFIG_PROC_SYSCTL=y
CONFIG_SYSFS=y
CONFIG_TMPFS=y
CONFIG_RAMFS=y
CONFIG_UFS_FS=y
CONFIG_UFS_FS_WRITE=y
CONFIG_CIFS=y
CONFIG_MSDOS_PARTITION=y
CONFIG_NLS=y
CONFIG_NLS_DEFAULT="utf8"
CONFIG_NLS_CODEPAGE_866=y
CONFIG_NLS_KOI8_R=y
CONFIG_NLS_UTF8=y
CONFIG_TRACE_IRQFLAGS_SUPPORT=y
CONFIG_ENABLE_MUST_CHECK=y
CONFIG_MAGIC_SYSRQ=y
CONFIG_DEBUG_KERNEL=y
CONFIG_DEBUG_SHIRQ=y
CONFIG_DETECT_SOFTLOCKUP=y
CONFIG_DEBUG_SLAB=y
CONFIG_DEBUG_SLAB_LEAK=y
CONFIG_DEBUG_PREEMPT=y
CONFIG_DEBUG_SPINLOCK=y
CONFIG_DEBUG_MUTEXES=y
CONFIG_DEBUG_LOCK_ALLOC=y
CONFIG_PROVE_LOCKING=y
CONFIG_LOCKDEP=y
CONFIG_TRACE_IRQFLAGS=y
CONFIG_DEBUG_SPINLOCK_SLEEP=y
CONFIG_STACKTRACE=y
CONFIG_DEBUG_BUGVERBOSE=y
CONFIG_DEBUG_VM=y
CONFIG_DEBUG_LIST=y
CONFIG_DEBUG_STACKOVERFLOW=y
CONFIG_DEBUG_STACK_USAGE=y
CONFIG_DEBUG_PAGEALLOC=y
CONFIG_DEBUG_RODATA=y
CONFIG_4KSTACKS=y
CONFIG_DOUBLEFAULT=y
CONFIG_BITREVERSE=y
CONFIG_CRC32=y
CONFIG_PLIST=y
CONFIG_HAS_IOMEM=y
CONFIG_HAS_IOPORT=y
CONFIG_HAS_DMA=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_X86_BIOS_REBOOT=y
CONFIG_KTIME_SCALAR=y

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Alexey Dobriyan]

On Mon, Jul 23, 2007 at 10:38:39PM +0400, Alexey Dobriyan wrote:
> Managed to hit BUG_ON() in kmap_atomic_prot() three times while doing
> nothing unusual for this box (two times it was under X, so I can't
> guarantee, one time while trying to reproduce via ./configure in gdb
> tarball)
> 
> Box has 2.5G of RAM. 2.6.22 was OK.
> 
> [dives into framebuffer console setup for complete oops]

kernel BUG at arch/i386/mm/highmem.c:38
PREEMPT DEBUG_PAGEALLOC SLAB
EIP at kmap_atomic_prot+0x32/0x93
	get_page_from_freelist
	__alloc_pages
	cache_alloc_refill
	cache_alloc_refill
	kmem_cache_alloc
	dst_alloc
	dst_alloc
	__ip_route_output_key
	[some junk I don't trust]

eax: 0000000c
ebx: 00000003
ecx: c065efe0
edx: 00000003
edi: 00000163


c010cc9b <kmap_atomic_prot>:
c010cc9b:	57                   	push   %edi
c010cc9c:	56                   	push   %esi
c010cc9d:	53                   	push   %ebx
c010cc9e:	89 c6                	mov    %eax,%esi
c010cca0:	89 d3                	mov    %edx,%ebx
c010cca2:	89 cf                	mov    %ecx,%edi
c010cca4:	b8 01 00 00 00       	mov    $0x1,%eax
c010cca9:	e8 dd 1b 00 00       	call   c010e88b <add_preempt_count>
c010ccae:	e8 b1 ac 0e 00       	call   c01f7964 <debug_smp_processor_id>
c010ccb3:	6b c0 0d             	imul   $0xd,%eax,%eax
c010ccb6:	8d 14 03             	lea    (%ebx,%eax,1),%edx
c010ccb9:	8d 04 95 00 00 00 00 	lea    0x0(,%edx,4),%eax
c010ccc0:	8b 0d 30 a1 3e c0    	mov    0xc03ea130,%ecx
c010ccc6:	29 c1                	sub    %eax,%ecx
c010ccc8:	83 39 00             	cmpl   $0x0,(%ecx)
c010cccb:	74 04                	je     c010ccd1 <kmap_atomic_prot+0x36>
c010cccd:	0f 0b                	ud2a   

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Andrew Morton]

On Mon, 23 Jul 2007 23:01:52 +0400
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Mon, Jul 23, 2007 at 10:38:39PM +0400, Alexey Dobriyan wrote:
> > Managed to hit BUG_ON() in kmap_atomic_prot() three times while doing
> > nothing unusual for this box (two times it was under X, so I can't
> > guarantee, one time while trying to reproduce via ./configure in gdb
> > tarball)

Yeah, I hit this several times a few days ago.  Same story: it just
randomly went splat in response to no obvious stimulus.  Reported it to
netdev, was greeted with stunned silence.


> > Box has 2.5G of RAM. 2.6.22 was OK.
> > 
> > [dives into framebuffer console setup for complete oops]
> 
> kernel BUG at arch/i386/mm/highmem.c:38
> PREEMPT DEBUG_PAGEALLOC SLAB
> EIP at kmap_atomic_prot+0x32/0x93
> 	get_page_from_freelist
> 	__alloc_pages
> 	cache_alloc_refill
> 	cache_alloc_refill
> 	kmem_cache_alloc
> 	dst_alloc
> 	dst_alloc
> 	__ip_route_output_key
> 	[some junk I don't trust]
> 
> eax: 0000000c
> ebx: 00000003
> ecx: c065efe0
> edx: 00000003
> edi: 00000163
> 
> 
> c010cc9b <kmap_atomic_prot>:
> c010cc9b:	57                   	push   %edi
> c010cc9c:	56                   	push   %esi
> c010cc9d:	53                   	push   %ebx
> c010cc9e:	89 c6                	mov    %eax,%esi
> c010cca0:	89 d3                	mov    %edx,%ebx
> c010cca2:	89 cf                	mov    %ecx,%edi
> c010cca4:	b8 01 00 00 00       	mov    $0x1,%eax
> c010cca9:	e8 dd 1b 00 00       	call   c010e88b <add_preempt_count>
> c010ccae:	e8 b1 ac 0e 00       	call   c01f7964 <debug_smp_processor_id>
> c010ccb3:	6b c0 0d             	imul   $0xd,%eax,%eax
> c010ccb6:	8d 14 03             	lea    (%ebx,%eax,1),%edx
> c010ccb9:	8d 04 95 00 00 00 00 	lea    0x0(,%edx,4),%eax
> c010ccc0:	8b 0d 30 a1 3e c0    	mov    0xc03ea130,%ecx
> c010ccc6:	29 c1                	sub    %eax,%ecx
> c010ccc8:	83 39 00             	cmpl   $0x0,(%ecx)
> c010cccb:	74 04                	je     c010ccd1 <kmap_atomic_prot+0x36>
> c010cccd:	0f 0b                	ud2a   

I had more complete info: http://article.gmane.org/gmane.linux.network/66966

You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.

I haven't worked out where that kmap_atomic() call is coming from yet. 
Both traces point up into the page allocator, but I _think_ that's stack
gunk.

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Alexey Dobriyan]

On Mon, Jul 23, 2007 at 01:24:31PM -0700, Andrew Morton wrote:
> On Mon, 23 Jul 2007 23:01:52 +0400
> Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > On Mon, Jul 23, 2007 at 10:38:39PM +0400, Alexey Dobriyan wrote:
> > > Managed to hit BUG_ON() in kmap_atomic_prot() three times while doing
> > > nothing unusual for this box (two times it was under X, so I can't
> > > guarantee, one time while trying to reproduce via ./configure in gdb
> > > tarball)
> 
> Yeah, I hit this several times a few days ago.  Same story: it just
> randomly went splat in response to no obvious stimulus.  Reported it to
> netdev, was greeted with stunned silence.
> 
> 
> > > Box has 2.5G of RAM. 2.6.22 was OK.
> > > 
> > > [dives into framebuffer console setup for complete oops]
> > 
> > kernel BUG at arch/i386/mm/highmem.c:38
> > PREEMPT DEBUG_PAGEALLOC SLAB
> > EIP at kmap_atomic_prot+0x32/0x93
> > 	get_page_from_freelist
> > 	__alloc_pages
> > 	cache_alloc_refill
> > 	cache_alloc_refill
> > 	kmem_cache_alloc
> > 	dst_alloc
> > 	dst_alloc
> > 	__ip_route_output_key
> > 	[some junk I don't trust]
> > 
> > eax: 0000000c
> > ebx: 00000003
> > ecx: c065efe0
> > edx: 00000003
> > edi: 00000163
> > 
> > 
> > c010cc9b <kmap_atomic_prot>:
> > c010cc9b:	57                   	push   %edi
> > c010cc9c:	56                   	push   %esi
> > c010cc9d:	53                   	push   %ebx
> > c010cc9e:	89 c6                	mov    %eax,%esi
> > c010cca0:	89 d3                	mov    %edx,%ebx
> > c010cca2:	89 cf                	mov    %ecx,%edi
> > c010cca4:	b8 01 00 00 00       	mov    $0x1,%eax
> > c010cca9:	e8 dd 1b 00 00       	call   c010e88b <add_preempt_count>
> > c010ccae:	e8 b1 ac 0e 00       	call   c01f7964 <debug_smp_processor_id>
> > c010ccb3:	6b c0 0d             	imul   $0xd,%eax,%eax
> > c010ccb6:	8d 14 03             	lea    (%ebx,%eax,1),%edx
> > c010ccb9:	8d 04 95 00 00 00 00 	lea    0x0(,%edx,4),%eax
> > c010ccc0:	8b 0d 30 a1 3e c0    	mov    0xc03ea130,%ecx
> > c010ccc6:	29 c1                	sub    %eax,%ecx
> > c010ccc8:	83 39 00             	cmpl   $0x0,(%ecx)
> > c010cccb:	74 04                	je     c010ccd1 <kmap_atomic_prot+0x36>
> > c010cccd:	0f 0b                	ud2a   
> 
> I had more complete info: http://article.gmane.org/gmane.linux.network/66966
> 
> You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.
> 
> I haven't worked out where that kmap_atomic() call is coming from yet. 
> Both traces point up into the page allocator, but I _think_ that's stack
> gunk.

Ahh, you suspect networking.

Here, setup is 2 cheap-ass 100Mb realtek 8139 NICs, one to campus network
receiving ~20 junk packets per second, one gathering netconsole output
and ssh to it, no conntracks and fancy stuff.

[reboots with cables physically unplugged]

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Alexey Dobriyan]

On Tue, Jul 24, 2007 at 12:40:45AM +0400, Alexey Dobriyan wrote:
> > I had more complete info: http://article.gmane.org/gmane.linux.network/66966
> > 
> > You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.
> > 
> > I haven't worked out where that kmap_atomic() call is coming from yet. 
> > Both traces point up into the page allocator, but I _think_ that's stack
> > gunk.
> 
> Ahh, you suspect networking.
> 
> Here, setup is 2 cheap-ass 100Mb realtek 8139 NICs, one to campus network
> receiving ~20 junk packets per second, one gathering netconsole output
> and ssh to it, no conntracks and fancy stuff.
> 
> [reboots with cables physically unplugged]

OK, I run gdb recompile, cat(1) every file in /usr/portage (shitload of
small files) with both cables unplugged. It all went fine for ~5 minutes
after that it crashed exactly same way after 10 secs after plugging one
of them.

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Andrew Morton]

On Tue, 24 Jul 2007 01:01:53 +0400
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Tue, Jul 24, 2007 at 12:40:45AM +0400, Alexey Dobriyan wrote:
> > > I had more complete info: http://article.gmane.org/gmane.linux.network/66966
> > > 
> > > You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.
> > > 
> > > I haven't worked out where that kmap_atomic() call is coming from yet. 
> > > Both traces point up into the page allocator, but I _think_ that's stack
> > > gunk.
> > 
> > Ahh, you suspect networking.
> > 
> > Here, setup is 2 cheap-ass 100Mb realtek 8139 NICs, one to campus network
> > receiving ~20 junk packets per second, one gathering netconsole output
> > and ssh to it, no conntracks and fancy stuff.
> > 
> > [reboots with cables physically unplugged]
> 
> OK, I run gdb recompile, cat(1) every file in /usr/portage (shitload of
> small files) with both cables unplugged. It all went fine for ~5 minutes
> after that it crashed exactly same way after 10 secs after plugging one
> of them.

It'd be nice to get a clean trace.  Are you able to obtain the full
trace with CONFIG_FRAME_POINTER=y?

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Linus Torvalds]

On Mon, 23 Jul 2007, Andrew Morton wrote:
> 
> It'd be nice to get a clean trace.  Are you able to obtain the full
> trace with CONFIG_FRAME_POINTER=y?

If you are talking about

	http://userweb.kernel.org/~akpm/dsc03659.jpg

then I think that _is_ a full trace. It's certainly not very messy, and it 
seems accurate. It's just that inlining makes it much harder to see the 
call-graphs, but that's what inlining does..

For example, missing from the call graph is

	get_page_from_freelist ->
	  buffered_rmqueue ->		[ missing - inlined ]
	    prep_new_page ->		[ missing - inlined ]
	      prep_zero_page ->		[ missing - inlined ]
		clear_highpage ->	[ missing - inlined ]
		  kmap_atomic ->	[ missing - tailcall ]
		    kmap_atomic_prot

but I'm pretty sure the call trace is good (and I'm also pretty sure gcc 
is overly aggressive at inlining, and that it causes us pain for 
debugging, but whatever)

The earlier part of the trace looks fine too.

The only odd part I see is the existence of "dput()" there, so maybe it's 
not *quite* clean and enabling frame pointers might get rid of a few bogus 
entries, but it looks pretty close to clean.

			Linus

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Adrian Bunk]

On Mon, Jul 23, 2007 at 02:28:11PM -0700, Linus Torvalds wrote:
> 
> 
> On Mon, 23 Jul 2007, Andrew Morton wrote:
> > 
> > It'd be nice to get a clean trace.  Are you able to obtain the full
> > trace with CONFIG_FRAME_POINTER=y?
> 
> If you are talking about
> 
> 	http://userweb.kernel.org/~akpm/dsc03659.jpg
> 
> then I think that _is_ a full trace. It's certainly not very messy, and it 
> seems accurate. It's just that inlining makes it much harder to see the 
> call-graphs, but that's what inlining does..
> 
> For example, missing from the call graph is
> 
> 	get_page_from_freelist ->
> 	  buffered_rmqueue ->		[ missing - inlined ]
> 	    prep_new_page ->		[ missing - inlined ]
> 	      prep_zero_page ->		[ missing - inlined ]
> 		clear_highpage ->	[ missing - inlined ]
> 		  kmap_atomic ->	[ missing - tailcall ]
> 		    kmap_atomic_prot
> 
> but I'm pretty sure the call trace is good (and I'm also pretty sure gcc 
> is overly aggressive at inlining, and that it causes us pain for 
> debugging, but whatever)
>...

For prep_zero_page() and clear_highpage() we can't blame gcc since we 
force gcc to always inline them.

buffered_rmqueue() and prep_new_page() are static functions with only 
one caller each, and for the normal non-debug case it's a really nice 
optimization to have them inlined automatically. But it might make sense 
to add -fno-inline-functions-called-once to the CFLAGS depending on some 
debug option?

> 			Linus

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()

[Linus Torvalds]

On Tue, 24 Jul 2007, Adrian Bunk wrote:
> 
> buffered_rmqueue() and prep_new_page() are static functions with only 
> one caller each, and for the normal non-debug case it's a really nice 
> optimization to have them inlined automatically.

I'm not at all sure I agree.

Inlining big functions doesn't actually tend to generally generate any 
better code, so if gcc's logic is "single callsite - always inline", then 
that logic is likely not right.

		Linus

Re: commit 7e92b4fc34 - x86, serial: convert legacy COM ports to platform devices - broke my serial console

[H. Peter Anvin]

Jeff Garzik wrote:
> 
> On Tue, 24 Jul 2007, Adrian Bunk wrote:
>> buffered_rmqueue() and prep_new_page() are static functions with only 
>> one caller each, and for the normal non-debug case it's a really nice 
>> optimization to have them inlined automatically.
> 
> I'm not at all sure I agree.
> 
> Inlining big functions doesn't actually tend to generally generate any 
> better code, so if gcc's logic is "single callsite - always inline", then 
> that logic is likely not right.
> 

Only up to a threshold, as far as I know.

	-hpa