LinuxLists.cc - 2.6.26-rc8-mm1

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

KOSAKI Motohiro wrote:
> Hi Michael,
>
> my server output following error message on 2.6.26-rc8-mm1.
> Is this a bug?
>
> ------------------------------------------------------------------
> tg3.c:v3.93 (May 22, 2008)
> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51
> tg3 0000:06:01.0: PCI INT A -> GSI 72 (level, low) -> IRQ 51
> firmware: requesting tigon/tg3_tso.bin
> tg3: Failed to load firmware "tigon/tg3_tso.bin"
> tg3 0000:06:01.0: PCI INT A disabled
> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51 unregistered
> tg3: probe of 0000:06:01.0 failed with error -2
> GSI 73 (level, low) -> CPU 0 (0x0001) vector 51
> tg3 0000:06:01.1: PCI INT B -> GSI 73 (level, low) -> IRQ 52
> firmware: requesting tigon/tg3_tso.bin

This change did not come from the network developers or Broadcom, so
someone else broke tg3 in -mm...

Jeff

2008-07-03 12:33:44

by Kamalesh Babulal

[permalink] [raw]

Subject: [BUILD-FAILURE] 2.6.26-rc8-mm1 - x86 - __ptep_modify_prot_start() missing

Hi Andrew,

2.6.26-rc8-mm1 kernel build fails on the x86

CC arch/x86/kernel/paravirt.o
arch/x86/kernel/paravirt.c:383: error: ‘__ptep_modify_prot_start’ undeclared here (not in a function)
make[1]: *** [arch/x86/kernel/paravirt.o] Error 1
make: *** [arch/x86/kernel] Error 2

linux-next patches has the changes to the adds the function
__ptep_modify_prot_start as inline, the patch s390-build-fixes.patch
is coverting it into macro. Reverting the s390-build-fixes.patch
fixes the build failure.

--
Thanks & Regards,
Kamalesh Babulal,
Linux Technology Center,
IBM, ISTL.

2008-07-03 13:04:50

by Hugh Dickins

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 3 Jul 2008, Jeff Garzik wrote:
> KOSAKI Motohiro wrote:
> > Hi Michael,
> >
> > my server output following error message on 2.6.26-rc8-mm1.
> > Is this a bug?
> >
> > ------------------------------------------------------------------
> > tg3.c:v3.93 (May 22, 2008)
> > GSI 72 (level, low) -> CPU 0 (0x0001) vector 51
> > tg3 0000:06:01.0: PCI INT A -> GSI 72 (level, low) -> IRQ 51
> > firmware: requesting tigon/tg3_tso.bin
> > tg3: Failed to load firmware "tigon/tg3_tso.bin"
> > tg3 0000:06:01.0: PCI INT A disabled
> > GSI 72 (level, low) -> CPU 0 (0x0001) vector 51 unregistered
> > tg3: probe of 0000:06:01.0 failed with error -2
> > GSI 73 (level, low) -> CPU 0 (0x0001) vector 51
> > tg3 0000:06:01.1: PCI INT B -> GSI 73 (level, low) -> IRQ 52
> > firmware: requesting tigon/tg3_tso.bin
>
> This change did not come from the network developers or Broadcom, so someone
> else broke tg3 in -mm...

I think it's a consequence of not choosing CONFIG_FIRMWARE_IN_KERNEL=y.

That caught me out on PowerMac G5 trying mmotm yesterday, it just hung
for a few minutes in earlyish boot with a message about tg3_tso.bin,
and then proceeded to boot up but without the network. I was unclear
whether I'd been stupid, or the FIRMWARE_IN_KERNEL Kconfigery was poor.

I avoid initrd, and have tigon3 built in, if that's of any relevance.

I wonder if that's Andrew's problem with 2.6.26-rc8-mm1 on his G5:
mine here boots up fine (now I know to CONFIG_FIRMWARE_IN_KERNEL=y).

Hugh

2008-07-03 13:11:32

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Hugh Dickins wrote:
> On Thu, 3 Jul 2008, Jeff Garzik wrote:
>> KOSAKI Motohiro wrote:
>>> Hi Michael,
>>>
>>> my server output following error message on 2.6.26-rc8-mm1.
>>> Is this a bug?
>>>
>>> ------------------------------------------------------------------
>>> tg3.c:v3.93 (May 22, 2008)
>>> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51
>>> tg3 0000:06:01.0: PCI INT A -> GSI 72 (level, low) -> IRQ 51
>>> firmware: requesting tigon/tg3_tso.bin
>>> tg3: Failed to load firmware "tigon/tg3_tso.bin"
>>> tg3 0000:06:01.0: PCI INT A disabled
>>> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51 unregistered
>>> tg3: probe of 0000:06:01.0 failed with error -2
>>> GSI 73 (level, low) -> CPU 0 (0x0001) vector 51
>>> tg3 0000:06:01.1: PCI INT B -> GSI 73 (level, low) -> IRQ 52
>>> firmware: requesting tigon/tg3_tso.bin
>> This change did not come from the network developers or Broadcom, so someone
>> else broke tg3 in -mm...
>
> I think it's a consequence of not choosing CONFIG_FIRMWARE_IN_KERNEL=y.
>
> That caught me out on PowerMac G5 trying mmotm yesterday, it just hung
> for a few minutes in earlyish boot with a message about tg3_tso.bin,
> and then proceeded to boot up but without the network. I was unclear
> whether I'd been stupid, or the FIRMWARE_IN_KERNEL Kconfigery was poor.
>
> I avoid initrd, and have tigon3 built in, if that's of any relevance.
>
> I wonder if that's Andrew's problem with 2.6.26-rc8-mm1 on his G5:
> mine here boots up fine (now I know to CONFIG_FIRMWARE_IN_KERNEL=y).

dwmw2 has been told repeatedly that his changes will cause PRECISELY
these problems, but he refuses to take the simple steps necessary to
ensure people can continue to boot their kernels after his changes go in.

Presently his tg3 changes have been nak'd, in part, because of this
obviously, forseeable, work-around-able breakage.

Jeff

2008-07-03 13:33:46

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 2008-07-03 at 09:11 -0400, Jeff Garzik wrote:
> Hugh Dickins wrote:
> > On Thu, 3 Jul 2008, Jeff Garzik wrote:
> >> KOSAKI Motohiro wrote:
> >>> Hi Michael,
> >>>
> >>> my server output following error message on 2.6.26-rc8-mm1.
> >>> Is this a bug?
> >>>
> >>> ------------------------------------------------------------------
> >>> tg3.c:v3.93 (May 22, 2008)
> >>> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51
> >>> tg3 0000:06:01.0: PCI INT A -> GSI 72 (level, low) -> IRQ 51
> >>> firmware: requesting tigon/tg3_tso.bin
> >>> tg3: Failed to load firmware "tigon/tg3_tso.bin"
> >>> tg3 0000:06:01.0: PCI INT A disabled
> >>> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51 unregistered
> >>> tg3: probe of 0000:06:01.0 failed with error -2
> >>> GSI 73 (level, low) -> CPU 0 (0x0001) vector 51
> >>> tg3 0000:06:01.1: PCI INT B -> GSI 73 (level, low) -> IRQ 52
> >>> firmware: requesting tigon/tg3_tso.bin
> >> This change did not come from the network developers or Broadcom, so someone
> >> else broke tg3 in -mm...
> >
> > I think it's a consequence of not choosing CONFIG_FIRMWARE_IN_KERNEL=y.
> >
> > That caught me out on PowerMac G5 trying mmotm yesterday, it just hung
> > for a few minutes in earlyish boot with a message about tg3_tso.bin,
> > and then proceeded to boot up but without the network. I was unclear
> > whether I'd been stupid, or the FIRMWARE_IN_KERNEL Kconfigery was poor.

I shall respectfully refrain from commenting on the likelihood of the
former. With regard to the latter, here is the help text for the
FIRMWARE_IN_KERNEL option:

help
The kernel source tree includes a number of firmware 'blobs'
which are used by various drivers. The recommended way to
use these is to run "make firmware_install" and to copy the
resulting binary files created in usr/lib/firmware directory
of the kernel tree to the /lib/firmware on your system so
that they can be loaded by userspace helpers on request.

Enabling this option will build each required firmware blob
into the kernel directly, where request_firmware() will find
them without having to call out to userspace. This may be
useful if your root file system requires a device which uses
such firmware, and do not wish to use an initrd.

This single option controls the inclusion of firmware for
every driver which usees request_firmare() and ships its
firmware in the kernel source tree, to avoid a proliferation
of 'Include firmware for xxx device' options.

Say 'N' and let firmware be loaded from userspace.

If you think you can improve it, please let me have a revised attempt.

> > I avoid initrd, and have tigon3 built in, if that's of any relevance.
> >
> > I wonder if that's Andrew's problem with 2.6.26-rc8-mm1 on his G5:
> > mine here boots up fine (now I know to CONFIG_FIRMWARE_IN_KERNEL=y).
>
>
> dwmw2 has been told repeatedly that his changes will cause PRECISELY
> these problems, but he refuses to take the simple steps necessary to
> ensure people can continue to boot their kernels after his changes go in.

Complete nonsense. Setting CONFIG_FIRMWARE_IN_KERNEL isn't hard. But
shouldn't be the _default_, either.

> Presently his tg3 changes have been nak'd, in part, because of this
> obviously, forseeable, work-around-able breakage.

They haven't even been reviewed. Nobody seems to have actually looked at
the real changes (in particular, and commented on whether the device can
run anyway without the TSO firmware being loaded, as some people seem to
report). You're just throwing your toys out of the pram because of the
'default n' on a patch about 30 commits earlier in my tree.

--
dwmw2

2008-07-03 13:39:00

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
>> dwmw2 has been told repeatedly that his changes will cause PRECISELY
>> these problems, but he refuses to take the simple steps necessary to
>> ensure people can continue to boot their kernels after his changes go in.
>
> Complete nonsense. Setting CONFIG_FIRMWARE_IN_KERNEL isn't hard. But
> shouldn't be the _default_, either.
>
>> Presently his tg3 changes have been nak'd, in part, because of this
>> obviously, forseeable, work-around-able breakage.
>
> They haven't even been reviewed. Nobody seems to have actually looked at

Yes, they have. You just didn't like the answers you received.

In particular, the Kconfig default for built-in tg3 firmware should
result in the current behavior, without the user having to take extra steps.

Because of your stubborn refusal on this Kconfig defaults issue, WE
ALREADY HAVE DRIVER-DOES-NOT-WORK BREAKAGE, JUST AS PREDICTED.

Wake up and smell reality. Please.

Jeff

2008-07-03 13:53:22

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 2008-07-03 at 09:38 -0400, Jeff Garzik wrote:
> David Woodhouse wrote:
> >> dwmw2 has been told repeatedly that his changes will cause PRECISELY
> >> these problems, but he refuses to take the simple steps necessary to
> >> ensure people can continue to boot their kernels after his changes go in.
> >
> > Complete nonsense. Setting CONFIG_FIRMWARE_IN_KERNEL isn't hard. But
> > shouldn't be the _default_, either.
> >
> >> Presently his tg3 changes have been nak'd, in part, because of this
> >> obviously, forseeable, work-around-able breakage.
> >
> > They haven't even been reviewed. Nobody seems to have actually looked at
>
>
> Yes, they have. You just didn't like the answers you received.

I received no comment on any part of the changes within tg3.c; only
whining about the default behaviour -- which isn't even _set_ as part of
the patch in question, any more.

> In particular, the Kconfig default for built-in tg3 firmware should
> result in the current behavior, without the user having to take extra steps.

After feedback from a number of people, there is no individual Kconfig
option for the various firmwares; there is only one which controls them
all -- CONFIG_FIRMWARE_IN_KERNEL. The thing you're whining about isn't
even part of the patch which needs review.

> Because of your stubborn refusal on this Kconfig defaults issue, WE
> ALREADY HAVE DRIVER-DOES-NOT-WORK BREAKAGE, JUST AS PREDICTED.

I strongly disagree that CONFIG_FIRMWARE_IN_KERNEL=y should be the
default. But if I add this patch elsewhere in the kernel, will you quit
your whining and actually review the patch you were asked to review? ...

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 339c148..d47482f 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -37,6 +37,7 @@ config FW_LOADER
config FIRMWARE_IN_KERNEL
bool "Include in-kernel firmware blobs in kernel binary"
depends on FW_LOADER
+ default y
help
The kernel source tree includes a number of firmware 'blobs'
which are used by various drivers. The recommended way to

--
dwmw2

2008-07-03 14:27:32

by Jiri Slaby

[permalink] [raw]

Subject: WARNING at acpi/.../utmisc.c:1043 [Was: 2.6.26-rc8-mm1]

Andrew Morton napsal(a):
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.26-rc8/2.6.26-rc8-mm1/

Running this in qemu shows up these 3 warnings while booting (It's tainted
due to previous MTRR warning which was there for ever):

PCI: Using configuration type 1 for base access
------------[ cut here ]------------
WARNING: at /home/latest/xxx/drivers/acpi/utilities/utmisc.c:1043
acpi_ut_exception+0x3c/0xb9()
Modules linked in:
Pid: 1, comm: swapper Tainted: G AW 2.6.26-rc8-mm1-nohz #7

Call Trace:
[<ffffffff8023c2df>] warn_on_slowpath+0x5f/0x90
[<ffffffff803674c1>] ? acpi_ns_evaluate+0x39/0x1c4
[<ffffffff8036719c>] ? acpi_evaluate_object+0x1ea/0x1fe
[<ffffffff803736a4>] ? ec_parse_io_ports+0x0/0x34
[<ffffffff8036f32e>] ? acpi_ut_remove_reference+0x2d/0x31
[<ffffffff80366a55>] ? acpi_ns_search_one_scope+0x1d/0x46
[<ffffffff80366b3a>] ? acpi_ns_search_and_enter+0xbc/0x18a
[<ffffffff8036e24e>] acpi_ut_exception+0x3c/0xb9
[<ffffffff8052b1c0>] ? _spin_unlock_irqrestore+0x30/0x40
[<ffffffff80257a74>] ? up+0x34/0x50
[<ffffffff8052b1c0>] ? _spin_unlock_irqrestore+0x30/0x40
[<ffffffff80257acc>] ? down_timeout+0x3c/0x60
[<ffffffff8052b1c0>] ? _spin_unlock_irqrestore+0x30/0x40
[<ffffffff80257a74>] ? up+0x34/0x50
[<ffffffff803737f7>] ? acpi_ec_gpe_handler+0x0/0x109
[<ffffffff8035e475>] acpi_install_gpe_handler+0x12c/0x13a
[<ffffffff8070f829>] ? acpi_init+0x0/0x221
[<ffffffff80374121>] ec_install_handlers+0x2e/0x9c
[<ffffffff8070fe1e>] acpi_ec_ecdt_probe+0xee/0x124
[<ffffffff8070f8ae>] acpi_init+0x85/0x221
[<ffffffff8033a3db>] ? kset_create_and_add+0x6b/0xa0
[<ffffffff8034b2e0>] ? pci_slot_init+0x0/0x50
[<ffffffff806f95b7>] do_one_initcall+0x35/0x15d
[<ffffffff802719f8>] ? register_irq_proc+0xe8/0x110
[<ffffffff802f0000>] ? __inode_dir_notify+0x30/0xf0
[<ffffffff806f987a>] kernel_init+0x19b/0x1a6
[<ffffffff802396f7>] ? schedule_tail+0x27/0x60
[<ffffffff8020c788>] child_rip+0xa/0x12
[<ffffffff806f96df>] ? kernel_init+0x0/0x1a6
[<ffffffff8020c77e>] ? child_rip+0x0/0x12

---[ end trace 4eaa2a86a8e2da22 ]---
ACPI Exception (evxface-0645): AE_BAD_PARAMETER, Installing notify handler
failed [20080609]
ACPI: Interpreter enabled

ACPI: EC: driver started in poll mode
------------[ cut here ]------------
WARNING: at /home/latest/xxx/drivers/acpi/utilities/utmisc.c:1043
acpi_ut_exception+0x3c/0xb9()
Modules linked in:
Pid: 1, comm: swapper Tainted: G AW 2.6.26-rc8-mm1-nohz #7

Call Trace:
[<ffffffff8023c2df>] warn_on_slowpath+0x5f/0x90
[<ffffffff80340963>] ? __const_udelay+0x43/0x50
[<ffffffff8023c61e>] ? __call_console_drivers+0x6e/0x90
[<ffffffff80257a74>] ? up+0x34/0x50
[<ffffffff8023cc04>] ? release_console_sem+0x1e4/0x1f0
[<ffffffff8036e24e>] acpi_ut_exception+0x3c/0xb9
[<ffffffff80338d82>] ? idr_get_empty_slot+0x102/0x2b0
[<ffffffff80257acc>] ? down_timeout+0x3c/0x60
[<ffffffff80257acc>] ? down_timeout+0x3c/0x60
[<ffffffff80257a74>] ? up+0x34/0x50
[<ffffffff803737f7>] ? acpi_ec_gpe_handler+0x0/0x109
[<ffffffff8035e475>] acpi_install_gpe_handler+0x12c/0x13a
[<ffffffff80374121>] ec_install_handlers+0x2e/0x9c
[<ffffffff803741af>] acpi_ec_start+0x20/0x44
[<ffffffff80371c07>] acpi_start_single_object+0x2a/0x54
[<ffffffff80372341>] acpi_device_probe+0x78/0x8c
[<ffffffff803b2892>] driver_probe_device+0xa2/0x1e0
[<ffffffff803b2a5b>] __driver_attach+0x8b/0x90
[<ffffffff803b29d0>] ? __driver_attach+0x0/0x90
[<ffffffff803b204b>] bus_for_each_dev+0x6b/0xa0
[<ffffffff80339dca>] ? kobject_get+0x1a/0x30
[<ffffffff803b26dc>] driver_attach+0x1c/0x20
[<ffffffff803b18d8>] bus_add_driver+0x208/0x280
[<ffffffff8070fc94>] ? acpi_ec_init+0x0/0x61
[<ffffffff803b2c40>] driver_register+0x70/0x160
[<ffffffff8070fc94>] ? acpi_ec_init+0x0/0x61
[<ffffffff8037264a>] acpi_bus_register_driver+0x3e/0x40
[<ffffffff8070fcd3>] acpi_ec_init+0x3f/0x61
[<ffffffff806f95b7>] do_one_initcall+0x35/0x15d
[<ffffffff802719f8>] ? register_irq_proc+0xe8/0x110
[<ffffffff802f0000>] ? __inode_dir_notify+0x30/0xf0
[<ffffffff806f987a>] kernel_init+0x19b/0x1a6
[<ffffffff802396f7>] ? schedule_tail+0x27/0x60
[<ffffffff8020c788>] child_rip+0xa/0x12
[<ffffffff806f96df>] ? kernel_init+0x0/0x1a6
[<ffffffff8020c77e>] ? child_rip+0x0/0x12

---[ end trace 4eaa2a86a8e2da22 ]---
ACPI Exception (evxface-0645): AE_BAD_PARAMETER, Installing notify handler
failed [20080609]
ACPI: PCI Root Bridge [PCI0] (0000:00)

processor ACPI0007:00: registered as cooling_device0
------------[ cut here ]------------
WARNING: at /home/latest/xxx/drivers/acpi/utilities/utmisc.c:1043
acpi_ut_exception+0x3c/0xb9()
Modules linked in:
Pid: 1, comm: swapper Tainted: G AW 2.6.26-rc8-mm1-nohz #7

Call Trace:
[<ffffffff8023c2df>] warn_on_slowpath+0x5f/0x90
[<ffffffff80257a74>] ? up+0x34/0x50
[<ffffffff803576d6>] ? acpi_os_release_object+0x9/0xd
[<ffffffff8036f92c>] ? acpi_ut_delete_object_desc+0x48/0x4c
[<ffffffff8036f103>] ? acpi_ut_delete_internal_obj+0x167/0x16f
[<ffffffff8036f162>] ? acpi_ut_update_ref_count+0x57/0xa3
[<ffffffff8036f2a5>] ? acpi_ut_update_object_reference+0xf7/0x153
[<ffffffff8036e24e>] acpi_ut_exception+0x3c/0xb9
[<ffffffff80357912>] ? acpi_os_signal_semaphore+0x23/0x27
[<ffffffff8036719c>] ? acpi_evaluate_object+0x1ea/0x1fe
[<ffffffff8036f103>] ? acpi_ut_delete_internal_obj+0x167/0x16f
[<ffffffff803582e2>] ? acpi_evaluate_integer+0xbf/0xd1
[<ffffffff8037cbb6>] acpi_thermal_trips_update+0x6a/0x56c
[<ffffffff8036719c>] ? acpi_evaluate_object+0x1ea/0x1fe
[<ffffffff802fe7c0>] ? sysfs_ilookup_test+0x0/0x20
[<ffffffff8052b27e>] ? _spin_unlock+0x2e/0x40
[<ffffffff803582e2>] ? acpi_evaluate_integer+0xbf/0xd1
[<ffffffff8037d9e8>] acpi_thermal_add+0x3cf/0x43e
[<ffffffff80372312>] acpi_device_probe+0x49/0x8c
[<ffffffff803b2892>] driver_probe_device+0xa2/0x1e0
[<ffffffff803b2a5b>] __driver_attach+0x8b/0x90
[<ffffffff803b29d0>] ? __driver_attach+0x0/0x90
[<ffffffff803b204b>] bus_for_each_dev+0x6b/0xa0
[<ffffffff80339dca>] ? kobject_get+0x1a/0x30
[<ffffffff803b26dc>] driver_attach+0x1c/0x20
[<ffffffff803b18d8>] bus_add_driver+0x208/0x280
[<ffffffff8071032f>] ? acpi_thermal_init+0x0/0x83
[<ffffffff803b2c40>] driver_register+0x70/0x160
[<ffffffff8071032f>] ? acpi_thermal_init+0x0/0x83
[<ffffffff8037264a>] acpi_bus_register_driver+0x3e/0x40
[<ffffffff80710390>] acpi_thermal_init+0x61/0x83
[<ffffffff806f95b7>] do_one_initcall+0x35/0x15d
[<ffffffff802719f8>] ? register_irq_proc+0xe8/0x110
[<ffffffff802f0000>] ? __inode_dir_notify+0x30/0xf0
[<ffffffff806f987a>] kernel_init+0x19b/0x1a6
[<ffffffff802396f7>] ? schedule_tail+0x27/0x60
[<ffffffff8020c788>] child_rip+0xa/0x12
[<ffffffff806f96df>] ? kernel_init+0x0/0x1a6
[<ffffffff8020c77e>] ? child_rip+0x0/0x12

---[ end trace 4eaa2a86a8e2da22 ]---
ACPI Exception (thermal-0377): AE_OK, No or invalid critical threshold
[20080609]
Real Time Clock Driver v1.12ac

2008-07-03 14:38:57

[permalink] [raw]

Subject: Re: WARNING at acpi/.../utmisc.c:1043 [Was: 2.6.26-rc8-mm1]

Jiri Slaby wrote:
> Andrew Morton napsal(a):
>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.26-rc8/2.6.26-rc8-mm1/
>>
>
> Running this in qemu shows up these 3 warnings while booting (It's
> tainted due to previous MTRR warning which was there for ever):
>
> PCI: Using configuration type 1 for base access
> ------------[ cut here ]------------
> WARNING: at /home/latest/xxx/drivers/acpi/utilities/utmisc.c:1043

Not sure where that is coming from. My tree and my copy of linux-next
doesn't have a WARN_ON in this function.

Anyways, I assume you always saw this message right?

> ACPI Exception (evxface-0645): AE_BAD_PARAMETER, Installing notify
> handler failed [20080609]
> ACPI: Interpreter enabled

And the only thing new is the backtrace right?

Similar with the other messages. If you ignore the backtraces
is there any difference?

-Andi

2008-07-03 15:25:25

by Jiri Slaby

[permalink] [raw]

Subject: Re: WARNING at acpi/.../utmisc.c:1043 [Was: 2.6.26-rc8-mm1]

Andi Kleen napsal(a):
> Jiri Slaby wrote:
>> Andrew Morton napsal(a):
>>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.26-rc8/2.6.26-rc8-mm1/
>>>
>>
>> Running this in qemu shows up these 3 warnings while booting (It's
>> tainted due to previous MTRR warning which was there for ever):
>>
>> PCI: Using configuration type 1 for base access
>> ------------[ cut here ]------------
>> WARNING: at /home/latest/xxx/drivers/acpi/utilities/utmisc.c:1043
>
> Not sure where that is coming from. My tree and my copy of linux-next
> doesn't have a WARN_ON in this function.

It's from
acpi-utmisc-use-warn_on-instead-of-warn_on_slowpath.patch

> Anyways, I assume you always saw this message right?

Yes (almost, see below), I've checked this now.

>> ACPI Exception (evxface-0645): AE_BAD_PARAMETER, Installing notify
>> handler failed [20080609]
>> ACPI: Interpreter enabled
>
> And the only thing new is the backtrace right?

In the two cases. The thermal one is new:

processor ACPI0007:00: registered as cooling_device0
-thermal LNXTHERM:01: registered as thermal_zone0
-ACPI: Critical trip point
-Critical temperature reached (60 C), shutting down.
-ACPI: Thermal Zone [THRM] (60 C)
+------------[ cut here ]------------
+WARNING: at /home/latest/xxx/drivers/acpi/utilities/utmisc.c:1043
acpi_ut_exception+0x3c/0xb9()
+Modules linked in:
+Pid: 1, comm: swapper Tainted: G AW 2.6.26-rc8-mm1-nohz #9
+
+Call Trace:
+ [<ffffffff8023c2df>] warn_on_slowpath+0x5f/0x90
+ [<ffffffff80257a74>] ? up+0x34/0x50
+ [<ffffffff803576d6>] ? acpi_os_release_object+0x9/0xd
+ [<ffffffff8036f92c>] ? acpi_ut_delete_object_desc+0x48/0x4c
+ [<ffffffff8036f103>] ? acpi_ut_delete_internal_obj+0x167/0x16f
+ [<ffffffff8036f162>] ? acpi_ut_update_ref_count+0x57/0xa3
+ [<ffffffff8036f2a5>] ? acpi_ut_update_object_reference+0xf7/0x153
+ [<ffffffff8036e24e>] acpi_ut_exception+0x3c/0xb9
+ [<ffffffff80357912>] ? acpi_os_signal_semaphore+0x23/0x27
+ [<ffffffff8036719c>] ? acpi_evaluate_object+0x1ea/0x1fe
+ [<ffffffff8036f103>] ? acpi_ut_delete_internal_obj+0x167/0x16f
+ [<ffffffff803582e2>] ? acpi_evaluate_integer+0xbf/0xd1
+ [<ffffffff8037cbb6>] acpi_thermal_trips_update+0x6a/0x56c
+ [<ffffffff8036719c>] ? acpi_evaluate_object+0x1ea/0x1fe
+ [<ffffffff802fe7c0>] ? sysfs_ilookup_test+0x0/0x20
+ [<ffffffff8052b27e>] ? _spin_unlock+0x2e/0x40
+ [<ffffffff803582e2>] ? acpi_evaluate_integer+0xbf/0xd1
+ [<ffffffff8037d9e8>] acpi_thermal_add+0x3cf/0x43e
+ [<ffffffff80372312>] acpi_device_probe+0x49/0x8c
+ [<ffffffff803b2892>] driver_probe_device+0xa2/0x1e0
+ [<ffffffff803b2a5b>] __driver_attach+0x8b/0x90
+ [<ffffffff803b29d0>] ? __driver_attach+0x0/0x90
+ [<ffffffff803b204b>] bus_for_each_dev+0x6b/0xa0
+ [<ffffffff80339dca>] ? kobject_get+0x1a/0x30
+ [<ffffffff803b26dc>] driver_attach+0x1c/0x20
+ [<ffffffff803b18d8>] bus_add_driver+0x208/0x280
+ [<ffffffff8071032f>] ? acpi_thermal_init+0x0/0x83
+ [<ffffffff803b2c40>] driver_register+0x70/0x160
+ [<ffffffff8071032f>] ? acpi_thermal_init+0x0/0x83
+ [<ffffffff8037264a>] acpi_bus_register_driver+0x3e/0x40
+ [<ffffffff80710390>] acpi_thermal_init+0x61/0x83
+ [<ffffffff806f95b7>] do_one_initcall+0x35/0x15d
+ [<ffffffff802719f8>] ? register_irq_proc+0xe8/0x110
+ [<ffffffff802f0000>] ? __inode_dir_notify+0x30/0xf0
+ [<ffffffff806f987a>] kernel_init+0x19b/0x1a6
+ [<ffffffff802396f7>] ? schedule_tail+0x27/0x60
+ [<ffffffff8020c788>] child_rip+0xa/0x12
+ [<ffffffff806f96df>] ? kernel_init+0x0/0x1a6
+ [<ffffffff8020c77e>] ? child_rip+0x0/0x12
+
+---[ end trace 4eaa2a86a8e2da22 ]---
+ACPI Exception (thermal-0377): AE_OK, No or invalid critical threshold
[20080609]
Real Time Clock Driver v1.12ac

2008-07-03 16:11:07

by Chuck Lever III

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Jul 3, 2008, at 7:59 AM, KOSAKI Motohiro wrote:
> Hi Michael,
>
> my server output following error message on 2.6.26-rc8-mm1.
> Is this a bug?
>
> ------------------------------------------------------------------
> tg3.c:v3.93 (May 22, 2008)
> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51
> tg3 0000:06:01.0: PCI INT A -> GSI 72 (level, low) -> IRQ 51
> firmware: requesting tigon/tg3_tso.bin
> tg3: Failed to load firmware "tigon/tg3_tso.bin"
> tg3 0000:06:01.0: PCI INT A disabled
> GSI 72 (level, low) -> CPU 0 (0x0001) vector 51 unregistered
> tg3: probe of 0000:06:01.0 failed with error -2
> GSI 73 (level, low) -> CPU 0 (0x0001) vector 51
> tg3 0000:06:01.1: PCI INT B -> GSI 73 (level, low) -> IRQ 52
> firmware: requesting tigon/tg3_tso.bin

Same problem here with linux-next on a Dell Latitude D620.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

2008-07-03 17:33:20

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, Jul 03, 2008 at 02:52:55PM +0100, David Woodhouse wrote:
>
> After feedback from a number of people, there is no individual Kconfig
> option for the various firmwares; there is only one which controls them
> all -- CONFIG_FIRMWARE_IN_KERNEL. The thing you're whining about isn't
> even part of the patch which needs review.
>
> > Because of your stubborn refusal on this Kconfig defaults issue, WE
> > ALREADY HAVE DRIVER-DOES-NOT-WORK BREAKAGE, JUST AS PREDICTED.
>
> I strongly disagree that CONFIG_FIRMWARE_IN_KERNEL=y should be the
> default. But if I add this patch elsewhere in the kernel, will you quit
> your whining and actually review the patch you were asked to review? ...

I don't think it's whining. If your patch introduces changes which
cause people .config to break by default after upgrading to a newer
kernel and doing "make oldconfig" --- then that's a problem with your
patch, and the missing hunk to enable CONFIG_FIRMWARE_IN_KERNEL=y is
critically important.

Linus has ruled this way in the past, when he's gotten screwed by this
sort of issue in the past, and he was justifiably annoyed. We should
treat the users who are willing to test and provide feedback on the
latest kernel.org kernels with the same amount of regard. And if
there are licensing religious fundamentalists who feel strongly about
the firmware issue, then fine, they can change the .config. But the
default should be to avoid users from having broken kernels, and a
number of (quite clueful) users have already demonstrated that without
setting CONFIG_FIRMWARE_IN_KERNEL=y as the default, your patches cause
breakage.

Regards,

- Ted

2008-07-03 17:48:20

[permalink] [raw]

Subject: Re: [BUILD-FAILURE] 2.6.26-rc8-mm1 - x86 - __ptep_modify_prot_start() missing

On Thu, 3 Jul 2008 17:59:43 +0530 Kamalesh Babulal <[email protected]> wrote:

> Hi Andrew,
>
> 2.6.26-rc8-mm1 kernel build fails on the x86
>
> CC arch/x86/kernel/paravirt.o
> arch/x86/kernel/paravirt.c:383: error: _____ptep_modify_prot_start___ undeclared here (not in a function)
> make[1]: *** [arch/x86/kernel/paravirt.o] Error 1
> make: *** [arch/x86/kernel] Error 2
>
> linux-next patches has the changes to the adds the function
> __ptep_modify_prot_start as inline, the patch s390-build-fixes.patch
> is coverting it into macro. Reverting the s390-build-fixes.patch
> fixes the build failure.

grump. Who did all this stuff?

I dunno. I'll drop s390-build-fixes.patch, add some ccs and stomp off.

From: Andrew Morton <[email protected]>

In file included from include/asm/pgtable.h:1087,
from include/linux/mm.h:39,
from arch/s390/mm/hugetlbpage.c:8:
include/asm-generic/pgtable.h: In function '__ptep_modify_prot_start':
include/asm-generic/pgtable.h:209: error: dereferencing pointer to incomplete type

Cc: Heiko Carstens <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

include/asm-generic/pgtable.h | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)

diff -puN include/asm-generic/pgtable.h~s390-build-fixes include/asm-generic/pgtable.h
--- a/include/asm-generic/pgtable.h~s390-build-fixes
+++ a/include/asm-generic/pgtable.h
@@ -197,17 +197,13 @@ static inline int pmd_none_or_clear_bad(
}
#endif /* CONFIG_MMU */

-static inline pte_t __ptep_modify_prot_start(struct mm_struct *mm,
- unsigned long addr,
- pte_t *ptep)
-{
- /*
- * Get the current pte state, but zero it out to make it
- * non-present, preventing the hardware from asynchronously
- * updating it.
- */
- return ptep_get_and_clear(mm, addr, ptep);
-}
+/*
+ * Get the current pte state, but zero it out to make it
+ * non-present, preventing the hardware from asynchronously
+ * updating it.
+ */
+#define __ptep_modify_prot_start(mm, addr, ptep) \
+ ptep_get_and_clear(mm, addr, ptep)

static inline void __ptep_modify_prot_commit(struct mm_struct *mm,
unsigned long addr,
@@ -235,12 +231,8 @@ static inline void __ptep_modify_prot_co
* queue the update to be done at some later time. The update must be
* actually committed before the pte lock is released, however.
*/
-static inline pte_t ptep_modify_prot_start(struct mm_struct *mm,
- unsigned long addr,
- pte_t *ptep)
-{
- return __ptep_modify_prot_start(mm, addr, ptep);
-}
+#define ptep_modify_prot_start(mm, addr, ptep) \
+ __ptep_modify_prot_start(mm, addr, ptep)

/*
* Commit an update to a pte, leaving any hardware-controlled bits in
_

2008-07-03 18:00:52

by Jeremy Fitzhardinge

[permalink] [raw]

Subject: Re: [BUILD-FAILURE] 2.6.26-rc8-mm1 - x86 - __ptep_modify_prot_start() missing

Andrew Morton wrote:
> On Thu, 3 Jul 2008 17:59:43 +0530 Kamalesh Babulal <[email protected]> wrote:
>
>
>> Hi Andrew,
>>
>> 2.6.26-rc8-mm1 kernel build fails on the x86
>>
>> CC arch/x86/kernel/paravirt.o
>> arch/x86/kernel/paravirt.c:383: error: _____ptep_modify_prot_start___ undeclared here (not in a function)
>>

Where did all those _____underlines___ come from?

>> make[1]: *** [arch/x86/kernel/paravirt.o] Error 1
>> make: *** [arch/x86/kernel] Error 2
>>
>> linux-next patches has the changes to the adds the function
>> __ptep_modify_prot_start as inline, the patch s390-build-fixes.patch
>> is coverting it into macro. Reverting the s390-build-fixes.patch
>> fixes the build failure.
>>
>
> grump. Who did all this stuff?
>
> I dunno. I'll drop s390-build-fixes.patch, add some ccs and stomp off.
>
>
> From: Andrew Morton <[email protected]>
>
> In file included from include/asm/pgtable.h:1087,
> from include/linux/mm.h:39,
> from arch/s390/mm/hugetlbpage.c:8:
> include/asm-generic/pgtable.h: In function '__ptep_modify_prot_start':
> include/asm-generic/pgtable.h:209: error: dereferencing pointer to incomplete type
>

We can't turn them into macros because we're expecting to be able to
take the address of __ptep_modify_prot_start/commit. What type is not
defined on s390 at that point? Would simply adding an extra include to
arch/s390/mm/hugetlbpage.c fix the problem?

In the worst case we could push __ptep_modify_proc_start/commit out of
line somewhere appropriate, but that's a bit sad given how simple they are.

J

> Cc: Heiko Carstens <[email protected]>
> Cc: Martin Schwidefsky <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> include/asm-generic/pgtable.h | 26 +++++++++-----------------
> 1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff -puN include/asm-generic/pgtable.h~s390-build-fixes include/asm-generic/pgtable.h
> --- a/include/asm-generic/pgtable.h~s390-build-fixes
> +++ a/include/asm-generic/pgtable.h
> @@ -197,17 +197,13 @@ static inline int pmd_none_or_clear_bad(
> }
> #endif /* CONFIG_MMU */
>
> -static inline pte_t __ptep_modify_prot_start(struct mm_struct *mm,
> - unsigned long addr,
> - pte_t *ptep)
> -{
> - /*
> - * Get the current pte state, but zero it out to make it
> - * non-present, preventing the hardware from asynchronously
> - * updating it.
> - */
> - return ptep_get_and_clear(mm, addr, ptep);
> -}
> +/*
> + * Get the current pte state, but zero it out to make it
> + * non-present, preventing the hardware from asynchronously
> + * updating it.
> + */
> +#define __ptep_modify_prot_start(mm, addr, ptep) \
> + ptep_get_and_clear(mm, addr, ptep)
>
> static inline void __ptep_modify_prot_commit(struct mm_struct *mm,
> unsigned long addr,
> @@ -235,12 +231,8 @@ static inline void __ptep_modify_prot_co
> * queue the update to be done at some later time. The update must be
> * actually committed before the pte lock is released, however.
> */
> -static inline pte_t ptep_modify_prot_start(struct mm_struct *mm,
> - unsigned long addr,
> - pte_t *ptep)
> -{
> - return __ptep_modify_prot_start(mm, addr, ptep);
> -}
> +#define ptep_modify_prot_start(mm, addr, ptep) \
> + __ptep_modify_prot_start(mm, addr, ptep)
>
> /*
> * Commit an update to a pte, leaving any hardware-controlled bits in
> _
>
>
>

2008-07-03 18:16:19

[permalink] [raw]

Subject: Re: [BUILD-FAILURE] 2.6.26-rc8-mm1 - x86 - __ptep_modify_prot_start() missing

On Thu, 03 Jul 2008 11:00:35 -0700 Jeremy Fitzhardinge <[email protected]> wrote:

> Andrew Morton wrote:
> > On Thu, 3 Jul 2008 17:59:43 +0530 Kamalesh Babulal <[email protected]> wrote:
> >
> >
> >> Hi Andrew,
> >>
> >> 2.6.26-rc8-mm1 kernel build fails on the x86
> >>
> >> CC arch/x86/kernel/paravirt.o
> >> arch/x86/kernel/paravirt.c:383: error: _____ptep_modify_prot_start___ undeclared here (not in a function)
> >>
>
> Where did all those _____underlines___ come from?

gcc idiocy - emitting non-ascii characters from a programming tool.
Their survival rate through downstream handling is about 2%. LANG=C
helps.

> >> make[1]: *** [arch/x86/kernel/paravirt.o] Error 1
> >> make: *** [arch/x86/kernel] Error 2
> >>
> >> linux-next patches has the changes to the adds the function
> >> __ptep_modify_prot_start as inline, the patch s390-build-fixes.patch
> >> is coverting it into macro. Reverting the s390-build-fixes.patch
> >> fixes the build failure.
> >>
> >
> > grump. Who did all this stuff?
> >
> > I dunno. I'll drop s390-build-fixes.patch, add some ccs and stomp off.
> >
> >
> > From: Andrew Morton <[email protected]>
> >
> > In file included from include/asm/pgtable.h:1087,
> > from include/linux/mm.h:39,
> > from arch/s390/mm/hugetlbpage.c:8:
> > include/asm-generic/pgtable.h: In function '__ptep_modify_prot_start':
> > include/asm-generic/pgtable.h:209: error: dereferencing pointer to incomplete type
> >
>
> We can't turn them into macros because we're expecting to be able to
> take the address of __ptep_modify_prot_start/commit. What type is not
> defined on s390 at that point? Would simply adding an extra include to
> arch/s390/mm/hugetlbpage.c fix the problem?
>
> In the worst case we could push __ptep_modify_proc_start/commit out of
> line somewhere appropriate, but that's a bit sad given how simple they are.

I think the s390 guys just fixed the original build error.

2008-07-03 18:56:33

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 2008-07-03 at 13:30 -0400, Theodore Tso wrote:
> I don't think it's whining.

Neither is it an adequate review of the actual patch which was
submitted.

> If your patch introduces changes which
> cause people .config to break by default after upgrading to a newer
> kernel and doing "make oldconfig"

They had to 'make oldconfig' and then actually _choose_ to say 'no' to
an option which is fairly clearly documented, that they are the
relatively unusual position of wanting to have said 'yes' to. You're
getting into Aunt Tillie territory, when you complain about that.

Although it does make me wonder if it was better the way I had it
originally, with individual options like TIGON3_FIRMWARE_IN_KERNEL
attached to each driver, rather than a single FIRMWARE_IN_KERNEL option
which controls them all.

Perhaps one way to help Aunt Tillie would be to tweak Kbuild to look at
the MODULE_FIRMWARE() statements for in-kernel drivers, and to print a
warning when the build finishes: "Your static kernel image may require
the following firmware files, which are not included: ..."

It's wrong to change the CONFIG_FIRMWARE_IN_KERNEL default to 'Y',
because the _normal_ setting for that option _really_ should be 'N'.
Using request_firmware() satisfied from userspace is best practice these
days, and almost all recent drivers do it that way _unconditionally_
anyway.

What we're doing now is just cleaning up the older drivers which don't
use request_firmware(), to conform to what is now common practice. And
while we're retaining the _option_ to continue to build their firmware
into the static kernel image, it isn't recommended and really shouldn't
be the default configuration.

> Linus has ruled this way in the past, when he's gotten screwed by this
> sort of issue in the past, and he was justifiably annoyed.

I am content to let Linus decide on what the default for the
FIRMWARE_IN_KERNEL option will be. I am adamant that it _should_ be 'N',
but it's easy enough for Linus to overrule me with a one-line change.

In the meantime, it would be useful if Jeff would quit throwing his toys
out of the pram on that issue and actually review the _code_ changes. In
particular, are the reports correct that the device operates just fine
without the TSO firmware loaded? Should we change the request_firmware()
error path to just disable TSO and continue with the initialisation?

I can understand why he might not want to answer that if the answer is
affirmative, I suppose -- it detracts even _further_ from his already
rather dubious argument about 'breaking' the driver, if it'll actually
continue to work even when the firmware is completely absent. But it
would be nice to get an honest and straightforward review of the code
from _someone_ who actually knows the hardware.

> And if there are licensing religious fundamentalists who feel
> strongly about the firmware issue, then fine, they can change
> the .config.

Less of the ad hominem, please. Especially when it's so misdirected.

Updating these drivers to remove large blobs of static unswappable data
from the kernel, and having it provided from userspace on demand as
modern Linux drivers do, is a perfectly sensible technical goal all on
its own.

And given the GPL's explicit provisions with regard to collective works
there are also entirely reasonable, non-"fundamentalist" grounds for
believing that it _may_ pose a licensing problem, and for wanting to err
on the side of caution in that respect too.

Fedora is almost certain to ship with CONFIG_FIRMWARE_IN_KERNEL=n, and
I'd be very surprised if Debian and other major distributions don't
follow suit. It is the sensible, pragmatic, technically sound choice.

> But the default should be to avoid users from having broken kernels,
> and a number of (quite clueful) users have already demonstrated that
> without setting CONFIG_FIRMWARE_IN_KERNEL=y as the default, your
> patches cause breakage.

By this argument, shouldn't we include images in the static kernel for
_all_ drivers which currently use request_firmware()? Otherwise, it's
possible for the user to 'break' them, right?

--
dwmw2

2008-07-03 19:03:38

by Heiko Carstens

[permalink] [raw]

Subject: Re: [BUILD-FAILURE] 2.6.26-rc8-mm1 - x86 - __ptep_modify_prot_start() missing

On Thu, Jul 03, 2008 at 11:14:18AM -0700, Andrew Morton wrote:
> On Thu, 03 Jul 2008 11:00:35 -0700 Jeremy Fitzhardinge <[email protected]> wrote:
> > >> make[1]: *** [arch/x86/kernel/paravirt.o] Error 1
> > >> make: *** [arch/x86/kernel] Error 2
> > >>
> > >> linux-next patches has the changes to the adds the function
> > >> __ptep_modify_prot_start as inline, the patch s390-build-fixes.patch
> > >> is coverting it into macro. Reverting the s390-build-fixes.patch
> > >> fixes the build failure.
> > >>
> > >
> > > grump. Who did all this stuff?
> > >
> > > I dunno. I'll drop s390-build-fixes.patch, add some ccs and stomp off.
> > >
> > >
> > > From: Andrew Morton <[email protected]>
> > >
> > > In file included from include/asm/pgtable.h:1087,
> > > from include/linux/mm.h:39,
> > > from arch/s390/mm/hugetlbpage.c:8:
> > > include/asm-generic/pgtable.h: In function '__ptep_modify_prot_start':
> > > include/asm-generic/pgtable.h:209: error: dereferencing pointer to incomplete type
> > >
> >
> > We can't turn them into macros because we're expecting to be able to
> > take the address of __ptep_modify_prot_start/commit. What type is not
> > defined on s390 at that point? Would simply adding an extra include to
> > arch/s390/mm/hugetlbpage.c fix the problem?

We need struct task_struct... I added an include <linux/sched.h> to
asm-s390/pgtable.h and it seems to fix the build problem.
I expected include dependency hell...

> > In the worst case we could push __ptep_modify_proc_start/commit out of
> > line somewhere appropriate, but that's a bit sad given how simple they are.
>
> I think the s390 guys just fixed the original build error.

Not this one, but if it works I'm going to push that to git-s390.
Currently wading through the s390 build bugs in linux-next and trying to
fix them all :/

2008-07-03 19:32:27

by Valdis Klētnieks

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 03 Jul 2008 19:56:02 BST, David Woodhouse said:

> They had to 'make oldconfig' and then actually _choose_ to say 'no' to
> an option which is fairly clearly documented, that they are the
> relatively unusual position of wanting to have said 'yes' to. You're
> getting into Aunt Tillie territory, when you complain about that.

Note that some of us chose 'no' because we *thought* that we already *had*
everything in /lib/firmware that we needed (in my case, the iwl3945 wireless
firmware and the Intel cpu microcode). The first that I realized that
the tg3 *had* firmware was when I saw the failure message, because before
that, the binary blob was inside the kernel. And then, it wasn't trivially
obvious how to get firmware loaded if the tg3 driver was builtin rather
than a module.

And based on some of the other people who apparently got bit by this same
exact behavior change on this same exact "builtin but no firmware in kernel"
config with this same exact driver, it's obvious that one of two things is true:

1) Several of the highest-up maintainers are Aunt Tillies.
or
2) This is sufficiently subtle and complicated that far more experienced
people than Aunt Tillie will Get It Very Wrong.

Attachments:

(No filename) (226.00 B)

2008-07-03 19:49:26

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 2008-07-03 at 15:31 -0400, [email protected] wrote:
> On Thu, 03 Jul 2008 19:56:02 BST, David Woodhouse said:
>
> > They had to 'make oldconfig' and then actually _choose_ to say 'no' to
> > an option which is fairly clearly documented, that they are the
> > relatively unusual position of wanting to have said 'yes' to. You're
> > getting into Aunt Tillie territory, when you complain about that.
>
> Note that some of us chose 'no' because we *thought* that we already *had*
> everything in /lib/firmware that we needed (in my case, the iwl3945 wireless
> firmware and the Intel cpu microcode). The first that I realized that
> the tg3 *had* firmware was when I saw the failure message, because before
> that, the binary blob was inside the kernel. And then, it wasn't trivially
> obvious how to get firmware loaded if the tg3 driver was builtin rather
> than a module.
>
> And based on some of the other people who apparently got bit by this same
> exact behavior change on this same exact "builtin but no firmware in kernel"
> config with this same exact driver, it's obvious that one of two things is true:
>
> 1) Several of the highest-up maintainers are Aunt Tillies.
> or
> 2) This is sufficiently subtle and complicated that far more experienced
> people than Aunt Tillie will Get It Very Wrong.

Not really. It's just a transitional thing. As you said, you know
perfectly well that modern Linux drivers like iwl3945 handle their
firmware separately through request_firmware() rather than including it
in unswappable memory in the static kernel. We're just updating some of
the older drivers to match.

I've often managed to configure a kernel which doesn't boot, when I've
updated and not paid attention to the questions which 'oldconfig' asks
me. It's fairly easy to do. But I don't advocate that 'allyesconfig'
should be the default, just in case someone needs one of the options...

But as I said, I'm content to let Linus make that decision. In the
meantime, I'd prefer to get back to the simple business of updating
drivers to use request_firmware() as they should, and have maintainers
actually respond to the _patches_ rather than refusing to even look at
the code changes because they disagree with the default setting for the
CONFIG_FIRMWARE_IN_KERNEL option.

--
dwmw2

2008-07-03 20:34:41

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Jeff Garzik <[email protected]>
Date: Thu, 03 Jul 2008 09:11:09 -0400

> dwmw2 has been told repeatedly that his changes will cause PRECISELY
> these problems, but he refuses to take the simple steps necessary to
> ensure people can continue to boot their kernels after his changes go in.
>
> Presently his tg3 changes have been nak'd, in part, because of this
> obviously, forseeable, work-around-able breakage.

I agree with Jeff, obviously.

We both saw this song and dance coming. Now the reports are coming in
from confused people who are losing their network. It is no surprise.

And the person who introduced this swath of regressions acts like it's
some kind of chore to enforce the obviously correct default behavior.

Why is it such a big deal to make "obviously working" the default?

In effect, you lied to us, in that you said that by default users
wouldn't have to do anything to keep getting a working setup. But
that is provably not true, look at all of these reports. Are you
saying these people are idiots and don't know how to configure their
kernel? Every single one of them?

So don't be surprised how pissed off some of us are about these
changes. You are inflicting pain on driver maintainers because now
they have to sift through these "firmware not found" reports in
addition to their normal workload.

And David make it seem like it's inconvenient for him to implement the
correct default, which in particular pisses me personally off the
most. It's totally irresponsible, and I don't care what the legal or
ideological motivation is.

Given that, how in the world can you be surprised that the effected
driver maintainers have no interest in reviewing the substance of
these patches? You don't piss people off, then say "help me review
this stuff." It doesn't work like that.

2008-07-03 20:50:50

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thursday, 3 of July 2008, David Woodhouse wrote:
> On Thu, 2008-07-03 at 15:31 -0400, [email protected] wrote:
> > On Thu, 03 Jul 2008 19:56:02 BST, David Woodhouse said:
> >
> > > They had to 'make oldconfig' and then actually _choose_ to say 'no' to
> > > an option which is fairly clearly documented, that they are the
> > > relatively unusual position of wanting to have said 'yes' to. You're
> > > getting into Aunt Tillie territory, when you complain about that.
> >
> > Note that some of us chose 'no' because we *thought* that we already *had*
> > everything in /lib/firmware that we needed (in my case, the iwl3945 wireless
> > firmware and the Intel cpu microcode). The first that I realized that
> > the tg3 *had* firmware was when I saw the failure message, because before
> > that, the binary blob was inside the kernel. And then, it wasn't trivially
> > obvious how to get firmware loaded if the tg3 driver was builtin rather
> > than a module.
> >
> > And based on some of the other people who apparently got bit by this same
> > exact behavior change on this same exact "builtin but no firmware in kernel"
> > config with this same exact driver, it's obvious that one of two things is true:
> >
> > 1) Several of the highest-up maintainers are Aunt Tillies.
> > or
> > 2) This is sufficiently subtle and complicated that far more experienced
> > people than Aunt Tillie will Get It Very Wrong.
>
> Not really. It's just a transitional thing. As you said, you know
> perfectly well that modern Linux drivers like iwl3945 handle their
> firmware separately through request_firmware() rather than including it
> in unswappable memory in the static kernel. We're just updating some of
> the older drivers to match.
>
> I've often managed to configure a kernel which doesn't boot, when I've
> updated and not paid attention to the questions which 'oldconfig' asks
> me. It's fairly easy to do. But I don't advocate that 'allyesconfig'
> should be the default, just in case someone needs one of the options...
>
> But as I said, I'm content to let Linus make that decision. In the
> meantime, I'd prefer to get back to the simple business of updating
> drivers to use request_firmware() as they should, and have maintainers
> actually respond to the _patches_ rather than refusing to even look at
> the code changes because they disagree with the default setting for the
> CONFIG_FIRMWARE_IN_KERNEL option.

Hm, well, but if the driver in question is in a module, then whether or not
this option is set really doesn't matter, does it?

Rafael

2008-07-03 20:55:19

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 2008-07-03 at 13:34 -0700, David Miller wrote:
> From: Jeff Garzik <[email protected]>
> Date: Thu, 03 Jul 2008 09:11:09 -0400
>
> > dwmw2 has been told repeatedly that his changes will cause PRECISELY
> > these problems, but he refuses to take the simple steps necessary to
> > ensure people can continue to boot their kernels after his changes go in.
> >
> > Presently his tg3 changes have been nak'd, in part, because of this
> > obviously, forseeable, work-around-able breakage.
>
> I agree with Jeff, obviously.
>
> We both saw this song and dance coming. Now the reports are coming in
> from confused people who are losing their network. It is no surprise.
>
> And the person who introduced this swath of regressions acts like it's
> some kind of chore to enforce the obviously correct default behavior.
>
> Why is it such a big deal to make "obviously working" the default?

> In effect, you lied to us, in that you said that by default users
> wouldn't have to do anything to keep getting a working setup.

No, I didn't lie to you. The conversation, in case you forgot, went like
this...

On Wed, 2008-06-18 at 16:23 -0700, David Miller wrote:
> On Thu, 2008-06-19 at 00:16 +0100, David Woodhouse wrote:
> > On Wed, 2008-06-18 at 16:05 -0700, David Miller wrote:
> > > Tell me this, how can I (with the default config option settings)
> > > netboot properly without an initial ramdisk after these tg3 patches
> > > and still get the proper firmware?
> >
> > I suppose the facetious answer is that you can't, just as you can't do
> > it with a default config _before_ these patches -- because neither
> > CONFIG_TIGON3 nor the various options you need for nfsroot are enabled
> > by default.
> >
> > But if you _have_ a working nfsroot+tg3 config and you apply these
> > patches, then all you need to do is say 'y' when you're asked if you
> > want to include the firmware for it:
> > CONFIG_TIGON3_FIRMWARE=y
> >
> > If you are competent enough to get nfsroot working, it isn't really very
> > credible to claim you lack the wit to say 'y' when asked that question,
> > surely?
> >
> > Solving that problem was step #1 in the process of converting drivers to
> > use request_firmware(). You _have_ to be able to build the firmware into
> > the kernel image, and you _can_.
>
> Fair enough.
>
> I'll step back and let you work out the objections Jeff raised.

Since then, I responded to feedback and changed the individual
CONFIG_XXX_FIRMWARE options to a single CONFIG_FIRMWARE_IN_KERNEL which
controls them all, but basically nothing has changed. What you accepted
then is still true.

On Thu, 2008-07-03 at 13:34 -0700, David Miller wrote:
So don't be surprised how pissed off some of us are about these
> changes. You are inflicting pain on driver maintainers because now
> they have to sift through these "firmware not found" reports in
> addition to their normal workload.

It also improves coverage testing, and found a real bug in the failure
path...

> And David make it seem like it's inconvenient for him to implement the
> correct default, which in particular pisses me personally off the
> most. It's totally irresponsible, and I don't care what the legal or
> ideological motivation is.

It's not inconvenient at all. It's just the _wrong_ default. But if we
really can't get past it otherwise, then let's just set it to 'y' for
now. I've committed the following, which will appear in linux-next
tomorrow.

Now, can someone _please_ give me a straight response to the allegation
that the TSO firmware on the tg3 is _optional_ anyway, and that it can
work without it? If that's true, we should fix the code path where
request_firmware() fails, so it doesn't abort the initialisation. (And
most of the whining about the driver being 'broken' is nonsense too.)

----
>From 400f1b05a9707bd181a044877ca590e87c400749 Mon Sep 17 00:00:00 2001
From: David Woodhouse <[email protected]>
Date: Thu, 3 Jul 2008 21:36:11 +0100
Subject: [PATCH] firmware: default CONFIG_FIRMWARE_IN_KERNEL=y

This is obviously the wrong thing to do in the long (or even medium)
term -- since the recommended way of handling firmware, as used
_unconditionally_ by modern drivers, is to rely on request_firmware()
being satisfied from userspace rather than keeping the firmware in
unswappable static kernel memory.

But this change preserves the property, for now, that the fixes to make
older drivers use request_firmware() introduce _no_ "regressions" when
Aunt Tillie runs 'make oldconfig' and accepts the defaults without
looking at what she's doing.

Signed-off-by: David Woodhouse <[email protected]>
---
drivers/base/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 339c148..d47482f 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -37,6 +37,7 @@ config FW_LOADER
config FIRMWARE_IN_KERNEL
bool "Include in-kernel firmware blobs in kernel binary"
depends on FW_LOADER
+ default y
help
The kernel source tree includes a number of firmware 'blobs'
which are used by various drivers. The recommended way to
--
1.5.5.1

--
dwmw2

2008-07-03 21:03:48

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> Although it does make me wonder if it was better the way I had it
> originally, with individual options like TIGON3_FIRMWARE_IN_KERNEL
> attached to each driver, rather than a single FIRMWARE_IN_KERNEL option
> which controls them all.

IMO, individual options would be better.

Plus, unless I am misunderstanding, the firmware is getting built into
the kernel image not the tg3 module?

If that is the case, then that creates problems by not moving with the
driver.

If that is not the case, all good.

Jeff

2008-07-03 21:33:59

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Jeff Garzik wrote:
> David Woodhouse wrote:
>> Although it does make me wonder if it was better the way I had it
>> originally, with individual options like TIGON3_FIRMWARE_IN_KERNEL
>> attached to each driver, rather than a single FIRMWARE_IN_KERNEL option
>> which controls them all.
>
> IMO, individual options would be better.

They had individual options for a long time, but the consensus was that
I should remove them -- a consensus which was probably right. It was
moderately inconvenient going back through it all and recommitting it
without that, but it was worth it to get it right...

> Plus, unless I am misunderstanding, the firmware is getting built into
> the kernel image not the tg3 module?

That's right, although it doesn't really matter when they're both in the
vmlinux.

When it's actually a module, there really is no good reason not to let
request_firmware() get satisfied from userspace. If you can load
modules, then you can load firmware too -- the required udev stuff has
been there as standard for a _long_ time, as most modern drivers
_require_ it without even giving you the built-in-firmware option at all.

It makes no more sense to object to that than it does to object to the
module depending on _other_ modules. Both those other modules, and the
required firmware, are _installed_ by the kernel Makefiles, after all.

It wouldn't be _impossible_ to put firmware blobs into the foo.ko files
themselves and find them there. The firmware blobs in the kernel are
done in a separate section (like initcalls, exceptions tables, pci
fixups, and a bunch of other stuff). It'd just take some work in
module.c to link them into a global list, and some locking shenanigans
in the lookups (and lifetime issues to think about). But it just isn't
worth the added complexity, given that userspace is known to be alive
and working. It's pointless not to just use request_firmware() normally,
from a module.

--
dwmw2

2008-07-03 21:42:22

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thursday, 3 of July 2008, David Woodhouse wrote:
> Jeff Garzik wrote:
> > David Woodhouse wrote:
> >> Although it does make me wonder if it was better the way I had it
> >> originally, with individual options like TIGON3_FIRMWARE_IN_KERNEL
> >> attached to each driver, rather than a single FIRMWARE_IN_KERNEL option
> >> which controls them all.
> >
> > IMO, individual options would be better.
>
> They had individual options for a long time, but the consensus was that
> I should remove them -- a consensus which was probably right. It was
> moderately inconvenient going back through it all and recommitting it
> without that, but it was worth it to get it right...
>
> > Plus, unless I am misunderstanding, the firmware is getting built into
> > the kernel image not the tg3 module?
>
> That's right, although it doesn't really matter when they're both in the
> vmlinux.
>
> When it's actually a module, there really is no good reason not to let
> request_firmware() get satisfied from userspace. If you can load
> modules, then you can load firmware too -- the required udev stuff has
> been there as standard for a _long_ time, as most modern drivers
> _require_ it without even giving you the built-in-firmware option at all.
>
> It makes no more sense to object to that than it does to object to the
> module depending on _other_ modules. Both those other modules, and the
> required firmware, are _installed_ by the kernel Makefiles, after all.
>
> It wouldn't be _impossible_ to put firmware blobs into the foo.ko files
> themselves and find them there. The firmware blobs in the kernel are
> done in a separate section (like initcalls, exceptions tables, pci
> fixups, and a bunch of other stuff). It'd just take some work in
> module.c to link them into a global list, and some locking shenanigans
> in the lookups (and lifetime issues to think about). But it just isn't
> worth the added complexity, given that userspace is known to be alive
> and working. It's pointless not to just use request_firmware() normally,
> from a module.

Still, maybe we can add some kbuild magic to build the blobs along with
their modules and to install them under /lib/firmware (by default) when the
modules are installed in /lib/modules/... ?

Rafael

2008-07-03 21:44:17

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Rafael J. Wysocki wrote:
> Still, maybe we can add some kbuild magic to build the blobs along with
> their modules and to install them under /lib/firmware (by default) when the
> modules are installed in /lib/modules/... ?

Something like appending this to Makefile?

firmware_and_modules_install: firmware_install modules_install

(I'm still wondering if we should make 'firmware_install' install to
/lib/firmware by default, instead of into the build tree as
'headers_install' does. The Aunt Tillie answer would definitely be
'yes', although that means it requires root privs; like modules_install
does.)

--
dwmw2

2008-07-03 21:51:04

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thursday, 3 of July 2008, David Woodhouse wrote:
> Rafael J. Wysocki wrote:
> > Still, maybe we can add some kbuild magic to build the blobs along with
> > their modules and to install them under /lib/firmware (by default) when the
> > modules are installed in /lib/modules/... ?
>
> Something like appending this to Makefile?
>
> firmware_and_modules_install: firmware_install modules_install
>
> (I'm still wondering if we should make 'firmware_install' install to
> /lib/firmware by default, instead of into the build tree as
> 'headers_install' does. The Aunt Tillie answer would definitely be
> 'yes', although that means it requires root privs; like modules_install
> does.)

I would prefer 'make firmware_install' to just copy the blobs into specific
location in analogy with 'make modules_install', so that you can build the
blobs as a normal user (for example, on an NFS server) and then put them
into the right place as root (for example, on an NFS client that has no write
privilege on the server).

2008-07-03 21:55:20

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Rafael J. Wysocki wrote:
> On Thursday, 3 of July 2008, David Woodhouse wrote:
>> Rafael J. Wysocki wrote:
>>> Still, maybe we can add some kbuild magic to build the blobs along with
>>> their modules and to install them under /lib/firmware (by default) when the
>>> modules are installed in /lib/modules/... ?
>> Something like appending this to Makefile?
>>
>> firmware_and_modules_install: firmware_install modules_install
>>
>> (I'm still wondering if we should make 'firmware_install' install to
>> /lib/firmware by default, instead of into the build tree as
>> 'headers_install' does. The Aunt Tillie answer would definitely be
>> 'yes', although that means it requires root privs; like modules_install
>> does.)
>
> I would prefer 'make firmware_install' to just copy the blobs into specific
> location in analogy with 'make modules_install', so that you can build the
> blobs as a normal user (for example, on an NFS server) and then put them
> into the right place as root (for example, on an NFS client that has no write
> privilege on the server).

Not entirely sure which you mean. You _can't_ run 'make modules_install'
as a normal user, unless you override $(INSTALL_MOD_PATH) on the command
line.

Do you want 'make firmware_install' to be the same? It isn't at the
moment -- it installs to a subdirectory of the kernel build tree, like
'make headers_install' does. But I'm not sure which is better.

--
dwmw2

2008-07-03 22:22:51

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> Jeff Garzik wrote:
>> David Woodhouse wrote:
>>> Although it does make me wonder if it was better the way I had it
>>> originally, with individual options like TIGON3_FIRMWARE_IN_KERNEL
>>> attached to each driver, rather than a single FIRMWARE_IN_KERNEL option
>>> which controls them all.
>>
>> IMO, individual options would be better.
>
> They had individual options for a long time, but the consensus was that
> I should remove them -- a consensus which was probably right. It was
> moderately inconvenient going back through it all and recommitting it
> without that, but it was worth it to get it right...
>
>> Plus, unless I am misunderstanding, the firmware is getting built into
>> the kernel image not the tg3 module?
>
> That's right, although it doesn't really matter when they're both in the
> vmlinux.
>
> When it's actually a module, there really is no good reason not to let
> request_firmware() get satisfied from userspace. If you can load
> modules, then you can load firmware too -- the required udev stuff has
> been there as standard for a _long_ time, as most modern drivers
> _require_ it without even giving you the built-in-firmware option at all.
>
> It makes no more sense to object to that than it does to object to the
> module depending on _other_ modules. Both those other modules, and the
> required firmware, are _installed_ by the kernel Makefiles, after all.
>
> It wouldn't be _impossible_ to put firmware blobs into the foo.ko files
> themselves and find them there. The firmware blobs in the kernel are
> done in a separate section (like initcalls, exceptions tables, pci
> fixups, and a bunch of other stuff). It'd just take some work in
> module.c to link them into a global list, and some locking shenanigans
> in the lookups (and lifetime issues to think about). But it just isn't
> worth the added complexity, given that userspace is known to be alive
> and working. It's pointless not to just use request_firmware() normally,
> from a module.

It is pointless -- if you assume everybody wants to run their distro and
universe your way.

If a firmware is built-in, then 'make firmware_install' is clearly
optional and may be omitted. That invalidates several of your
assumptions above.

Further, all current kernel build and test etc. scripts are unaware of
'make firmware_install', and it is unfair to everybody to force a
flag-day build process change on people, just to keep their drivers in
the same working state today as it was yesterday.

Conclusion - kernel build process must produce a working driver after
your changes are applied, even in absence of 'make firmware_install'.

That does not, repeat /not/ exclude the desired end goal of course --
separating the firmware from the driver, installing in /lib/firmware via
'make firmware_install', etc.

I support your end goal, I really do. But I continue to feel there is a
lack of regard for breakage and regressions. You are either ignoring or
apparently just not seeing
* how many new ways this can produce a non-working driver
* how important it is in this specific case to fail-safe,
and avoid a broken driver at all costs.

As a concrete example, in the above quoted text you assume that a user
will never copy kernel modules around. I can tell you that, with tg3.ko
being nice and self-contained, yes it does get copied around (to
multiple machines, etc.). With the firmware newly separated from
tg3.ko, you have introduced breakage for any user that is unaware of
this new requirement (kernel module == requires additional file now).

Scripts that build install disks must be updated, otherwise a script
that builds a boot image will include the drivers it knows about, but
/not/ include the crucial firmware.

None of this stuff is "pointless", none of this stuff may be dismissed
as "making no sense". All these are real world examples where users
FOLLOWING THEIR NORMAL, PROSCRIBED KERNEL PROCESSES will produce
non-working drivers.

The only valid assumption here is to assume that the user is /unaware/
of these new steps they must take in order to continue to have a working
system.

Regards,

Jeff

2008-07-03 22:25:54

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thursday, 3 of July 2008, David Woodhouse wrote:
> Rafael J. Wysocki wrote:
> > On Thursday, 3 of July 2008, David Woodhouse wrote:
> >> Rafael J. Wysocki wrote:
> >>> Still, maybe we can add some kbuild magic to build the blobs along with
> >>> their modules and to install them under /lib/firmware (by default) when the
> >>> modules are installed in /lib/modules/... ?
> >> Something like appending this to Makefile?
> >>
> >> firmware_and_modules_install: firmware_install modules_install
> >>
> >> (I'm still wondering if we should make 'firmware_install' install to
> >> /lib/firmware by default, instead of into the build tree as
> >> 'headers_install' does. The Aunt Tillie answer would definitely be
> >> 'yes', although that means it requires root privs; like modules_install
> >> does.)
> >
> > I would prefer 'make firmware_install' to just copy the blobs into specific
> > location in analogy with 'make modules_install', so that you can build the
> > blobs as a normal user (for example, on an NFS server) and then put them
> > into the right place as root (for example, on an NFS client that has no write
> > privilege on the server).
>
> Not entirely sure which you mean. You _can't_ run 'make modules_install'
> as a normal user, unless you override $(INSTALL_MOD_PATH) on the command
> line.

Yes, I know that.

> Do you want 'make firmware_install' to be the same?

Yes, I'd prefer it to behave in the same way as 'make modules_install'.

I'd use a config option like BUILD_FIRMWARE_BLOBS that, if set, would make
the build system create firmware bin files in the same directories where the
driver's .o files are located. Then, 'make firmware_install' would only copy
those bin files to wherever was appropriate (eg. /lib/firmware/).

Of course, there still would be a problem if there already were such firmware
files at the destination, but that would have to be resolved anyway by the user
wanting to install the new kernel along with the new firmware blobs.

> It isn't at the moment -- it installs to a subdirectory of the kernel build tree, like
> 'make headers_install' does. But I'm not sure which is better.

IMO 'make headers_install' is used for a different purpose. You don't have to
run it to be able to use the kernel in a usual way.

OTOH, everyone is familiar with the 'make modules_install' mechanics and it
seems natural to use analogous mechanics for firmware blobs.

2008-07-03 22:44:35

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

O
> Further, all current kernel build and test etc. scripts are unaware of
> 'make firmware_install', and it is unfair to everybody to force a
> flag-day build process change on people, just to keep their drivers in
> the same working state today as it was yesterday.

IMHO we want firmware built in as the default for the moment. If the
firmware model makes sense (as I think it does) then the distributions
will catch up, turn it on and sort out the default behaviour - exactly as
they did all those years ago with modules, more recently with "use an
initrd" and so on.

> as "making no sense". All these are real world examples where users
> FOLLOWING THEIR NORMAL, PROSCRIBED KERNEL PROCESSES will produce

I hope you mean "prescribed" ;)

> The only valid assumption here is to assume that the user is /unaware/
> of these new steps they must take in order to continue to have a working
> system.

To a large extent not the user but their distro - consider "make install"

2008-07-03 23:14:57

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
>> Further, all current kernel build and test etc. scripts are unaware of
>> 'make firmware_install', and it is unfair to everybody to force a
>> flag-day build process change on people, just to keep their drivers in
>> the same working state today as it was yesterday.
>
> IMHO we want firmware built in as the default for the moment. If the
> firmware model makes sense (as I think it does) then the distributions
> will catch up, turn it on and sort out the default behaviour - exactly as
> they did all those years ago with modules, more recently with "use an
> initrd" and so on.

Agreed.

>> as "making no sense". All these are real world examples where users
>> FOLLOWING THEIR NORMAL, PROSCRIBED KERNEL PROCESSES will produce
>
> I hope you mean "prescribed" ;)

heh, *cough* yes

>> The only valid assumption here is to assume that the user is /unaware/
>> of these new steps they must take in order to continue to have a working
>> system.
>
> To a large extent not the user but their distro - consider "make install"

Actually, I was tossing that about in my head:

Is it a better idea to eliminate 'make firmware_install' completely, and
instead implement it silently via 'make install'?

'make install' is already a big fat distro hook...

Jeff

2008-07-03 23:20:23

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> Actually, I was tossing that about in my head:
>
> Is it a better idea to eliminate 'make firmware_install' completely, and
> instead implement it silently via 'make install'?
>
> 'make install' is already a big fat distro hook...

make firmware_install can encapsulate a lot of kernel specific knowledge
so I think it belongs in the kernel tree to avoid problems in future. The
use of make firmware_install belongs in the distro make install hooks.

Otherwise we will mess up the distro stuff if we have to change the
innards of make firmware_install in future, as may well occur.

2008-07-03 23:21:34

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: David Woodhouse <[email protected]>
Date: Thu, 03 Jul 2008 19:56:02 +0100

> It's wrong to change the CONFIG_FIRMWARE_IN_KERNEL default to 'Y',
> because the _normal_ setting for that option _really_ should be 'N'.

On what basis? From a "obviously works" basis, the default should be
'y'.

> What we're doing now is just cleaning up the older drivers which don't
> use request_firmware(), to conform to what is now common practice.

You say "conform" I say "break".

> In the meantime, it would be useful if Jeff would quit throwing his toys
> out of the pram on that issue and actually review the _code_ changes. In
> particular, are the reports correct that the device operates just fine
> without the TSO firmware loaded? Should we change the request_firmware()
> error path to just disable TSO and continue with the initialisation?

No!

The 5701 A0 firmware is necessary to load in order to work around
hardware and existing firmware bugs on those cards. It's an issue of
basic functionality, not just optimizations.

5701 A0 tg3 chips cannot operate at all without the firmware being
present in the driver.

Therefore, if you can't load the firmware, the card is not going to
work.

> Less of the ad hominem, please. Especially when it's so misdirected.

No, it is properly directed, you are breaking the tree for users.

> Updating these drivers to remove large blobs of static unswappable data
> from the kernel, and having it provided from userspace on demand as
> modern Linux drivers do, is a perfectly sensible technical goal all on
> its own.

I disagree.

> And given the GPL's explicit provisions with regard to collective works
> there are also entirely reasonable, non-"fundamentalist" grounds for
> believing that it _may_ pose a licensing problem, and for wanting to err
> on the side of caution in that respect too.

So now the real truth is revealed. You have no technical basis for
this stuff you are ramming down everyone's throats.

You want to choose a default based upon your legal agenda.

That explains all of the bullshit that is attached to your work, and
all of the bullshit arguments you make wrt. choosing defaults that
break things for users.

It's all about agendas rather than any real technical objectives.

If it was purely technical, you wouldn't be choosing defaults that
break things for users by default. Jeff and I warned you about this
from day one, you did not listen, and now we have at least 10 reports
just today of people with broken networking.

2008-07-03 23:36:14

[permalink] [raw]

Subject: Re: 2.6.26-rc8-mm1--No e100 :( logs say missing formware

On Thu, 3 Jul 2008 02:02:36 -0700, Andrew Morton <[email protected]> wrote:

>ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.26-rc8/2.6.26-rc8-mm1/

Hi, it booted up on a Core2Duo box but failed to connect via e100 NIC
to localnet.

/var/log/messages:

Jul 4 09:14:13 pooh kernel: firmware: requesting e100/d102e_ucode.bin
Jul 4 09:14:13 pooh firmware.sh[1666]: Cannot find firmware file 'e100/d102e_ucode.bin'

/var/log/syslog:

Jul 4 09:14:13 pooh kernel: e100: eth0: e100_request_firmware: Failed to load firmware "e100/d1
02e_ucode.bin": -2
Jul 4 09:14:13 pooh kernel: e100: eth0: e100_request_firmware: Failed to load firmware "e100/d1
02e_ucode.bin": -2
Jul 4 09:17:17 pooh kernel: e100: eth0: e100_request_firmware: Failed to load firmware "e100/d1
02e_ucode.bin": -2
Jul 4 09:17:30 pooh last message repeated 3 times

So where did the firmware go? -- I been using these e100 NICs for years,
2.4 & 2.6 kernels, never seen this kind of failure...

Thanks,
Grant.

2008-07-03 23:43:20

[permalink] [raw]

Subject: Re: 2.6.26-rc8-mm1--No e100 :( logs say missing firmware

2008-07-04 00:20:32

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, Jul 03, 2008 at 04:21:20PM -0700, David Miller wrote:
> > And given the GPL's explicit provisions with regard to collective works
> > there are also entirely reasonable, non-"fundamentalist" grounds for
> > believing that it _may_ pose a licensing problem, and for wanting to err
> > on the side of caution in that respect too.
>
> So now the real truth is revealed. You have no technical basis for
> this stuff you are ramming down everyone's throats.
>
> You want to choose a default based upon your legal agenda.

Yep, legal agenda. As I suspected, licensing religious fundamentalism. :-)

People who care can change the defaults. People who are real
religious nuts won't even let the firmware live in the same source
tarball. But I hope you agree we clearly don't have consensus to take
*that* step (rip out all firmware and make a whole bunch of drivers
non-functional and forcing users to go on a treasure-hunt to find some
new tarball they have to install on their existing system). So given
that we're not ready to take that step, why not just leave the default
as "yes" for now?

The staged approach means that if you really want to do this ASAP,
then start assembling the firmware tarball *now*, and for a while
(read: at least 9-18 months) we can distribute firmware both in the
kernel source tarball as well as separately in the
licensing-religion-firmware tarball. See if you can get distros
willing to ship it by default in most user's systems, and give people
plenty of time to understand that we are trying to decouple firmware
from the kernel sources. If we need to institute better versioning
regimes between the drivers and firmware release levels, that will
also give people a chance to get that all right. Then 6-9 months
later, we can turn the default to 'no', and then maybe another 6-9
months after that, we can talk about removing the firmware modules.
But it seems to me that you are skipping a few steps by arguing that
the default should be changed here-and-now.

We've been shipping firmware in the kernel for over a ***decade***; in
fact, probably over 15 years. For people who are legal freaks/geeks,
look up the legal terms "Estoppel" and "Laches". That provides a
fairly large amount of protection right there. For people who aren't
legal geeks, we've been doing this for well over a decade; another
year or two really isn't a big deal. It certainly doesn't justify
breaking users by default just to try to hurry up this process.

> If it was purely technical, you wouldn't be choosing defaults that
> break things for users by default. Jeff and I warned you about this
> from day one, you did not listen, and now we have at least 10 reports
> just today of people with broken networking.

Not 15 minutes after David posted his note, we're now up to 11
reports; and this is only from an -mm patch series. Can you imagine
the number of bug reports if this were allowed to ship in a mainline
kernel.org release? One good thing is that we can definitely show
that there people that are downloading, compiling and trying to build
the -mm kernel. :-)

- Ted

2008-07-04 00:25:49

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Miller wrote:
> From: David Woodhouse <[email protected]>
> Date: Thu, 03 Jul 2008 19:56:02 +0100
>
>> It's wrong to change the CONFIG_FIRMWARE_IN_KERNEL default to 'Y',
>> because the _normal_ setting for that option _really_ should be 'N'.
>
> On what basis? From a "obviously works" basis, the default should be
> 'y'.

I already changed it to 'y'.

>> What we're doing now is just cleaning up the older drivers which don't
>> use request_firmware(), to conform to what is now common practice.
>
> You say "conform" I say "break".

You mean...
"What we're doing now is just cleaning up the older drivers
which don't use request_firmware(), to break to what is now
common practice."
?

Doesn't really scan, does it?

Common practice in modern Linux drivers is to use request_firmware().
I'm just going through and fixing up the older ones to do that too.

(After making it possible to build that firmware _into_ the kernel so
that we aren't forcing people to use an initrd where they didn't before,
of course.)

The word for that is definitely 'conform'. I know you don't _like_ the
modern accepted practice, but that's _your_ windmill to tilt at. I have
my own :)

>> In the meantime, it would be useful if Jeff would quit throwing his toys
>> out of the pram on that issue and actually review the _code_ changes. In
>> particular, are the reports correct that the device operates just fine
>> without the TSO firmware loaded? Should we change the request_firmware()
>> error path to just disable TSO and continue with the initialisation?
>
> No!
>
> The 5701 A0 firmware is necessary to load in order to work around
> hardware and existing firmware bugs on those cards. It's an issue of
> basic functionality, not just optimizations.
>
> 5701 A0 tg3 chips cannot operate at all without the firmware being
> present in the driver.
>
> Therefore, if you can't load the firmware, the card is not going to
> work.

Neat avoidance of question there... it was fairly clear that the 5701_A0
firmware was going to be mandatory; I was asking about the TSO firmware.

Does anyone _else_ actually want to give a straight answer to a simple
question? Someone who wouldn't have to follow it with an apology because
of all their shouting about 'breakage' when the firmware in question is
actually optional anyway, perhaps?

> If it was purely technical, you wouldn't be choosing defaults that
> break things for users by default.

Actually, the beauty of Linux is that we _can_ change things where a
minor short-term inconvenience leads to a better situation in the long term.

> Jeff and I warned you about this from day one, you did not listen, and
> now we have at least 10 reports just today of people with broken
> networking.

Out of interest... of those, what proportion would be 'fixed' if they'd
just paid attention when running 'make oldconfig', which is now
addressed because I've changed the FIRMWARE_IN_KERNEL default to 'y'?

And how many would be 'fixed' if someone had given me a straight answer
when I asked about the TSO firmware, and that failure path no longer
aborted the driver initialisation but instead just fell back to non-TSO?

I'll look at making the requirement for 'make firmware_install' more
obvious, or even making it happen automatically as part of
'modules_install'.

--
dwmw2

2008-07-04 00:41:34

[permalink] [raw]

Subject: Re: 2.6.26-rc8-mm1--No e100 :( logs say missing formware

On Fri, 04 Jul 2008 09:35:43 +1000 Grant Coady <[email protected]> wrote:

> So where did the firmware go?

Dave ate it. Please see the thread "[bug?] tg3: Failed to load
firmware "tigon/tg3_tso.bin""

2008-07-04 01:10:01

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Theodore Tso wrote:
> On Thu, Jul 03, 2008 at 04:21:20PM -0700, David Miller wrote:
>> You want to choose a default based upon your legal agenda.
>
> Yep, legal agenda. As I suspected, licensing religious fundamentalism. :-)

You are mistaken, and you're responding some words that someone _else_
put in my mouth.

> The staged approach means that if you really want to do this ASAP,
> then start assembling the firmware tarball *now*,

That's easy enough. We can automatically generate a tree _from_ Linus'
tree, with a scripted transform so that it includes only the firmware
files (much like the kernel-headers tree automatically follows each
commit in Linus' tree, but includes only the exported headers).

And there are some hardware manufacturers who are willing to have their
firmware included in such a firmware tarball, but who will _not_ give
their permission to have it included in the Linux kernel because of the
legal concerns you're so dismissive of. But that's OK -- we can pull
from the automatically generated tree into a 'real' linux-firmware.git
tree, which includes those extra firmware files.

But there's no need to do it _now_. It can wait until the basic stuff is
in Linus' tree and it can automatically derive from that. There's no
particular rush, is there?

> and for a while (read: at least 9-18 months) we can distribute firmware
> both in the kernel source tarball as well as separately

That makes a certain amount of sense.

> in the licensing-religion-firmware tarball.

Please don't be gratuitously offensive, Ted. It's not polite, and it's
not a particularly good debating technique either. I expect better from you.

> See if you can get distros willing to ship it by default in most
> user's systems, and give people plenty of time to understand that we
> are trying to decouple firmware from the kernel sources.

The distros are certainly willing (and keen) to do it. The Fedora
Engineering Steering Committee has already stated that it wants to do
so, and the specfile changes to spit out a 'kernel-firmware' sub-package
with the kernel build are ready to go right now.

Fedora already modifies tarballs, for example 'openssh-5.0p1-noacss'. I
think it's quite likely they'd end up using a 'linux-2.6.27-nofirmware'
tarball too, and build the firmware package from the separate tree.

Some other distributions have been doing that kind of thing _already_,
even when it meant just ripping out certain drivers completely. That
seems excessive to me; I prefer not to actually _break_ anything.

> If we need to institute better versioning regimes between the drivers
> and firmware release levels, that will also give people a chance to
> get that all right. Then 6-9 months later, we can turn the default
> to 'no', and then maybe another 6-9 months after that, we can talk
> about removing the firmware modules.
> But it seems to me that you are skipping a few steps by arguing that
> the default should be changed here-and-now.

I disagree that the _default_ is such an issue -- largely because the
normal case for modern drivers is not to include the firmware _anyway_,
and the tools like 'mkinitrd' already cope with it just fine.

But I've changed the default to 'y' now, as I already said.

--
dwmw2

2008-07-04 01:29:19

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008 01:24:59 +0100, David Woodhouse <[email protected]> wrote:
...
>I'll look at making the requirement for 'make firmware_install' more
>obvious, or even making it happen automatically as part of
>'modules_install'.

I like this one: Automagically part of modules_install. No break existing
kernel build scripts for the expected-by-user build sequence?

And please put 'make firmware_install' into 'make help' if that's the way
you go.

And another point, at the moment I seem to get all sorts of odd things
built I didn't know were there?

root@pooh:/home/grant/linux/linux-2.6.26-rc8-mm1a# make firmware_install
HOSTCC firmware/ihex2fw
IHEX2FW firmware/atmsar11.fw
INSTALL usr/lib/firmware/atmsar11.fw
IHEX2FW firmware/dabusb/firmware.fw
INSTALL usr/lib/firmware/dabusb/firmware.fw
IHEX2FW firmware/emi26/loader.fw
INSTALL usr/lib/firmware/emi26/loader.fw
IHEX2FW firmware/emi26/firmware.fw
INSTALL usr/lib/firmware/emi26/firmware.fw
IHEX2FW firmware/emi26/bitstream.fw
INSTALL usr/lib/firmware/emi26/bitstream.fw
IHEX2FW firmware/emi62/loader.fw
INSTALL usr/lib/firmware/emi62/loader.fw
IHEX2FW firmware/emi62/bitstream.fw
INSTALL usr/lib/firmware/emi62/bitstream.fw
IHEX2FW firmware/emi62/spdif.fw
INSTALL usr/lib/firmware/emi62/spdif.fw
IHEX2FW firmware/emi62/midi.fw
INSTALL usr/lib/firmware/emi62/midi.fw
IHEX2FW firmware/keyspan/mpr.fw
INSTALL usr/lib/firmware/keyspan/mpr.fw
IHEX2FW firmware/keyspan/usa18x.fw
INSTALL usr/lib/firmware/keyspan/usa18x.fw
IHEX2FW firmware/keyspan/usa19.fw
INSTALL usr/lib/firmware/keyspan/usa19.fw
IHEX2FW firmware/keyspan/usa19qi.fw
INSTALL usr/lib/firmware/keyspan/usa19qi.fw
IHEX2FW firmware/keyspan/usa19qw.fw
INSTALL usr/lib/firmware/keyspan/usa19qw.fw
IHEX2FW firmware/keyspan/usa19w.fw
INSTALL usr/lib/firmware/keyspan/usa19w.fw
IHEX2FW firmware/keyspan/usa28.fw
INSTALL usr/lib/firmware/keyspan/usa28.fw
IHEX2FW firmware/keyspan/usa28xa.fw
INSTALL usr/lib/firmware/keyspan/usa28xa.fw
IHEX2FW firmware/keyspan/usa28xb.fw
INSTALL usr/lib/firmware/keyspan/usa28xb.fw
IHEX2FW firmware/keyspan/usa28x.fw
INSTALL usr/lib/firmware/keyspan/usa28x.fw
IHEX2FW firmware/keyspan/usa49w.fw
INSTALL usr/lib/firmware/keyspan/usa49w.fw
IHEX2FW firmware/keyspan/usa49wlc.fw
INSTALL usr/lib/firmware/keyspan/usa49wlc.fw
IHEX2FW firmware/whiteheat_loader.fw
INSTALL usr/lib/firmware/whiteheat_loader.fw
IHEX2FW firmware/whiteheat.fw
INSTALL usr/lib/firmware/whiteheat.fw
IHEX2FW firmware/keyspan_pda/keyspan_pda.fw
INSTALL usr/lib/firmware/keyspan_pda/keyspan_pda.fw
IHEX2FW firmware/keyspan_pda/xircom_pgs.fw
INSTALL usr/lib/firmware/keyspan_pda/xircom_pgs.fw
H16TOFW firmware/vicam/firmware.fw
INSTALL usr/lib/firmware/vicam/firmware.fw

The .config and dmesg are at:
http://bugsplatter.mine.nu/test/boxen/pooh/config-2.6.26-rc8-mm1a.gz
http://bugsplatter.mine.nu/test/boxen/pooh/dmesg-2.6.26-rc8-mm1a.gz

Grant.

2008-07-04 01:48:41

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 02:09:15AM +0100, David Woodhouse wrote:
> But there's no need to do it _now_. It can wait until the basic stuff is
> in Linus' tree and it can automatically derive from that. There's no
> particular rush, is there?

The only rush if the main agenda is to extirpate all firmware from the
mainline kernel sources. I don't think we can start the timer for
doing that until the firmware tarball is created and it starts being
used as the default way of delivering firmware for what you call
"legacy" drivers. If there's no particular rush to finally rm the
firmware for trivers such as tg3 from the source tree (and I
personally don't think there should be any rush), or any rush to
change the default for CONFIG_FIRMWARE_IN_KERNEL to "no", then I don't
see any rush in creating the firmware tarball.

If *you* think there is a rush in making CONFIG_FIRMWARE_IN_KERNEL
default to "no", then you might want to decide to create the firmware
tarball sooner, and get distro's everywhere to start using it, and to
get everyone to understand that they should start including it in
their systems. (Remember, not everyone uses the popular distributions
like Fedora, Debian, Ubuntu, Open SuSE, etc.)

But heck, that's up to you. :-)

>> and for a while (read: at least 9-18 months) we can distribute firmware
> > both in the kernel source tarball as well as separately
>
> That makes a certain amount of sense.

Glad we agree.

>> in the licensing-religion-firmware tarball.
>
> Please don't be gratuitously offensive, Ted. It's not polite, and it's
> not a particularly good debating technique either. I expect better from
> you.

Well, I think it's offensive to break users who have happily been
using drivers that have been including firmware for a long, long, LONG
time, and I expected better of you.

So there. :-)

- Ted

2008-07-04 02:32:53

by Mikael Pettersson

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox writes:
> > The only valid assumption here is to assume that the user is /unaware/
> > of these new steps they must take in order to continue to have a working
> > system.
>
> To a large extent not the user but their distro - consider "make install"
> --

Last time I checked only x86 had 'make install'. I regularly build
natively on ppc(32|64) and sparc64, and none of them implement
'make install' AFAIK. And on ARM I move the kernels over to a tftp
server for network boots, again w/o 'make install'.

Not that 'make install' is difficult. All it does it hand over to
/sbin/installkernel or something like that.

In the context of .config changes, 'make oldconfig' with 'select the
default' must IMO result in a working kernel similar to the previous
one. Anything else is madness or arrogance.

2008-07-04 02:42:21

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 4 Jul 2008, David Woodhouse wrote:

> David Miller wrote:
>> From: David Woodhouse <[email protected]>
>> Date: Thu, 03 Jul 2008 19:56:02 +0100
>>
>>> It's wrong to change the CONFIG_FIRMWARE_IN_KERNEL default to 'Y',
>>> because the _normal_ setting for that option _really_ should be 'N'.
>>
>> On what basis? From a "obviously works" basis, the default should be
>> 'y'.
>
> I already changed it to 'y'.
>
>>> What we're doing now is just cleaning up the older drivers which don't
>>> use request_firmware(), to conform to what is now common practice.
>>
>> You say "conform" I say "break".
>
> You mean...
> "What we're doing now is just cleaning up the older drivers
> which don't use request_firmware(), to break to what is now
> common practice."
> ?
>
> Doesn't really scan, does it?
>
> Common practice in modern Linux drivers is to use request_firmware(). I'm
> just going through and fixing up the older ones to do that too.
>
> (After making it possible to build that firmware _into_ the kernel so that we
> aren't forcing people to use an initrd where they didn't before, of course.)

has this taken place yet? (and if so, what kernel version first included
this fix)

>> If it was purely technical, you wouldn't be choosing defaults that
>> break things for users by default.
>
> Actually, the beauty of Linux is that we _can_ change things where a minor
> short-term inconvenience leads to a better situation in the long term.

but doing so should not be a easy and quick decision, and it needs to be
made very clear exactly what breakage is going to take place and why
(along with the explination of why the breakage couldn't be avoided)

>> Jeff and I warned you about this from day one, you did not listen, and
>> now we have at least 10 reports just today of people with broken
>> networking.
>
> Out of interest... of those, what proportion would be 'fixed' if they'd just
> paid attention when running 'make oldconfig', which is now addressed because
> I've changed the FIRMWARE_IN_KERNEL default to 'y'?
>
> And how many would be 'fixed' if someone had given me a straight answer when
> I asked about the TSO firmware, and that failure path no longer aborted the
> driver initialisation but instead just fell back to non-TSO?
>
> I'll look at making the requirement for 'make firmware_install' more obvious,
> or even making it happen automatically as part of 'modules_install'.

I won't mind this as long as I can get a working kernel without doing make
firmware_install or make modules_install (I almost never use modules, my
laptop is one of the few exceptions, and even there it's mostly becouse of
the intel wireless driver needing userspace for firmware)

David Lang

2008-07-04 02:43:15

[permalink] [raw]

Subject: Question: split-lur // Re: 2.6.26-rc8-mm1

In split-lru, zone->prev_priority seems not to be used.
(just remembers....)
Is this obsolete parameter ?

I'm sorry if I miss something.

Thanks,
-Kame

2008-07-04 08:58:30

[permalink] [raw]

Subject: memcg: lru scan fix (Was: 2.6.26-rc8-mm1

Since rc5-mm3, memcg easily goes into OOM when limit was low.
This is a fix to split-lru to fix OOM.

==
Under memcg, active anon tend not to go to inactive anon.
This will cause OOM in memcg easily when tons of anon was used at once.
This check was lacked in split-lru.

Signed-off-by:KAMEZAWA Hiroyuki <[email protected]>

Index: test-2.6.26-rc8-mm1/mm/vmscan.c
===================================================================
--- test-2.6.26-rc8-mm1.orig/mm/vmscan.c
+++ test-2.6.26-rc8-mm1/mm/vmscan.c
@@ -1501,6 +1501,8 @@ static unsigned long shrink_zone(int pri
*/
if (scan_global_lru(sc) && inactive_anon_is_low(zone))
shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
+ else if (!scan_global_lru(sc))
+ shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);

throttle_vm_writeout(sc->gfp_mask);
return nr_reclaimed;

2008-07-04 09:03:38

[permalink] [raw]

Subject: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

My swapcache accounting under memcg patch failed to catch tmpfs(shmem)'s one.
Can I test this under -mm tree ?
(If -mm is busy, I'm not in hurry.)
This patch works well in my box.
=
SwapCache handling fix.

shmem's swapcache behavior is a little different from anonymous's one and
memcg failed to handle it. This patch tries to fix it.

After this:

Any page marked as SwapCache is not uncharged. (delelte_from_swap_cache()
delete the SwapCache flag.)

To check a shmem-page-cache is alive or not we use
page->mapping && !PageAnon(page) instead of
pc->flags & PAGE_CGROUP_FLAG_CACHE.

Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>

Index: test-2.6.26-rc8-mm1/mm/memcontrol.c
===================================================================
--- test-2.6.26-rc8-mm1.orig/mm/memcontrol.c
+++ test-2.6.26-rc8-mm1/mm/memcontrol.c
@@ -687,11 +687,45 @@ __mem_cgroup_uncharge_common(struct page

VM_BUG_ON(pc->page != page);

- if ((ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED)
- && ((pc->flags & PAGE_CGROUP_FLAG_CACHE)
- || page_mapped(page)
- || PageSwapCache(page)))
+ /*
+ * File Cache
+ * If called with MEM_CGROUP_CHARGE_TYPE_MAPPED, check page->mapping.
+ * add_to_page_cache() .... charged before inserting radix-tree.
+ * remove_from_page_cache() .... uncharged at removing from radix-tree.
+ * page->mapping && !PageAnon(page) catches file cache.
+ *
+ * Anon/Shmem.....We check PageSwapCache(page).
+ * Anon .... charged before mapped.
+ * Shmem .... charged at add_to_page_cache() as usual File Cache.
+ *
+ * This page will be finally uncharged when removed from swap-cache
+ *
+ * we treat 2 cases here.
+ * A. anonymous page B. shmem.
+ * We never uncharge if page is marked as SwapCache.
+ * add_to_swap_cache() have nothing to do with charge/uncharge.
+ * SwapCache flag is deleted before delete_from_swap_cache() calls this
+ *
+ * shmem's behavior is following. (see shmem.c/swap_state.c also)
+ * at swap-out:
+ * 0. add_to_page_cache()//charged at page creation.
+ * 1. add_to_swap_cache() (marked as SwapCache)
+ * 2. remove_from_page_cache(). (calls this.)
+ * (finally) delete_from_swap_cache(). (calls this.)
+ * at swap-in:
+ * 3. add_to_swap_cache() (no charge here.)
+ * 4. add_to_page_cache() (charged here.)
+ * 5. delete_from_swap_cache() (calls this.)
+ * PageSwapCache(page) catches "2".
+ * page->mapping && !PageAnon() catches "5" and avoid uncharging.
+ */
+ if (PageSwapCache(page))
goto unlock;
+ /* called from unmap or delete_from_swap_cache() */
+ if ((ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED)
+ && (page_mapped(page)
+ || (page->mapping && !PageAnon(page))))/* alive cache ? */
+ goto unlock;

mz = page_cgroup_zoneinfo(pc);
spin_lock_irqsave(&mz->lru_lock, flags);

2008-07-04 10:08:12

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Thu, 2008-07-03 at 19:42 -0700, [email protected] wrote:
> > (After making it possible to build that firmware _into_ the kernel so that we
> > aren't forcing people to use an initrd where they didn't before, of course.)
>
> has this taken place yet? (and if so, what kernel version first included
> this fix)

It's in linux-next now. See the CONFIG_EXTRA_FIRMWARE option.

That's one of the reasons Ted's ranting about 'religious fundamentalism'
is so funny -- I've actually made it _easier_ for you to combine
arbitrary firmware files into your kernel.

> >> If it was purely technical, you wouldn't be choosing defaults that
> >> break things for users by default.
> >
> > Actually, the beauty of Linux is that we _can_ change things where a minor
> > short-term inconvenience leads to a better situation in the long term.
>
> but doing so should not be a easy and quick decision, and it needs to be
> made very clear exactly what breakage is going to take place and why
> (along with the explination of why the breakage couldn't be avoided)

All forms of change introduce _some_ risk of breakage, of course. In
this case, as usual, I've tried to be careful to avoid regressions. The
most important part, obviously, was having a way to build firmware into
the static kernel.

When it comes to _modules_, doing that would introduce a certain amount
of complexity which just doesn't seem necessary -- if you can load
modules, then you have userspace, and you can use hotplug for firmware
too. Especially given that so many modern drivers already _require_ you
to do that, so the users understand it, and the tools like mkinitrd
already cope with it -- checking MODULE_FIRMWARE() for the modules they
include and including the appropriate files automatically.

The alleged problem with modules seems to be _just_ about the fact that
people need to run 'make firmware_install', and need to have their
firmware installed. Something which all modern drivers require _anyway_,
and people are used to in the general case already. It's not exactly
hard; there's just the initial step of realising that the driver _you_
are using has now been updated to behave like all the others.

That step is _minor_, and it doesn't actually get any easier _however_
long you postpone it. Yes, you might get 10 people in the first day
tripping over it, and I'll look to see if I can make it clearer. But
it's still a very minor, short-term thing.

I suspect that the best option is just to hold off on updating the net
drivers until later, when people already _know_ that they need to run
'make firmware_install', (or it happens automatically or something, if
we go down that route). That way, Dave and Jeff shouldn't be affected by
the initial transition period.

There's plenty of other drivers which need updating, after all. And most
maintainers are _happy_ to see the patches to bring their drivers in
line with best current practice.

> > I'll look at making the requirement for 'make firmware_install' more obvious,
> > or even making it happen automatically as part of 'modules_install'.
>
> I won't mind this as long as I can get a working kernel without doing make
> firmware_install or make modules_install

You can. You need to stay sober for long enough to say 'Y' when it asks
you if you want to build the required firmware into the kernel. And we
even made that the _default_ now, for the benefit of those who can't
stay sober that long. (Perhaps we'll make 'allyesconfig' the default
next?)

> (I almost never use modules, my laptop is one of the few exceptions,
> and even there it's mostly becouse of the intel wireless driver
> needing userspace for firmware)

You don't need to do that any more :)

--
dwmw2

2008-07-04 10:11:20

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse <[email protected]> writes:
>
> I'll look at making the requirement for 'make firmware_install' more
> obvious, or even making it happen automatically as part of
> 'modules_install'.

Perhaps I didn't pay enough attention, but how are "only
boot bzImage without initrd or modules" setups supposed to work now
for those drivers? My testing setup relies on that heavily.

Will the firmware automatically end up in initramfs and be included
in the bzImage and loaded at the right point?

I hope we won't let lawyers decide technical topics here.

-Andi

2008-07-04 11:06:21

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

At Thu, 3 Jul 2008 14:04:36 +0100 (BST),
Hugh Dickins wrote:
>
> On Thu, 3 Jul 2008, Jeff Garzik wrote:
> > KOSAKI Motohiro wrote:
> > > Hi Michael,
> > >
> > > my server output following error message on 2.6.26-rc8-mm1.
> > > Is this a bug?
> > >
> > > ------------------------------------------------------------------
> > > tg3.c:v3.93 (May 22, 2008)
> > > GSI 72 (level, low) -> CPU 0 (0x0001) vector 51
> > > tg3 0000:06:01.0: PCI INT A -> GSI 72 (level, low) -> IRQ 51
> > > firmware: requesting tigon/tg3_tso.bin
> > > tg3: Failed to load firmware "tigon/tg3_tso.bin"
> > > tg3 0000:06:01.0: PCI INT A disabled
> > > GSI 72 (level, low) -> CPU 0 (0x0001) vector 51 unregistered
> > > tg3: probe of 0000:06:01.0 failed with error -2
> > > GSI 73 (level, low) -> CPU 0 (0x0001) vector 51
> > > tg3 0000:06:01.1: PCI INT B -> GSI 73 (level, low) -> IRQ 52
> > > firmware: requesting tigon/tg3_tso.bin
> >
> > This change did not come from the network developers or Broadcom, so someone
> > else broke tg3 in -mm...
>
> I think it's a consequence of not choosing CONFIG_FIRMWARE_IN_KERNEL=y.
>
> That caught me out on PowerMac G5 trying mmotm yesterday, it just hung
> for a few minutes in earlyish boot with a message about tg3_tso.bin,
> and then proceeded to boot up but without the network. I was unclear
> whether I'd been stupid, or the FIRMWARE_IN_KERNEL Kconfigery was poor.
>
> I avoid initrd, and have tigon3 built in, if that's of any relevance.
>
> I wonder if that's Andrew's problem with 2.6.26-rc8-mm1 on his G5:
> mine here boots up fine (now I know to CONFIG_FIRMWARE_IN_KERNEL=y).

Hmm, I got this error even with CONFIG_FIRMWARE_IN_KERNEL=y.

Through a quick look at the code, the firmwares are not built indeed.
I guess the fix like the following needed for building firmwares for
modules. Now trying to build the kernel again to check this...

Takashi

diff --git a/firmware/Makefile b/firmware/Makefile
index f88d746..e11fc2a 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -55,6 +55,8 @@ fw-shipped-$(CONFIG_USB_SERIAL_XIRCOM) += keyspan_pda/xircom_pgs.fw
fw-shipped-$(CONFIG_USB_VICAM) += vicam/firmware.fw
fw-shipped-$(CONFIG_VIDEO_CPIA2) += cpia2/stv0672_vp4.bin

+fw-shipped-y := $(fw-shipped-y) $(fw-shipped-m)
+
# If CONFIG_FIRMWARE_IN_KERNEL is not set, then don't include any firmware
ifneq ($(CONFIG_FIRMWARE_IN_KERNEL),y)
fw-shipped-y :=

2008-07-04 13:11:25

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 12:09 +0200, Andi Kleen wrote:
> David Woodhouse <[email protected]> writes:
> >
> > I'll look at making the requirement for 'make firmware_install' more
> > obvious, or even making it happen automatically as part of
> > 'modules_install'.
>
> Perhaps I didn't pay enough attention, but how are "only
> boot bzImage without initrd or modules" setups supposed to work now
> for those drivers? My testing setup relies on that heavily.

That will continue to work just fine.

> Will the firmware automatically end up in initramfs and be included
> in the bzImage and loaded at the right point?

No, not even in the initramfs. It's built _right_ into the static kernel
image, and request_firmware() finds it there without even having to call
out to userspace at all.
http://git.infradead.org/users/dwmw2/firmware-2.6.git?a=commitdiff;h=81d4e79a

--
dwmw2

2008-07-04 13:15:50

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Fri, 2008-07-04 at 12:09 +0200, Andi Kleen wrote:
>> David Woodhouse <[email protected]> writes:
>>> I'll look at making the requirement for 'make firmware_install' more
>>> obvious, or even making it happen automatically as part of
>>> 'modules_install'.
>> Perhaps I didn't pay enough attention, but how are "only
>> boot bzImage without initrd or modules" setups supposed to work now
>> for those drivers? My testing setup relies on that heavily.
>
> That will continue to work just fine.
>
>> Will the firmware automatically end up in initramfs and be included
>> in the bzImage and loaded at the right point?
>
> No, not even in the initramfs. It's built _right_ into the static kernel
> image, and request_firmware() finds it there without even having to call
> out to userspace at all.
> http://git.infradead.org/users/dwmw2/firmware-2.6.git?a=commitdiff;h=81d4e79a

However, there is still a broken element to the system: the firmware no
longer rides along in the module's .ko file. That introduces new
problems for any user and script that copies modules around.

The compiled-in firmware should be in the same place where it was before
your changes -- in the driver's kernel module.

So, yes, there should not be regressions for non-module users. Let's
now solve the regression problem for the other half of the world...

Jeff

2008-07-04 13:18:20

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 13:06 +0200, Takashi Iwai wrote:
> Hmm, I got this error even with CONFIG_FIRMWARE_IN_KERNEL=y.
>
> Through a quick look at the code, the firmwares are not built indeed.
> I guess the fix like the following needed for building firmwares for
> modules. Now trying to build the kernel again to check this...

For modules, you just need run
'make INSTALL_FW_PATH=/lib/firmare firmware_install'.

I should...

1. Change the default to /lib/firmware so that you don't have to set
INSTALL_FW_PATH.
2. Add that to the 'make help' text.
3. Look at making 'make modules_install' installl the firmware required
by the modules it's installing, so you don't even need to do
_anything_ new.

--
dwmw2

2008-07-04 13:27:16

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

At Fri, 04 Jul 2008 14:17:51 +0100,
David Woodhouse wrote:
>
> On Fri, 2008-07-04 at 13:06 +0200, Takashi Iwai wrote:
> > Hmm, I got this error even with CONFIG_FIRMWARE_IN_KERNEL=y.
> >
> > Through a quick look at the code, the firmwares are not built indeed.
> > I guess the fix like the following needed for building firmwares for
> > modules. Now trying to build the kernel again to check this...
>
> For modules, you just need run
> 'make INSTALL_FW_PATH=/lib/firmare firmware_install'.
>
> I should...
>
> 1. Change the default to /lib/firmware so that you don't have to set
> INSTALL_FW_PATH.
> 2. Add that to the 'make help' text.
> 3. Look at making 'make modules_install' installl the firmware required
> by the modules it's installing, so you don't even need to do
> _anything_ new.

Ah I see. I thought you implemented the built-in firmware even for
modules, but apparently it's not.

Is mkinitrd clever enough to put all needed firmware files to initrd
automatically? Otherwise this can still break the existing setup...

thanks,

Takashi

2008-07-04 13:27:55

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 09:15 -0400, Jeff Garzik wrote:
>
> However, there is still a broken element to the system: the firmware no
> longer rides along in the module's .ko file. That introduces new
> problems for any user and script that copies modules around.
>
> The compiled-in firmware should be in the same place where it was before
> your changes -- in the driver's kernel module.

No, Jeff. That is neither new, nor a real problem. You're just
posturing.

That's the way it has been for a _long_ time anyway, for any modern
driver which uses request_firmware(). The whole point about modules is
_modularity_. Yes, that means that sometimes they depend on _other_
modules, or on firmware.

The scripts which handle that kind of thing have handled inter-module
dependencies, and MODULE_FIRMWARE(), for a long time now.

If I ask mkinitrd to include the b43 driver in my initrd, for example,
it should quite happily include both mac80211.ko and the required
firmware.

All I'm doing is updating some of the older drivers which don't conform
to current best practice, and which still keep large chunks of data in
unswappable kernel memory instead of loading it on demand. And making
that more workable in the general case, but giving the _option_ of
building arbitrary firmware into the kernel, for _all_ modern drivers.

Your argument makes about as much sense as an argument that we should
link b43.ko with mac80211.ko so that the 802.11 core code "rides along
in the module's .ko file". It's just silly.

--
dwmw2

2008-07-04 13:28:54

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 15:26 +0200, Takashi Iwai wrote:
> Ah I see. I thought you implemented the built-in firmware even for
> modules, but apparently it's not.

It isn't really necessary. If you can load modules, then you have
userspace. And if you have userspace, you can load firmware too.

> Is mkinitrd clever enough to put all needed firmware files to initrd
> automatically? Otherwise this can still break the existing setup...

Yes, it's had to be clever enough for that for a long time anyway --
most modern drivers have _only_ the 'request_firmware()' option and
never gave you the choice of building it in. I'm just updating some of
the older drivers to catch up.

--
dwmw2

2008-07-04 13:29:17

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> 3. Look at making 'make modules_install' installl the firmware required
> by the modules it's installing, so you don't even need to do
> _anything_ new.

Anything that works with existing build processes would be a positive
step, preventing loads of build&test regressions.

This does not change the need, of course, to default this firmware
install stuff to 'off', and allow distros to turn it on when they're
ready (as Alan, Ted, and others have suggested in this thread).

Jeff

2008-07-04 13:40:11

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Fri, 2008-07-04 at 09:15 -0400, Jeff Garzik wrote:
>> However, there is still a broken element to the system: the firmware no
>> longer rides along in the module's .ko file. That introduces new
>> problems for any user and script that copies modules around.
>>
>> The compiled-in firmware should be in the same place where it was before
>> your changes -- in the driver's kernel module.
>
> No, Jeff. That is neither new, nor a real problem. You're just
> posturing.
>
> That's the way it has been for a _long_ time anyway, for any modern
> driver which uses request_firmware(). The whole point about modules is
> _modularity_. Yes, that means that sometimes they depend on _other_
> modules, or on firmware.
>
> The scripts which handle that kind of thing have handled inter-module
> dependencies, and MODULE_FIRMWARE(), for a long time now.

You have been told repeatedly that cp(1) and scp(1) are commonly used to
transport the module David and I care about -- tg3. It's been a single
file module since birth, and people take advantage of that fact.

Therefore, logically, you have introduced additional dependencies and
regressions into what was once a single-file copy.

If you wish to hand-wave away what developers and users do today as
posturing, that's up to you...

How difficult is it to see that you must create a system that LET'S
PEOPLE CHOOSE whether or not they like your stuff.

Why are you so hell-bent on removing choice?

Why is it so difficult to see the value of KEEPING STUFF WORKING AS IT
WORKS TODAY?

Doing so (a) keeps developers happy, (b) GUARANTEES no regressions, and
(c) in no way excludes /lib/firmware, moving firmware to userspace.

Sheesh.

Let developers, users, and distros endorse your plan on their own
schedule. Stop forcing your choices down our throats.

Jeff

2008-07-04 13:42:38

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> No, not even in the initramfs. It's built _right_ into the static kernel
> image, and request_firmware() finds it there without even having to call
> out to userspace at all.

Great. Thanks.

-Andi

2008-07-04 13:42:51

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Takashi Iwai wrote:
> Ah I see. I thought you implemented the built-in firmware even for
> modules, but apparently it's not.

Correct.

> Is mkinitrd clever enough to put all needed firmware files to initrd
> automatically? Otherwise this can still break the existing setup...

mkinitrd and similar scripts must be updated, so that drivers that
worked prior to dwmw2's changes will continue to work after dwmw2's changes.

If you fail to update some script somewhere, then the driver will be
copied into the initramfs, but not the firmware, with obvious results.

This is not a fail-safe system.

This is an enforced-breakage, flag-day change that will cause a lot of
additional work for a lot of people.

Jeff

2008-07-04 13:45:51

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 09:42 -0400, Jeff Garzik wrote:
> mkinitrd and similar scripts must be updated, so that drivers that
> worked prior to dwmw2's changes will continue to work after dwmw2's
> changes.

> If you fail to update some script somewhere, then the driver will be
> copied into the initramfs, but not the firmware, with obvious results.

No, mkinitrd works fine, because a whole boatload of drivers _already_
require it to work that way and have done for a long time.

Either you are severely mistaken, or you are being deliberately
misleading.

--
dwmw2

2008-07-04 13:46:34

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 09:39 -0400, Jeff Garzik wrote:
> You have been told repeatedly that cp(1) and scp(1) are commonly used
> to transport the module David and I care about -- tg3. It's been a
> single file module since birth, and people take advantage of that
> fact.

And you can _continue_ to do that. You'd need to install the firmware
just once, and that's all. It's a non-issue, and it isn't _worth_ the
added complexity of building the firmware into the module.

_Especially_ since I strongly suspect you'll come back with an even
sillier argument if we do.

Like claiming that you also run 'grep request_firmware tg3.ko' and your
scripts will fail if it matches...

--
dwmw2

2008-07-04 13:46:51

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> Why is it so difficult to see the value of KEEPING STUFF WORKING AS IT
> WORKS TODAY?

Sure Jeff. Lets delete libata, that caused all sorts of problems when it
was being added. We could freeze on linux 1.2.13-lmp, that was a good
release - why break it ?

There are good sound reasons for having a firmware tree, the fact tg3 is
a bit of dinosaur in this area doesn't make it wrong.

2008-07-04 13:49:00

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 14:27 +0100, Alan Cox wrote:
> > Why is it so difficult to see the value of KEEPING STUFF WORKING AS
> IT
> > WORKS TODAY?
>
> Sure Jeff. Lets delete libata, that caused all sorts of problems when
> it was being added.

A particularly good example. I used to be able to just copy around the
driver for my host controller. Now I have to copy _two_ files, so the
world is going to end! Should I have insisted that we link libata
statically into each and every module which uses it?

(Or at least I have to copy two files _if_ I actually want to make
changes to libata.ko too, which I almost never do...)

--
dwmw2

2008-07-04 14:06:53

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
>> Why is it so difficult to see the value of KEEPING STUFF WORKING AS IT
>> WORKS TODAY?
>
> Sure Jeff. Lets delete libata, that caused all sorts of problems when it
> was being added. We could freeze on linux 1.2.13-lmp, that was a good
> release - why break it ?
>
> There are good sound reasons for having a firmware tree, the fact tg3 is
> a bit of dinosaur in this area doesn't make it wrong.

I never said it was wrong.

I have said repeatedly that separating out the firmware is the right
thing to do.

But... you don't need to force the switchover. You don't need to break
things that work today, in order to accomplish this.

It is quite feasible to do both -- keep things working as they work
today, _and_ add /lib/firmware infrastructure. Then we can work to
switch distros over to the new system.

Further, it is not only feasible, but the only "nice" thing to do to
other developers, users, and distros: permit them to choose when to
stop the decades-old practice of building firmware into some drivers.

Perform the transition in a sane, staged, planned manner that doesn't
result in tons of non-working drivers. I have already provided many
real world examples where people, doing the same things they do today,
will be greeted with non-working drivers upon the next boot. Without
any warning or error messages along the way, hinting that something
might be wrong.

Or, for the cheap seats:

End goal: good

dwmw2's current path: very easy to produce dead driver

Needed resolution: first step should /not/ produce regressions;

current evidence demonstrates current
implementation is full of regressions
that seasoned kernel hackers are hitting

It /is/ possible to add /lib/firmware gadgetry while avoiding the
obvious low-hanging-fruit regressions and flag-day conversions that have
been pointed out here.

Just say no to flag-day changes like this. It is possible for each
distro and boot image script to have their own flag-day. Give them that
choice.

Jeff

2008-07-04 14:07:48

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Fri, 2008-07-04 at 09:39 -0400, Jeff Garzik wrote:
>> You have been told repeatedly that cp(1) and scp(1) are commonly used
>> to transport the module David and I care about -- tg3. It's been a
>> single file module since birth, and people take advantage of that
>> fact.
>
> And you can _continue_ to do that. You'd need to install the firmware
> just once, and that's all. It's a non-issue, and it isn't _worth_ the
> added complexity of building the firmware into the module.

We can stop there. Real-world examples are apparently non-issues not
worth your time.

Jeff

2008-07-04 14:11:13

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Fri, 2008-07-04 at 09:42 -0400, Jeff Garzik wrote:
>> mkinitrd and similar scripts must be updated, so that drivers that
>> worked prior to dwmw2's changes will continue to work after dwmw2's
>> changes.
>
>> If you fail to update some script somewhere, then the driver will be
>> copied into the initramfs, but not the firmware, with obvious results.
>
> No, mkinitrd works fine, because a whole boatload of drivers _already_
> require it to work that way and have done for a long time.
>
> Either you are severely mistaken, or you are being deliberately
> misleading.

It is a fact that mkinitrd, today, is unaware of your new system of
obtaining firmware from the kernel source[or build] tree.

Certainly it is aware of the need to copy firmware in general, but that
doesn't change the fact that the tg3 firmware will not make it into
initramfs, without additional steps taken.

So, no, it doesn't "work fine" -- the firmware doesn't make it into the
initramfs.

Jeff

2008-07-04 14:12:26

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 02:27:15PM +0100, David Woodhouse wrote:
>
> That's the way it has been for a _long_ time anyway, for any modern
> driver which uses request_firmware(). The whole point about modules is
> _modularity_. Yes, that means that sometimes they depend on _other_
> modules, or on firmware.
>
> The scripts which handle that kind of thing have handled inter-module
> dependencies, and MODULE_FIRMWARE(), for a long time now.

FYI, at least Ubuntu Hardy's initramfs does not seem to deal with
firmware for modules correctly.

https://bugs.launchpad.net/ubuntu/+source/initramfs-tools/+bug/180544

And remember, kernel/userspace interfaces are things which are far
more careful about than kernel ABI interfaces....

You can flame about Ubuntu being broken (and I predict you will :-),
but there are a large number of users who do use Ubuntu. And so
adding more breakages when it is *known* the distro's aren't moving as
quickly as you think is reasonable for quote, modern, unquote, drivers
is something you can flame about, but at the end of the day, *you* are
the one introducing changes that is causing more breakages.

Userspace interfaces (and this includes things like
mkinitramfs/mkinitrd, since we made the design decision --- in my
opinion a very bad decision --- to make initrd/initramfs creation it a
distro-specific thing instead of somethign where the kernel supplies
the scripts) by their very nature move much more slowly than things
which are inside the "shipped by the kernel" boundary.

And sometimes people like to take a RHEL4 or RHEL5 (or Ubuntu Hardy)
kernel and compile and build a much newer kernel from kernel.org, and
it is *highly* unfortunate when this breaks. After all, for people
who care to test our kernel.org kernels, we want to encourage them,
yes? More testers of kernel.org testers is something which I've heard
akpm claim is a good thing....

I do think your idea of including "make firmware_install" into "make
modules_install" does make a lot of sense, because it will reduce the
number of breakages. It won't eliminate them, but it will reduce them.

Regards,

- Ted

2008-07-04 14:13:47

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 10:10 -0400, Jeff Garzik wrote:
> David Woodhouse wrote:
> > On Fri, 2008-07-04 at 09:42 -0400, Jeff Garzik wrote:
> >> mkinitrd and similar scripts must be updated, so that drivers that
> >> worked prior to dwmw2's changes will continue to work after dwmw2's
> >> changes.
> >
> >> If you fail to update some script somewhere, then the driver will be
> >> copied into the initramfs, but not the firmware, with obvious results.
> >
> > No, mkinitrd works fine, because a whole boatload of drivers _already_
> > require it to work that way and have done for a long time.
> >
> > Either you are severely mistaken, or you are being deliberately
> > misleading.
>
> It is a fact that mkinitrd, today, is unaware of your new system of
> obtaining firmware from the kernel source[or build] tree.

Of _course_ it is. Just as it's unaware of the need to download the b43
driver and b43-fwcutter and extract it.... what was your point, again?

I'm working on making the required firmware get installed as part of
'make modules_install'. We already discussed that. There's no need for
you to continue crying wolf with nonsense like 'mkinitrd won't work'.

--
dwmw2

2008-07-04 14:24:06

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

At Fri, 4 Jul 2008 10:10:14 -0400,
Theodore Tso wrote:
>
> On Fri, Jul 04, 2008 at 02:27:15PM +0100, David Woodhouse wrote:
> >
> > That's the way it has been for a _long_ time anyway, for any modern
> > driver which uses request_firmware(). The whole point about modules is
> > _modularity_. Yes, that means that sometimes they depend on _other_
> > modules, or on firmware.
> >
> > The scripts which handle that kind of thing have handled inter-module
> > dependencies, and MODULE_FIRMWARE(), for a long time now.
>
> FYI, at least Ubuntu Hardy's initramfs does not seem to deal with
> firmware for modules correctly.

Neither SUSE's mkinitrd.
(Hannes, please correct if I'm wrong...)

Takashi

2008-07-04 14:27:00

by maximilian attems

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 10:10:14AM -0400, Theodore Tso wrote:
> On Fri, Jul 04, 2008 at 02:27:15PM +0100, David Woodhouse wrote:
> >
> > That's the way it has been for a _long_ time anyway, for any modern
> > driver which uses request_firmware(). The whole point about modules is
> > _modularity_. Yes, that means that sometimes they depend on _other_
> > modules, or on firmware.
> >
> > The scripts which handle that kind of thing have handled inter-module
> > dependencies, and MODULE_FIRMWARE(), for a long time now.
>
> FYI, at least Ubuntu Hardy's initramfs does not seem to deal with
> firmware for modules correctly.
>
> https://bugs.launchpad.net/ubuntu/+source/initramfs-tools/+bug/180544
>
> And remember, kernel/userspace interfaces are things which are far
> more careful about than kernel ABI interfaces....
>
> You can flame about Ubuntu being broken (and I predict you will :-),
> but there are a large number of users who do use Ubuntu. And so
> adding more breakages when it is *known* the distro's aren't moving as
> quickly as you think is reasonable for quote, modern, unquote, drivers
> is something you can flame about, but at the end of the day, *you* are
> the one introducing changes that is causing more breakages.

yes i'd call them severly broken.
as it is quite easy to pick up the modinfo firmware module output.

their trouble is that they sync initramfs-tools from Debian only
once every 2 years or so.

> Userspace interfaces (and this includes things like
> mkinitramfs/mkinitrd, since we made the design decision --- in my
> opinion a very bad decision --- to make initrd/initramfs creation it a
> distro-specific thing instead of somethign where the kernel supplies
> the scripts) by their very nature move much more slowly than things
> which are inside the "shipped by the kernel" boundary.

hpa is working to provide a lot of what is needed in klibc.
it isn't yet there as it misses mdadm, lvm2 and cryptsetup support,
but it is getting much better due to our Debian/Ubuntu exposure.
we added several features to klibc and fixed bugs. similar to the
early opensuse exposure which unfortunately got dropped.

although kinit strived for a very monolithic way that doesn't fit
the modular needs of a distribution. klibc and klibc-utils are
the base of our initramfs.

best regards

--
maks

2008-07-04 14:31:36

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 09:39:36AM -0400, Jeff Garzik wrote:
> You have been told repeatedly that cp(1) and scp(1) are commonly used to
> transport the module David and I care about -- tg3. It's been a single
> file module since birth, and people take advantage of that fact.

Here, I think I'll have to respectly disagree with you and say that
you are taking things too far. I don't think scp'ing individual
modules around counts as an "exported user interface" the same way,
say "make install; make modules_install" is a commonly understand and
used interface by users and scripts (i.e., such as Debian's make-kpkg,
which does NOT know about "make firmware_install", BTW).

Asking developers that they need to scp an additional module doesn't
seem terribly onerous to me --- especially if the firmware module is
much more likely to be static, and probably doesn't need to be changed
after each compile/edit/debug cycle.

So on this point I'd side with David, and say that folding "make
firmware_install" into "make modules_install" goes a long way towards
healing this particular breakage.

HOWEVER, as I mentioned in another message, it looks like not all
forms of mkinitd and/or mkinitramfs scripts deal with /lib/firmware
correctly, including the one used by the latest version of Ubuntu.
That to me is a strong argument for either (a) leaving drivers the way
they are now, or (b) making the new request_firmware() framework be
able to place the firemware in either the original driver module, or
in another tg3_firmware.ko module --- which could be unloaded
afterwards, if people really cared about the non-swappable kernel
memory being used up.)

And this is where we pay the price for not having a standard initrd
generation (with appropriate hooks so that distros could drop in their
own enhancements) as part of the kernel build process. If we did, it
would be a lot easier to make sure all distro's learn about new
requirements that we have imposed on the initrd. Because we haven't,
initrd's are effectively part of the "exported interface" where we
have to move slowly enough so that distro's can catch up depending on
their release schedule. (It also makes it much harder to run a
bleeding-edge kernel on a release distro system, at least without
tieing our hands with respect to changes involving the initrd.)

- Ted

2008-07-04 14:32:29

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 10:10 -0400, Theodore Tso wrote:
>
> FYI, at least Ubuntu Hardy's initramfs does not seem to deal with
> firmware for modules correctly.
>
> https://bugs.launchpad.net/ubuntu/+source/initramfs-tools/+bug/180544
>
> And remember, kernel/userspace interfaces are things which are far
> more careful about than kernel ABI interfaces....
>
> You can flame about Ubuntu being broken (and I predict you will :-),

Flaming about it being broken wouldn't help; we _have_ to cope with it.
Thanks for the reference; I'll keep an eye on it.

Again, though, this just makes it clear that it's a _already_ a problem
which affects _every_ modern driver that uses request_firmware() -- so
one might reasonably assume it's already quite a high priority for them
to fix. Remember, I'm not doing anything _new_ when I update drivers to
use request_firmware(). That's actually been the norm for new drivers,
like ipw2200, for a _long_ time now.

So I'd kind of expect that by the time Ubuntu gets round to shipping a
2.6.27 kernel, they'd have long since fixed the bug in their scripts.
The few extra drivers which we've updated to conform to best current
practice, in the few cases where people actually need them in the
initrd, should be fairly much in the noise.

But as I said before, maybe there is some sense in leaving the network
drivers for now, and getting on with all the _other_ drivers which need
updating to use request_firmware() first.

> And so adding more breakages when it is *known* the distro's aren't
> moving as quickly as you think is reasonable for quote, modern,
> unquote, drivers is something you can flame about, but at the end of
> the day, *you* are the one introducing changes that is causing more#
> breakages.

I don't think the Ubuntu bug you reference is because they aren't
"keeping up". AFAICT their tool _does_ look at MODULE_FIRMWARE() and
include the required firmware in the initramfs. But they have decided to
keep firmware in /lib/firmware/`uname -r`/ instead of /lib/firmware/,
and when making that policy change it looks like they forgot to update
that tool to cope. So it's being stored in the wrong place in the
initramfs.

It's purely a local screwup; just a bug. Not because they're being
'slow'. But it's certainly something we should bear in mind. Thanks for
pointing it out.

--
dwmw2

2008-07-04 14:37:19

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 04:24:03PM +0200, maximilian attems wrote:
> yes i'd call them severly broken.
> as it is quite easy to pick up the modinfo firmware module output.
>
> their trouble is that they sync initramfs-tools from Debian only
> once every 2 years or so.

Well, Takashi just mentioned that SuSE's mkinitrd may not handle
/lib/firmware correctly either....

> hpa is working to provide a lot of what is needed in klibc.
> it isn't yet there as it misses mdadm, lvm2 and cryptsetup support,
> but it is getting much better due to our Debian/Ubuntu exposure.
> we added several features to klibc and fixed bugs. similar to the
> early opensuse exposure which unfortunately got dropped.

I will definitely sign up to try this once there is lvm2 support. :-)

- Ted

2008-07-04 14:38:20

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 10:30 -0400, Theodore Tso wrote:
> HOWEVER, as I mentioned in another message, it looks like not all
> forms of mkinitd and/or mkinitramfs scripts deal with /lib/firmware
> correctly, including the one used by the latest version of Ubuntu.
> That to me is a strong argument for either (a) leaving drivers the way
> they are now, or (b) making the new request_firmware() framework be
> able to place the firemware in either the original driver module, or
> in another tg3_firmware.ko module --- which could be unloaded
> afterwards, if people really cared about the non-swappable kernel
> memory being used up.)

Yeah. I had checked that Ubuntu and Fedora _do_ cope with including
firmware in the kernel, but wasn't expecting that Ubuntu would then go
screw it up.

As I said, it's not _impossible_ to include firmware directly in the
module itself; it should just be a case of adding an additional section
like it was in the kernel too, and handling some lifetime issues.

If Ubuntu (and SuSE) are currently shipping broken initramfs tools, then
that may tip the balance from that being unnecessary complexity to
something we should probably do for the short term. Even though they're
_already_ broken, and we're only really taking it from "broken for 70
drivers in initramfs" to "broken for 90 drivers in initramfs". Or
whatever the numbers are. Admittedly I just made those ones up.

> And this is where we pay the price for not having a standard initrd
> generation (with appropriate hooks so that distros could drop in their
> own enhancements) as part of the kernel build process. If we did, it
> would be a lot easier to make sure all distro's learn about new
> requirements that we have imposed on the initrd. Because we haven't,
> initrd's are effectively part of the "exported interface" where we
> have to move slowly enough so that distro's can catch up depending on
> their release schedule. (It also makes it much harder to run a
> bleeding-edge kernel on a release distro system, at least without
> tieing our hands with respect to changes involving the initrd.)

Yeah, you're probably right.

--
dwmw2

2008-07-04 14:39:48

by Hannes Reinecke

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Hi Takashi,

Takashi Iwai wrote:
> At Fri, 4 Jul 2008 10:10:14 -0400,
> Theodore Tso wrote:
>> On Fri, Jul 04, 2008 at 02:27:15PM +0100, David Woodhouse wrote:
>>> That's the way it has been for a _long_ time anyway, for any modern
>>> driver which uses request_firmware(). The whole point about modules is
>>> _modularity_. Yes, that means that sometimes they depend on _other_
>>> modules, or on firmware.
>>>
>>> The scripts which handle that kind of thing have handled inter-module
>>> dependencies, and MODULE_FIRMWARE(), for a long time now.
>> FYI, at least Ubuntu Hardy's initramfs does not seem to deal with
>> firmware for modules correctly.
>
> Neither SUSE's mkinitrd.
> (Hannes, please correct if I'm wrong...)
>
???

Firmware loading is just a matter of copying the file at the correct
location (ie /lib/firmware) and with the name the fw loader expects.
mkinitrd should do it correctly.
But I wasn't aware that the tg3 has external firmware, so I doubt
we have any rpm for it.

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Markus Rex, HRB 16746 (AG N?rnberg)

2008-07-04 14:43:19

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 16:39 +0200, Hannes Reinecke wrote:
> Firmware loading is just a matter of copying the file at the correct
> location (ie /lib/firmware) and with the name the fw loader expects.
> mkinitrd should do it correctly.
> But I wasn't aware that the tg3 has external firmware, so I doubt
> we have any rpm for it.

It doesn't yet; that patch is in linux-next. The firmware is shipped as
part of the kernel source tree, and you currently need to run 'make
firmware_install' to put it in /lib/firmware, although we're looking at
making that easier because apparently having to run 'make
firmware_install' is too hard...

--
dwmw2

2008-07-04 14:44:53

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

At Fri, 04 Jul 2008 16:39:30 +0200,
Hannes Reinecke wrote:
>
> Hi Takashi,
>
> Takashi Iwai wrote:
> > At Fri, 4 Jul 2008 10:10:14 -0400,
> > Theodore Tso wrote:
> >> On Fri, Jul 04, 2008 at 02:27:15PM +0100, David Woodhouse wrote:
> >>> That's the way it has been for a _long_ time anyway, for any modern
> >>> driver which uses request_firmware(). The whole point about modules is
> >>> _modularity_. Yes, that means that sometimes they depend on _other_
> >>> modules, or on firmware.
> >>>
> >>> The scripts which handle that kind of thing have handled inter-module
> >>> dependencies, and MODULE_FIRMWARE(), for a long time now.
> >> FYI, at least Ubuntu Hardy's initramfs does not seem to deal with
> >> firmware for modules correctly.
> >
> > Neither SUSE's mkinitrd.
> > (Hannes, please correct if I'm wrong...)
> >
> ???
>
> Firmware loading is just a matter of copying the file at the correct
> location (ie /lib/firmware) and with the name the fw loader expects.
> mkinitrd should do it correctly.

OK, then I must have overlooked it in the mkinitrd code.
(Just to be sure - it's about the handling of firmware files in initrd
image, no on root disk.)

> But I wasn't aware that the tg3 has external firmware, so I doubt
> we have any rpm for it.

It's happening in mm and linux-next right now. For tg3 (and any
other) module, no built-in firmware any more.

thanks,

Takashi

2008-07-04 14:57:43

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008 10:07:23 -0400
Jeff Garzik <[email protected]> wrote:

> David Woodhouse wrote:
> > On Fri, 2008-07-04 at 09:39 -0400, Jeff Garzik wrote:
> >> You have been told repeatedly that cp(1) and scp(1) are commonly used
> >> to transport the module David and I care about -- tg3. It's been a
> >> single file module since birth, and people take advantage of that
> >> fact.
> >
> > And you can _continue_ to do that. You'd need to install the firmware
> > just once, and that's all. It's a non-issue, and it isn't _worth_ the
> > added complexity of building the firmware into the module.
>
> We can stop there. Real-world examples are apparently non-issues not
> worth your time.

Jeff - real world issues and gains are measured on a scale across the
userbase not "Jeff's having a tantrum because his specific usage case
requires one extra copy once"

And we had the same argument over ten years ago about those evil module
things which stopped you just using scp to copy the kernel in one go.
Fortunately the nay sayers lost so we have modules.

Alan

2008-07-04 18:02:50

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 10:30 -0400, Theodore Tso wrote:
> So on this point I'd side with David, and say that folding "make
> firmware_install" into "make modules_install" goes a long way towards
> healing this particular breakage.

make modules_install | tail ...
INSTALL fs/nfs/nfs.ko
INSTALL fs/nls/nls_iso8859-1.ko
INSTALL fs/vfat/vfat.ko
MKDIR /lib/firmware/acenic
INSTALL /lib/firmware/acenic/tg2.bin
MKDIR /lib/firmware/tigon
INSTALL /lib/firmware/tigon/tg3.bin
INSTALL /lib/firmware/tigon/tg3_tso.bin
INSTALL /lib/firmware/tigon/tg3_tso5.bin
DEPMOD 2.6.26-rc8

>From 9fcda0ce34142cb8d7450dd262173a1c7c629515 Mon Sep 17 00:00:00 2001
From: David Woodhouse <[email protected]>
Date: Fri, 4 Jul 2008 18:53:27 +0100
Subject: [PATCH] firmware: 'make modules_install' installs firmware to match modules

(and also the static kernel, when CONFIG_FIRMWARE_IN_KERNEL=n).

This means that people no longer have to know to run 'make
firmware_install' for themselves, and firmware should get installed
automatically.

Signed-off-by: David Woodhouse <[email protected]>
---
Makefile | 6 ++++--
firmware/Makefile | 39 +++++++++++++++++++++++++++++----------
scripts/Makefile.fwinst | 21 ++++++++++++++++++---
3 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 947a90f..676fa96 100644
--- a/Makefile
+++ b/Makefile
@@ -996,11 +996,12 @@ depend dep:

# ---------------------------------------------------------------------------
# Firmware install
-INSTALL_FW_PATH=/lib/firmware
+INSTALL_FW_PATH=$(INSTALL_MOD_PATH)/lib/firmware
export INSTALL_FW_PATH

PHONY += firmware_install
firmware_install: FORCE
+ @mkdir -p $(objtree)/firmware
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.fwinst obj=firmware __fw_install

# ---------------------------------------------------------------------------
@@ -1089,6 +1090,7 @@ _modinst_:
# boot script depmod is the master version.
PHONY += _modinst_post
_modinst_post: _modinst_
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.fwinst obj=firmware __fw_modinst
$(call cmd,depmod)

else # CONFIG_MODULES
@@ -1207,7 +1209,7 @@ help:
@echo '* modules - Build all modules'
@echo ' modules_install - Install all modules to INSTALL_MOD_PATH (default: /)'
@echo ' firmware_install- Install all firmware to INSTALL_FW_PATH'
- @echo ' (default: /lib/firmware)'
+ @echo ' (default: $$(INSTALL_MOD_PATH)/lib/firmware)'
@echo ' dir/ - Build all files in dir and below'
@echo ' dir/file.[ois] - Build specified target only'
@echo ' dir/file.ko - Build module including final link'
diff --git a/firmware/Makefile b/firmware/Makefile
index f88d746..da9c92b 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -9,10 +9,24 @@ fwabs := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))

fw-external-y := $(subst ",,$(CONFIG_EXTRA_FIRMWARE))

-ifneq ($(CONFIG_ACENIC_OMIT_TIGON_I),y)
-fw-shipped-$(CONFIG_ACENIC) += acenic/tg1.bin
+# There are three cases to care about:
+# 1. Building kernel with CONFIG_FIRMWARE_IN_KERNEL=y -- $(fw-shipped-y) should
+# include the firmware files to include, according to .config
+# 2. 'make modules_install', which will install firmware for modules, and
+# _also_ for the in-kernel drivers when CONFIG_FIRMWARE_IN_KERNEL=n
+# 3. 'make firmware_install', which installs all firmware, unconditionally.
+
+# For the former two cases we want $(fw-shipped-y) and $(fw-shipped-m) to be
+# accurate. In the latter case it doesn't matter -- it'll use $(fw-shipped-all).
+# But be aware that the config file might not be included at all.
+
+ifdef CONFIG_ACENIC_OMIT_TIGON_I
+acenic-objs := acenic/tg2.bin
+fw-shipped- += acenic/tg1.bin
+else
+acenic-objs := acenic/tg1.bin acenic/tg2.bin
endif
-fw-shipped-$(CONFIG_ACENIC) += acenic/tg2.bin
+fw-shipped-$(CONFIG_ACENIC) += $(acenic-objs)
fw-shipped-$(CONFIG_ATM_AMBASSADOR) += atmsar11.fw
fw-shipped-$(CONFIG_COMPUTONE) += intelliport2.bin
fw-shipped-$(CONFIG_DVB_AV7110) += av7110/bootcode.bin
@@ -35,6 +49,7 @@ fw-shipped-$(CONFIG_USB_EMI62) += emi62/loader.fw emi62/bitstream.fw \
fw-shipped-$(CONFIG_USB_KAWETH) += kaweth/new_code.bin kaweth/trigger_code.bin \
kaweth/new_code_fix.bin \
kaweth/trigger_code_fix.bin
+ifdef CONFIG_FIRMWARE_IN_KERNEL
fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_MPR) += keyspan/mpr.fw
fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_USA18X) += keyspan/usa18x.fw
fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_USA19) += keyspan/usa19.fw
@@ -47,6 +62,12 @@ fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_USA28XB) += keyspan/usa28xb.fw
fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_USA28X) += keyspan/usa28x.fw
fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_USA49W) += keyspan/usa49w.fw
fw-shipped-$(CONFIG_USB_SERIAL_KEYSPAN_USA49WLC) += keyspan/usa49wlc.fw
+else
+fw-shipped- := keyspan/mpr.fw keyspan/usa18x.fw keyspan/usa19.fw \
+ keyspan/usa19qi.fw keyspan/usa19qw.fw keyspan/usa19w.fw \
+ keyspan/usa28.fw keyspan/usa28xa.fw keyspan/usa28xb.fw \
+ keyspan/usa28x.fw keyspan/usa49w.fw keyspan/usa49wlc.fw
+endif
fw-shipped-$(CONFIG_USB_SERIAL_TI) += ti_3410.fw ti_5052.fw
fw-shipped-$(CONFIG_USB_SERIAL_WHITEHEAT) += whiteheat_loader.fw whiteheat.fw \
# whiteheat_loader_debug.fw
@@ -55,13 +76,10 @@ fw-shipped-$(CONFIG_USB_SERIAL_XIRCOM) += keyspan_pda/xircom_pgs.fw
fw-shipped-$(CONFIG_USB_VICAM) += vicam/firmware.fw
fw-shipped-$(CONFIG_VIDEO_CPIA2) += cpia2/stv0672_vp4.bin

-# If CONFIG_FIRMWARE_IN_KERNEL is not set, then don't include any firmware
-ifneq ($(CONFIG_FIRMWARE_IN_KERNEL),y)
-fw-shipped-y :=
-endif
+fw-shipped-all := $(fw-shipped-y) $(fw-shipped-m) $(fw-shipped-)

-firmware-y := $(fw-external-y) $(fw-shipped-y)
-firmware-dirs := $(sort $(patsubst %,$(objtree)/$(obj)/%/,$(dir $(firmware-y) $(fw-shipped-))))
+# Directories which we _might_ need to create, so we have a rule for them.
+firmware-dirs := $(sort $(patsubst %,$(objtree)/$(obj)/%/,$(dir $(fw-external-y) $(fw-shipped-all))))

quiet_cmd_mkdir = MKDIR $(patsubst $(objtree)/%,%,$@)
cmd_mkdir = mkdir -p $@
@@ -146,7 +164,8 @@ $(obj)/%.fw: $(obj)/%.H16 $(obj)/ihex2fw | $(objtree)/$(obj)/$$(dir %)
$(firmware-dirs):
$(call cmd,mkdir)

-obj-y := $(patsubst %,%.gen.o, $(firmware-y))
+obj-y += $(patsubst %,%.gen.o, $(fw-external-y))
+obj-$(CONFIG_FIRMWARE_IN_KERNEL) += $(patsubst %,%.gen.o, $(fw-shipped-y))

# Remove .S files and binaries created from ihex
# (during 'make clean' .config isn't included so they're all in $(fw-shipped-))
diff --git a/scripts/Makefile.fwinst b/scripts/Makefile.fwinst
index 8301802..df91610 100644
--- a/scripts/Makefile.fwinst
+++ b/scripts/Makefile.fwinst
@@ -8,13 +8,25 @@
INSTALL := install
src := $(obj)

+# For modules_install installing firmware, we want to see .config
+# But for firmware_install, we don't care, but don't want to require it.
+-include $(objtree)/.config
+
include scripts/Kbuild.include
include $(srctree)/$(obj)/Makefile

include scripts/Makefile.host

-installed-fw := $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-))
-installed-fw-dirs := $(sort $(dir $(installed-fw)))
+mod-fw := $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-m))
+
+# If CONFIG_FIRMWARE_IN_KERNEL isn't set, then install the
+# firmware for in-kernel drivers too.
+ifndef CONFIG_FIRMWARE_IN_KERNEL
+mod-fw += $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-y))
+endif
+
+installed-fw := $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-all))
+installed-fw-dirs := $(sort $(dir $(installed-fw))) $(INSTALL_FW_PATH)/.

quiet_cmd_install = INSTALL $(subst $(srctree)/,,$@)
cmd_install = $(INSTALL) -m0644 $< $@
@@ -25,7 +37,10 @@ $(installed-fw-dirs):
$(installed-fw): $(INSTALL_FW_PATH)/%: $(obj)/% | $(INSTALL_FW_PATH)/$$(dir %)/
$(call cmd,install)

-.PHONY: __fw_install FORCE
+.PHONY: __fw_install __fw_modinst FORCE
+
__fw_install: $(installed-fw)
+__fw_modinst: $(mod-fw)
+

FORCE:
--
1.5.5.1

--
dwmw2

2008-07-04 19:17:22

by Rik van Riel

[permalink] [raw]

Subject: Re: memcg: lru scan fix (Was: 2.6.26-rc8-mm1

On Fri, 4 Jul 2008 18:02:26 +0900
KAMEZAWA Hiroyuki <[email protected]> wrote:

> Index: test-2.6.26-rc8-mm1/mm/vmscan.c
> ===================================================================
> --- test-2.6.26-rc8-mm1.orig/mm/vmscan.c
> +++ test-2.6.26-rc8-mm1/mm/vmscan.c
> @@ -1501,6 +1501,8 @@ static unsigned long shrink_zone(int pri
> */
> if (scan_global_lru(sc) && inactive_anon_is_low(zone))
> shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
> + else if (!scan_global_lru(sc))
> + shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);

Makes sense.

Acked-by: Rik van Riel <[email protected]>

--
All rights reversed.

2008-07-04 19:26:06

[permalink] [raw]

Subject: Re: memcg: lru scan fix (Was: 2.6.26-rc8-mm1

On Fri, 4 Jul 2008 15:16:56 -0400 Rik van Riel <[email protected]> wrote:

> On Fri, 4 Jul 2008 18:02:26 +0900
> KAMEZAWA Hiroyuki <[email protected]> wrote:
>
> > Index: test-2.6.26-rc8-mm1/mm/vmscan.c
> > ===================================================================
> > --- test-2.6.26-rc8-mm1.orig/mm/vmscan.c
> > +++ test-2.6.26-rc8-mm1/mm/vmscan.c
> > @@ -1501,6 +1501,8 @@ static unsigned long shrink_zone(int pri
> > */
> > if (scan_global_lru(sc) && inactive_anon_is_low(zone))
> > shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
> > + else if (!scan_global_lru(sc))
> > + shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
>
> Makes sense.
>
> Acked-by: Rik van Riel <[email protected]>
>

Thanks. Poor old me needs to work out which patch this patch fixes.
It's always appreciated if others tell me :)

2008-07-04 20:27:52

by Sam Ravnborg

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 07:01:56PM +0100, David Woodhouse wrote:
> On Fri, 2008-07-04 at 10:30 -0400, Theodore Tso wrote:
> > So on this point I'd side with David, and say that folding "make
> > firmware_install" into "make modules_install" goes a long way towards
> > healing this particular breakage.
>
> make modules_install | tail ...
> INSTALL fs/nfs/nfs.ko
> INSTALL fs/nls/nls_iso8859-1.ko
> INSTALL fs/vfat/vfat.ko
> MKDIR /lib/firmware/acenic
> INSTALL /lib/firmware/acenic/tg2.bin
> MKDIR /lib/firmware/tigon
> INSTALL /lib/firmware/tigon/tg3.bin
> INSTALL /lib/firmware/tigon/tg3_tso.bin
> INSTALL /lib/firmware/tigon/tg3_tso5.bin
> DEPMOD 2.6.26-rc8

I have not followed the threads about firmware - but installing
the firmware on par with modules_install seems like a good idea to me.
I gave the kbuild bits a quick look and they have my:
Acked-by: Sam Ravnborg <[email protected]>

I have yet to give all the firmware stuff a detailed review
but that will be when I get time to it.

Sam

2008-07-04 20:37:34

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: David Woodhouse <[email protected]>
Date: Fri, 04 Jul 2008 14:27:15 +0100

> Your argument makes about as much sense as an argument that we should
> link b43.ko with mac80211.ko so that the 802.11 core code "rides along
> in the module's .ko file". It's just silly.

I totally disagree with you. Jeff is right and you are wrong.

We have a large set of drivers which you are basically breaking.

You keep harping on this "current best practices" crap but it's simply
that, crap. The only argument behind why you're doing this is a legal
one.

And for one, I have consistently argued that this "best practice" is
the "worst practice" from a technical perspective. It is the worst
because it means mistakes are possible to make between driver and
firmware versions. Even with versioning it is not fool proof.
Whereas if you link the firmware into the driver, it's impossible to
get wrong.

It's the difference between "might get it wrong" and "impossible
to get wrong." And what you're doing is swaying these drivers
away from the latter and towards the former.

That to me is a strong technical argument against your changes.

So stop bringing up this "best practices" garbage. It's "best
practices" from someone's legal perspective, not from a kernel
technical one, and you know it.

Tell me, would you even invest one single second of your time on this
bogus set of changes without the legal impetus? Would you sit here
and listen to Jeff, myself, and the others scream at you for breaking
things on people for what you claim are "technical" reasons?

No, you would absolutely not work on this without the legal incentive.
There would be no reason to, since everything works perfectly fine now.

I want you to be completely honest about the real reason why you're
making any of these decisions the way you are, RIGHT NOW. I don't
want to hear any more of this "best practices" request_firmware()
crap, because that's just nonsense.

It seems your employer is telling you to work on this in order to sort
out some perceived legal issue. And that's the only reason you're
investing any effort into this.

2008-07-04 20:39:19

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Jeff Garzik <[email protected]>
Date: Fri, 04 Jul 2008 09:39:36 -0400

> Stop forcing your choices down our throats.

I completely agree with Jeff.

2008-07-04 20:42:47

by Arjan van de Ven

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008 13:37:21 -0700 (PDT)
David Miller <[email protected]> wrote:

> It seems your employer is telling you to work on this in order to sort
> out some perceived legal issue. And that's the only reason you're
> investing any effort into this.

I assume you meant Davids previous employer (Red Hat) with this, not
his current one (Intel). I seriously doubt (and know for sure we never
asked/told David anything around this) that Intel cares about what
happens in tg3.

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-07-04 20:43:42

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Alan Cox <[email protected]>
Date: Fri, 4 Jul 2008 14:27:53 +0100

> There are good sound reasons for having a firmware tree, the fact tg3 is
> a bit of dinosaur in this area doesn't make it wrong.

And bnx2, and bnx2x, and e100's ucode (hope David caught that one!).

It isn't just tg3.

External firmware is by design an error prone system, even with
versioning. But by being built and linked into the driver, it
is fool proof.

On a technical basis alone, we would never disconnect a crucial
component such as firmware, from the driver. The only thing
charging these transoformations, from day one, is legal concerns.

I've been against request_firmware() from the beginning, because
they make life unnecessarily difficult, and it is error prone no
matter how well you design the validation step.

2008-07-04 20:44:14

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 13:42 -0700, Arjan van de Ven wrote:
> On Fri, 04 Jul 2008 13:37:21 -0700 (PDT)
> David Miller <[email protected]> wrote:
>
> > It seems your employer is telling you to work on this in order to sort
> > out some perceived legal issue. And that's the only reason you're
> > investing any effort into this.
>
> I assume you meant Davids previous employer (Red Hat) with this, not
> his current one (Intel). I seriously doubt (and know for sure we never
> asked/told David anything around this) that Intel cares about what
> happens in tg3.

He must do. After all, I was working for Red Hat when I started on
cleaning up these drivers.

--
dwmw2

2008-07-04 20:52:05

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Arjan van de Ven <[email protected]>
Date: Fri, 4 Jul 2008 13:42:08 -0700

> On Fri, 04 Jul 2008 13:37:21 -0700 (PDT)
> David Miller <[email protected]> wrote:
>
> > It seems your employer is telling you to work on this in order to sort
> > out some perceived legal issue. And that's the only reason you're
> > investing any effort into this.
>
> I assume you meant Davids previous employer (Red Hat) with this, not
> his current one (Intel). I seriously doubt (and know for sure we never
> asked/told David anything around this) that Intel cares about what
> happens in tg3.

Yet that is exactly the impression that I have gotten over
all of the communication I've received.

And yes I am more than well aware of who David's current
and former employers are. :)

2008-07-04 20:53:16

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: David Woodhouse <[email protected]>
Date: Fri, 04 Jul 2008 21:43:38 +0100

> He must do. After all, I was working for Red Hat when I started on
> cleaning up these drivers.

Then I hope you've caught the e100 ucode in your changes, for
the sake of full transparency :-)

2008-07-04 20:54:18

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 13:37 -0700, David Miller wrote:
> And for one, I have consistently argued that this "best practice" is
> the "worst practice" from a technical perspective. It is the worst
> because it means mistakes are possible to make between driver and
> firmware versions. Even with versioning it is not fool proof.
> Whereas if you link the firmware into the driver, it's impossible to
> get wrong.

And you've been wrong about that from the start, which is why the rest
of the kernel has moved on while the drivers you control are left
behind.

But we've already stopped working on drivers/net; we're fixing the rest
of the older drivers first. And every affected maintainer except you and
Jeff seems _happy_ to see their drivers being brought up to date.

--
dwmw2

2008-07-04 20:59:34

by Arjan van de Ven

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008 13:51:50 -0700 (PDT)
David Miller <[email protected]> wrote:

> From: Arjan van de Ven <[email protected]>
> Date: Fri, 4 Jul 2008 13:42:08 -0700
>
> > On Fri, 04 Jul 2008 13:37:21 -0700 (PDT)
> > David Miller <[email protected]> wrote:
> >
> > > It seems your employer is telling you to work on this in order to
> > > sort out some perceived legal issue. And that's the only reason
> > > you're investing any effort into this.
> >
> > I assume you meant Davids previous employer (Red Hat) with this, not
> > his current one (Intel). I seriously doubt (and know for sure we
> > never asked/told David anything around this) that Intel cares about
> > what happens in tg3.
>
> Yet that is exactly the impression that I have gotten over
> all of the communication I've received.
>

then I'd like to set that impression straight (and burry the
conspiracy theories)... I've never asked David to do this for any kind
of legal theory or otherwise. Any of it, tg3 or otherwise. And while I
can't speak for Intel on legal aspects (if there are any here), I can
speak as Davids manager that this entire project hasn't originated from
anything we (Intel) asked him to do.

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-07-04 21:06:28

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 13:52 -0700, David Miller wrote:
> From: David Woodhouse <[email protected]>
> Date: Fri, 04 Jul 2008 21:43:38 +0100
>
> > He must do. After all, I was working for Red Hat when I started on
> > cleaning up these drivers.
>
> Then I hope you've caught the e100 ucode in your changes, for
> the sake of full transparency :-)

Yes, I have. I sincerely hope that patch was posted for review;
certainly it _should_ have been. If not, I apologise. It's in
linux-next.

But as I said, I've stopped working on drivers/net/ for now; we're
concentrating on the rest of the kernel where the maintainers are
_happy_ to be brought up to date.

The intention was always that this set of patches should be obviously
correct and uncontentious, without inflaming the religious nutters on
_either_ side of the debate. The fact that there that the most vocal of
the fanatics on _both_ sides are flaming about the sensible middle
ground, where I'm just consolidating what has been common practice for
ages _anyway_, is rather bemusing to me...

--
dwmw2

2008-07-04 21:12:25

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 2008-07-04 at 13:59 -0700, Arjan van de Ven wrote:
> On Fri, 04 Jul 2008 13:51:50 -0700 (PDT)
> David Miller <[email protected]> wrote:
> > Yet that is exactly the impression that I have gotten over
> > all of the communication I've received.
> >
>
> then I'd like to set that impression straight (and burry the
> conspiracy theories)... I've never asked David to do this for any kind
> of legal theory or otherwise. Any of it, tg3 or otherwise. And while I
> can't speak for Intel on legal aspects (if there are any here), I can
> speak as Davids manager that this entire project hasn't originated from
> anything we (Intel) asked him to do.

I wouldn't worry, Arjan. There is no basis for Dave's claim, and he
knows perfectly well I was working on it before I was working for Intel.

It's perfectly sensible janitorial-type work which has needed doing for
ages, and which I got interested in after a discussion about the kernel
that Fedora ships.

The Fedora Engineering Steering Committee (of which I'm a member) agreed
that Fedora would _like_ to ship a firmware-less kernel, if that was
technically feasible (and didn't involve actually breaking drivers).

--
dwmw2

2008-07-04 21:24:48

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> External firmware is by design an error prone system, even with
> versioning. But by being built and linked into the driver, it
> is fool proof.
>
> On a technical basis alone, we would never disconnect a crucial
> component such as firmware, from the driver. The only thing
> charging these transoformations, from day one, is legal concerns.

As I said: We had this argument ten years ago (more than that now
actually). People said the same thing about modules.

Alan

2008-07-04 21:29:06

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> It seems your employer is telling you to work on this in order to sort
> out some perceived legal issue. And that's the only reason you're
> investing any effort into this.

To point out the blindingly obvious

1. - David has had *two* different employers while working on this project

2. - I think you could reasonably assume that if your employers legal
team had reached a specific conclusion that there was a compliance issue
they would have directed you to do something about it as well, not
operated a mysterious conspiracy (in conjunction one assumes with David's
current employer) to change it.

And it's worth remembering that the 'lunatic fringe' you seem to be
trying to associate David with don't care about the firmware changes
because their view is that you shouldn't even have drivers for such
hardware as it cannot be used in a free way (just as they argue not to
write java without free interpreters). [1]

Alan
[1] No I don't know where they get their free x86 engine from either ;)

2008-07-04 21:35:51

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008 15:42:37 +0100, David Woodhouse <[email protected]> wrote:

>On Fri, 2008-07-04 at 16:39 +0200, Hannes Reinecke wrote:
>> Firmware loading is just a matter of copying the file at the correct
>> location (ie /lib/firmware) and with the name the fw loader expects.
>> mkinitrd should do it correctly.
>> But I wasn't aware that the tg3 has external firmware, so I doubt
>> we have any rpm for it.
>
>It doesn't yet; that patch is in linux-next. The firmware is shipped as
>part of the kernel source tree, and you currently need to run 'make
>firmware_install' to put it in /lib/firmware, although we're looking at
>making that easier because apparently having to run 'make
>firmware_install' is too hard...

Oh come on now -- not a case of too hard, it's a case of breaking _all_
existing kernel build scripts.

Grant.

2008-07-04 22:08:42

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 2008-07-05 at 07:34 +1000, Grant Coady wrote:
> On Fri, 04 Jul 2008 15:42:37 +0100, David Woodhouse <[email protected]> wrote:
>
> >On Fri, 2008-07-04 at 16:39 +0200, Hannes Reinecke wrote:
> >> Firmware loading is just a matter of copying the file at the correct
> >> location (ie /lib/firmware) and with the name the fw loader expects.
> >> mkinitrd should do it correctly.
> >> But I wasn't aware that the tg3 has external firmware, so I doubt
> >> we have any rpm for it.
> >
> >It doesn't yet; that patch is in linux-next. The firmware is shipped as
> >part of the kernel source tree, and you currently need to run 'make
> >firmware_install' to put it in /lib/firmware, although we're looking at
> >making that easier because apparently having to run 'make
> >firmware_install' is too hard...
>
> Oh come on now -- not a case of too hard, it's a case of breaking _all_
> existing kernel build scripts.

It's done as part of 'make modules_install' now anyway.

--
dwmw2

2008-07-04 22:49:45

by Mariusz Kozlowski

[permalink] [raw]

Subject: Re: 2.6.26-rc8-mm1: unable to mount nfs shares

$ mount some/nfs/share
mount.nfs: Input/output error

dmesg says: RPC: transport (0) not supported

but I guess it's known issue http://lkml.org/lkml/2008/7/1/438 ?

Mariusz

2008-07-04 23:07:29

[permalink] [raw]

Subject: Re: 2.6.26-rc8-mm1: unable to mount nfs shares

On Sat, 5 Jul 2008 00:49:33 +0200 Mariusz Kozlowski <[email protected]> wrote:

> $ mount some/nfs/share
> mount.nfs: Input/output error
>
> dmesg says: RPC: transport (0) not supported
>
> but I guess it's known issue http://lkml.org/lkml/2008/7/1/438 ?
>

OK, thanks, I put Trond's fix into hot-fixes/

--- a/fs/nfs/super.c~nfs-fix-the-mount-protocol-defaults-for-binary-mounts
+++ a/fs/nfs/super.c
@@ -1571,6 +1571,7 @@ static int nfs_validate_mount_data(void

if (!(data->flags & NFS_MOUNT_TCP))
args->nfs_server.protocol = XPRT_TRANSPORT_UDP;
+ nfs_set_transport_defaults(args);
/* N.B. caller will free nfs_server.hostname in all cases */
args->nfs_server.hostname = kstrdup(data->hostname, GFP_KERNEL);
args->namlen = data->namlen;
_

2008-07-04 23:13:37

by Henrique de Moraes Holschuh

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, Jul 04, 2008 at 03:42:37PM +0100, David Woodhouse wrote:
> It doesn't yet; that patch is in linux-next. The firmware is shipped as
> part of the kernel source tree, and you currently need to run 'make
> firmware_install' to put it in /lib/firmware, although we're looking at
> making that easier because apparently having to run 'make
> firmware_install' is too hard...

Won't that break multiple kernel installs on any binary packaging
system that cares about file collisions? Multiple kernel rpms
providing the same /lib/firmware files would break things wouldn't
they ?

OG.

2008-07-04 23:59:37

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 05 Jul 2008, Olivier Galibert wrote:
> On Fri, Jul 04, 2008 at 03:42:37PM +0100, David Woodhouse wrote:
> > It doesn't yet; that patch is in linux-next. The firmware is shipped as
> > part of the kernel source tree, and you currently need to run 'make
> > firmware_install' to put it in /lib/firmware, although we're looking at
> > making that easier because apparently having to run 'make
> > firmware_install' is too hard...
>
> Won't that break multiple kernel installs on any binary packaging
> system that cares about file collisions? Multiple kernel rpms
> providing the same /lib/firmware files would break things wouldn't
> they ?

Correct.

We will probably need per-kernel directories, exactly like what is done for
modules. And since there are (now) both kernel-version-specific, and
non-kernel-version-specific firmware, this means the firmware loader should
look first on the version-specific directory (say, /lib/firmware/$(uname
-r)/), then if not found, on the general directory (/lib/firmware).

Nothing too dificult to pull off, but something that needs to be done before
this entire thing gets deployed on unsuspecting users. It is bad enough
that it created such a commotion when deployed on unsuspecting developers...

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2008-07-05 00:07:04

by Gene Heskett

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Friday 04 July 2008, Grant Coady wrote:
>On Fri, 04 Jul 2008 15:42:37 +0100, David Woodhouse <[email protected]>
wrote:
>>On Fri, 2008-07-04 at 16:39 +0200, Hannes Reinecke wrote:
>>> Firmware loading is just a matter of copying the file at the correct
>>> location (ie /lib/firmware) and with the name the fw loader expects.
>>> mkinitrd should do it correctly.
>>> But I wasn't aware that the tg3 has external firmware, so I doubt
>>> we have any rpm for it.
>>
>>It doesn't yet; that patch is in linux-next. The firmware is shipped as
>>part of the kernel source tree, and you currently need to run 'make
>>firmware_install' to put it in /lib/firmware, although we're looking at
>>making that easier because apparently having to run 'make
>>firmware_install' is too hard...
>
>Oh come on now -- not a case of too hard, it's a case of breaking _all_
>existing kernel build scripts.
>
1. Something in the header of this email is preventing me from replying to the
list, all I can get is your personal address if I try, Grant. However, kmail
can edit the to: and cc: lists so its fixed.

2. I am intimately familiar with my own build script, and I have a handy copy of
vim, so if the info to do the edit is available, this excuse won't hold enough
water to measure.

>Grant.
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/

--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
Solipsists of the World... you are already united.
-- Kayvan Sylvan

2008-07-05 00:53:46

by Trent Piepho

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 4 Jul 2008, Henrique de Moraes Holschuh wrote:
> On Sat, 05 Jul 2008, Olivier Galibert wrote:
>> Won't that break multiple kernel installs on any binary packaging
>> system that cares about file collisions? Multiple kernel rpms
>> providing the same /lib/firmware files would break things wouldn't
>> they ?
>
> We will probably need per-kernel directories, exactly like what is done for
> modules. And since there are (now) both kernel-version-specific, and
> non-kernel-version-specific firmware, this means the firmware loader should
> look first on the version-specific directory (say, /lib/firmware/$(uname
> -r)/), then if not found, on the general directory (/lib/firmware).

How about /lib/modules/`uname -r`/firmware

Keeps all the stuff for a given kernel together in one directory. Easier to
delete, e.g. when getting ride of an old kernel or when wiping a broken kernel
install clean. The non-kernel-specific directory could be for firmwares that
don't come with the kernel and aren't specific to the driver version. That
avoids the complexity of providing kernel version specific packages when it's
not necessary.

2008-07-05 03:52:31

by Henrique de Moraes Holschuh

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008, Trent Piepho wrote:
> On Fri, 4 Jul 2008, Henrique de Moraes Holschuh wrote:
> > On Sat, 05 Jul 2008, Olivier Galibert wrote:
> >> Won't that break multiple kernel installs on any binary packaging
> >> system that cares about file collisions? Multiple kernel rpms
> >> providing the same /lib/firmware files would break things wouldn't
> >> they ?
> >
> > We will probably need per-kernel directories, exactly like what is done for
> > modules. And since there are (now) both kernel-version-specific, and
> > non-kernel-version-specific firmware, this means the firmware loader should
> > look first on the version-specific directory (say, /lib/firmware/$(uname
> > -r)/), then if not found, on the general directory (/lib/firmware).
>
> How about /lib/modules/`uname -r`/firmware

I am fine with it, it certainly has a few advantages.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2008-07-05 03:55:05

[permalink] [raw]

Subject: Re: memcg: lru scan fix (Was: 2.6.26-rc8-mm1

On Fri, 4 Jul 2008 12:24:59 -0700
Andrew Morton <[email protected]> wrote:

> On Fri, 4 Jul 2008 15:16:56 -0400 Rik van Riel <[email protected]> wrote:
>
> > On Fri, 4 Jul 2008 18:02:26 +0900
> > KAMEZAWA Hiroyuki <[email protected]> wrote:
> >
> > > Index: test-2.6.26-rc8-mm1/mm/vmscan.c
> > > ===================================================================
> > > --- test-2.6.26-rc8-mm1.orig/mm/vmscan.c
> > > +++ test-2.6.26-rc8-mm1/mm/vmscan.c
> > > @@ -1501,6 +1501,8 @@ static unsigned long shrink_zone(int pri
> > > */
> > > if (scan_global_lru(sc) && inactive_anon_is_low(zone))
> > > shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
> > > + else if (!scan_global_lru(sc))
> > > + shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
> >
> > Makes sense.
> >
> > Acked-by: Rik van Riel <[email protected]>
> >
>
> Thanks. Poor old me needs to work out which patch this patch fixes.
> It's always appreciated if others tell me :)
>

Maybe mine is against this one.
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.26-rc8/2.6.26-rc8-mm1/broken-out/vmscan-second-chance-replacement-for-anonymous-pages.patch

This adds inactive_anon_is_low() logic.

Thanks,
-Kame

2008-07-05 04:05:49

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> But we've already stopped working on drivers/net; we're fixing the rest
> of the older drivers first. And every affected maintainer except you and
> Jeff seems _happy_ to see their drivers being brought up to date.

Disliking breakage != unhappy at driver change

Since you apparently still do not see the difference between a
breakage-filled path to goodness, and goodness itself.

Jeff

2008-07-05 04:07:28

by Valdis Klētnieks

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008 13:52:58 PDT, David Miller said:
> From: David Woodhouse <[email protected]>
> Date: Fri, 04 Jul 2008 21:43:38 +0100
>
> > He must do. After all, I was working for Red Hat when I started on
> > cleaning up these drivers.
>
> Then I hope you've caught the e100 ucode in your changes, for
> the sake of full transparency :-)

Yes, he did - somebody was asking WTF happened to the e100 driver. :)

Attachments:

(No filename) (226.00 B)

2008-07-05 04:11:09

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Henrique de Moraes Holschuh wrote:
> Nothing too dificult to pull off, but something that needs to be done before
> this entire thing gets deployed on unsuspecting users. It is bad enough
> that it created such a commotion when deployed on unsuspecting developers...

If I may... that summarizes my entire position in this thread.

The current mess is light years from being deployable to average users.

Jeff

2008-07-05 04:36:23

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Theodore Tso wrote:
> On Fri, Jul 04, 2008 at 09:39:36AM -0400, Jeff Garzik wrote:
>> You have been told repeatedly that cp(1) and scp(1) are commonly used to
>> transport the module David and I care about -- tg3. It's been a single
>> file module since birth, and people take advantage of that fact.
>
> Here, I think I'll have to respectly disagree with you and say that
> you are taking things too far. I don't think scp'ing individual
> modules around counts as an "exported user interface" the same way,
> say "make install; make modules_install" is a commonly understand and
> used interface by users and scripts (i.e., such as Debian's make-kpkg,
> which does NOT know about "make firmware_install", BTW).

It's not just netdev developers that use that method, root (notably
router) image and driver disk build scripts use it too. They've been
able to skate around module dependencies because network drivers rarely
have module dependencies or require big multi-module systems.

Example -- the driver disk kit that RH informally gave out, which was
widely used, but does not use normal kernel build processes:

http://people.redhat.com/dledford/mod_devel_kit.tgz

Even if one modifies 'make modules_install' as discussed[1], kits like
these will report "100% success! driver disk created", yet ship a dead
driver disk.

That is why putting the firmware in the kernel image, as dwmw2 has done,
does not fix regressions here: driver disk authors do not necessarily
have the luxury of updating the kernel.

Conclusion - we should not build a system today that /excludes/ the
possibility of building drivers as they are built today -- with the
firmware inside the module [if CONFIG_FOO=m] or kernel image [if
CONFIG_FOO=y].

That is the only path that gives everyone a chance to deal with this
transition.

Jeff

[1] a laudable and useful thing to do, and it sounds like it is being
accomplished. great!

2008-07-05 05:41:26

by Balbir Singh

[permalink] [raw]

Subject: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

KAMEZAWA Hiroyuki wrote:
> My swapcache accounting under memcg patch failed to catch tmpfs(shmem)'s one.
> Can I test this under -mm tree ?
> (If -mm is busy, I'm not in hurry.)
> This patch works well in my box.
> =
> SwapCache handling fix.
>
> shmem's swapcache behavior is a little different from anonymous's one and
> memcg failed to handle it. This patch tries to fix it.
>
> After this:
>
> Any page marked as SwapCache is not uncharged. (delelte_from_swap_cache()
> delete the SwapCache flag.)
>
> To check a shmem-page-cache is alive or not we use
> page->mapping && !PageAnon(page) instead of
> pc->flags & PAGE_CGROUP_FLAG_CACHE.
>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>

Though I am not opposed to this, I do sit up and think if keeping the reference
count around could avoid this complexity and from my point, the maintenance
overhead of this logic/code (I fear there might be more special cases :( )

The trade-off is complexity versus the overhead of reference counting.

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-07-05 05:57:20

by Jaswinder Singh Rajput

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Respected Sirs,

On Sat, Jul 5, 2008 at 2:07 AM, David Miller <[email protected]>
wrote:
> From: David Woodhouse <[email protected]>
> Date: Fri, 04 Jul 2008 14:27:15 +0100
>
>> Your argument makes about as much sense as an argument that we should
>> link b43.ko with mac80211.ko so that the 802.11 core code "rides
along
>> in the module's .ko file". It's just silly.
>
> I totally disagree with you. Jeff is right and you are wrong.
>

Please let me know, if you found the BUG in the tg3 firmare patch :-
http://git.infradead.org/users/dwmw2/firmware-2.6.git?a=commitdiff;h=be4e9388e35b22d6f8aa104baf39f8339825424e

I will try to fix it.

Thank you,

Jaswinder Singh.

2008-07-05 06:02:15

by Bill Fink

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 5 Jul 2008, Henrique de Moraes Holschuh wrote:

> On Fri, 04 Jul 2008, Trent Piepho wrote:
> > On Fri, 4 Jul 2008, Henrique de Moraes Holschuh wrote:
> > > On Sat, 05 Jul 2008, Olivier Galibert wrote:
> > >> Won't that break multiple kernel installs on any binary packaging
> > >> system that cares about file collisions? Multiple kernel rpms
> > >> providing the same /lib/firmware files would break things wouldn't
> > >> they ?
> > >
> > > We will probably need per-kernel directories, exactly like what is done for
> > > modules. And since there are (now) both kernel-version-specific, and
> > > non-kernel-version-specific firmware, this means the firmware loader should
> > > look first on the version-specific directory (say, /lib/firmware/$(uname
> > > -r)/), then if not found, on the general directory (/lib/firmware).
> >
> > How about /lib/modules/`uname -r`/firmware
>
> I am fine with it, it certainly has a few advantages.

Why not put it in the same /lib/modules directory as the foo.ko
kernel module itself? Then those who like to scp kernel modules
around (which I've done myself on occasion) just need to learn
to scp foo.* instead of foo.ko. Why replicate a separate
/lib/modules/`uname -r`/firmware directory?

-Bill

2008-07-05 06:06:33

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Miller wrote:
> External firmware is by design an error prone system, even with
> versioning. But by being built and linked into the driver, it
> is fool proof.
>
> On a technical basis alone, we would never disconnect a crucial
> component such as firmware, from the driver. The only thing
> charging these transoformations, from day one, is legal concerns.
>
> I've been against request_firmware() from the beginning, because
> they make life unnecessarily difficult, and it is error prone no
> matter how well you design the validation step.

Precisely. External firmware is quite simply less error prone, since it
is always with the driver code that uses it. No other system can
approach that reliability.

But I did (and do) think request_firmware() is a necessary piece of the
puzzle. Personally I've always felt it is a design choice by the
individual driver author, whether to compile-in firmware or use external
firmware.

Jeff

2008-07-05 06:06:56

[permalink] [raw]

Subject: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

On Sat, 05 Jul 2008 11:11:10 +0530
Balbir Singh <[email protected]> wrote:

> KAMEZAWA Hiroyuki wrote:
> > My swapcache accounting under memcg patch failed to catch tmpfs(shmem)'s one.
> > Can I test this under -mm tree ?
> > (If -mm is busy, I'm not in hurry.)
> > This patch works well in my box.
> > =
> > SwapCache handling fix.
> >
> > shmem's swapcache behavior is a little different from anonymous's one and
> > memcg failed to handle it. This patch tries to fix it.
> >
> > After this:
> >
> > Any page marked as SwapCache is not uncharged. (delelte_from_swap_cache()
> > delete the SwapCache flag.)
> >
> > To check a shmem-page-cache is alive or not we use
> > page->mapping && !PageAnon(page) instead of
> > pc->flags & PAGE_CGROUP_FLAG_CACHE.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
>
> Though I am not opposed to this, I do sit up and think if keeping the reference
> count around could avoid this complexity and from my point, the maintenance
> overhead of this logic/code (I fear there might be more special cases :( )

yes, to me. but we have to fix..

But I don't like old code's refcnt handling which does
- increment
- does this increment was really neccesary ?
No? ok, decrement it again.

This was much more complex to me than current code.

And old ones will needs the check at treating swap-cache. (it couldn't but if we want)

>
> The trade-off is complexity versus the overhead of reference counting.
>
refcnt was also very complex ;)

Thanks,
-Kame

> --
> Warm Regards,
> Balbir Singh
> Linux Technology Center
> IBM, ISTL
>

2008-07-05 06:14:25

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> I'm working on making the required firmware get installed as part of
> 'make modules_install'.

Great! That will definitely reduce silent regressions due to build
process changes.

Jeff

2008-07-05 06:45:31

by Jaswinder Singh Rajput

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Respected Jeff,

On Sat, Jul 5, 2008 at 11:44 AM, Jeff Garzik <[email protected]> wrote:
> David Woodhouse wrote:
>>
>> I'm working on making the required firmware get installed as part of
>> 'make modules_install'.
>
> Great! That will definitely reduce silent regressions due to build process
> changes.

If other issues are solved, please look at the code of TG3 firmware Patch :-

http://git.infradead.org/users/dwmw2/firmware-2.6.git?a=commitdiff;h=be4e9388e35b22d6f8aa104baf39f8339825424e

And give us your valuable feedback regarding code :)

Can I move to another net drivers ?

Thank you,

Jaswinder Singh.

2008-07-05 06:49:29

by Balbir Singh

[permalink] [raw]

Subject: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

KAMEZAWA Hiroyuki wrote:
> On Sat, 05 Jul 2008 11:11:10 +0530
> Balbir Singh <[email protected]> wrote:
>
>> KAMEZAWA Hiroyuki wrote:
>>> My swapcache accounting under memcg patch failed to catch tmpfs(shmem)'s one.
>>> Can I test this under -mm tree ?
>>> (If -mm is busy, I'm not in hurry.)
>>> This patch works well in my box.
>>> =
>>> SwapCache handling fix.
>>>
>>> shmem's swapcache behavior is a little different from anonymous's one and
>>> memcg failed to handle it. This patch tries to fix it.
>>>
>>> After this:
>>>
>>> Any page marked as SwapCache is not uncharged. (delelte_from_swap_cache()
>>> delete the SwapCache flag.)
>>>
>>> To check a shmem-page-cache is alive or not we use
>>> page->mapping && !PageAnon(page) instead of
>>> pc->flags & PAGE_CGROUP_FLAG_CACHE.
>>>
>>> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
>> Though I am not opposed to this, I do sit up and think if keeping the reference
>> count around could avoid this complexity and from my point, the maintenance
>> overhead of this logic/code (I fear there might be more special cases :( )
>
> yes, to me. but we have to fix..
>
> But I don't like old code's refcnt handling which does
> - increment
> - does this increment was really neccesary ?
> No? ok, decrement it again.
>
> This was much more complex to me than current code.
>

That can be redone -- the moment a page is used by a path, refcnt (increment)
it. Undo the same when the page is no longer in use.

I expect

rmap path to increment/decrement it on mapping
radix-tree (cache's) to do the same

Using a kref we should be able to get this logic right - no?

> And old ones will needs the check at treating swap-cache. (it couldn't but if we want)
>
>> The trade-off is complexity versus the overhead of reference counting.
>>
> refcnt was also very complex ;)

I think that is easier to simply, instead of adding the complex checks we have
right now. refcnt is easier to prove as working correct than the checks.

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-07-05 07:42:20

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

At Sat, 5 Jul 2008 01:13:22 +0200,
Olivier Galibert wrote:
>
> On Fri, Jul 04, 2008 at 03:42:37PM +0100, David Woodhouse wrote:
> > It doesn't yet; that patch is in linux-next. The firmware is shipped as
> > part of the kernel source tree, and you currently need to run 'make
> > firmware_install' to put it in /lib/firmware, although we're looking at
> > making that easier because apparently having to run 'make
> > firmware_install' is too hard...
>
> Won't that break multiple kernel installs on any binary packaging
> system that cares about file collisions? Multiple kernel rpms
> providing the same /lib/firmware files would break things wouldn't
> they ?

Yes, it will, if the firmware blobs are packed into the kernel
package. In a long term, we can put firmware files into a separate,
architecture independent noarch package, though. This will save the
total package size, too.

But, right now, it's difficult because the installation and build of
firmware files depend on the kernel config. We'd need a make rule for
installing the all firmware files for that purpose.

Takashi

2008-07-05 08:17:01

[permalink] [raw]

Subject: Re: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

----- Original Message -----
>Date: Sat, 05 Jul 2008 12:19:11 +0530
>From: Balbir Singh <[email protected]>
>To: KAMEZAWA Hiroyuki <[email protected]>
>CC: Andrew Morton <[email protected]>, [email protected],
> [email protected], "[email protected]" <[email protected]>,
> "[email protected]" <[email protected]>,
> "[email protected]" <[email protected]>
>Subject: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1
>
>
>KAMEZAWA Hiroyuki wrote:
>> On Sat, 05 Jul 2008 11:11:10 +0530
>> Balbir Singh <[email protected]> wrote:
>>
>>> KAMEZAWA Hiroyuki wrote:
>>>> My swapcache accounting under memcg patch failed to catch tmpfs(shmem)'s
one.
>>>> Can I test this under -mm tree ?
>>>> (If -mm is busy, I'm not in hurry.)
>>>> This patch works well in my box.
>>>> =
>>>> SwapCache handling fix.
>>>>
>>>> shmem's swapcache behavior is a little different from anonymous's one and
>>>> memcg failed to handle it. This patch tries to fix it.
>>>>
>>>> After this:
>>>>
>>>> Any page marked as SwapCache is not uncharged. (delelte_from_swap_cache()
>>>> delete the SwapCache flag.)
>>>>
>>>> To check a shmem-page-cache is alive or not we use
>>>> page->mapping && !PageAnon(page) instead of
>>>> pc->flags & PAGE_CGROUP_FLAG_CACHE.
>>>>
>>>> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
>>> Though I am not opposed to this, I do sit up and think if keeping the refe
rence
>>> count around could avoid this complexity and from my point, the maintenanc
e
>>> overhead of this logic/code (I fear there might be more special cases :( )
>>
>> yes, to me. but we have to fix..
>>
>> But I don't like old code's refcnt handling which does
>> - increment
>> - does this increment was really neccesary ?
>> No? ok, decrement it again.
>>
>> This was much more complex to me than current code.
>>
At first, what I have to say is
"this is a fix against handle-swapcache patch not against remove-refcnt"

This complex comes from handle-swapcache. (But it's necessary.)

>
>That can be redone -- the moment a page is used by a path, refcnt (increment)
>it. Undo the same when the page is no longer in use.
>
>I expect
>
>rmap path to increment/decrement it on mapping
>radix-tree (cache's) to do the same
>
>
>Using a kref we should be able to get this logic right - no?
>
no
What the old code does was

- a page is added to rmap (mapcount 0->1) +1
- a page is removed from rmap (mapcount ->0) -1
- a page is added to radix-tree (+1)
- a page is removed from radix-tree (-1)

All information is recorded in struct page because it exists for.
Then, why duplicates information ? It's usually bad habit.

>> And old ones will needs the check at treating swap-cache. (it couldn't but
if we want)
>>
>>> The trade-off is complexity versus the overhead of reference counting.
>>>
>> refcnt was also very complex ;)
>
>I think that is easier to simply, instead of adding the complex checks we hav
e
>right now. refcnt is easier to prove as working correct than the checks.

About swap-cache, refcnt is just obstacle because you can't handle
add-to-swapcache by refcnt.

If you want to add refcnt (or some code) for "debug", I have no objection.

Thanks,
-Kame

2008-07-05 08:50:43

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 2008-07-05 at 09:41 +0200, Takashi Iwai wrote:
> At Sat, 5 Jul 2008 01:13:22 +0200,
> Olivier Galibert wrote:
> >
> > On Fri, Jul 04, 2008 at 03:42:37PM +0100, David Woodhouse wrote:
> > > It doesn't yet; that patch is in linux-next. The firmware is shipped as
> > > part of the kernel source tree, and you currently need to run 'make
> > > firmware_install' to put it in /lib/firmware, although we're looking at
> > > making that easier because apparently having to run 'make
> > > firmware_install' is too hard...
> >
> > Won't that break multiple kernel installs on any binary packaging
> > system that cares about file collisions? Multiple kernel rpms
> > providing the same /lib/firmware files would break things wouldn't
> > they ?
>
> Yes, it will, if the firmware blobs are packed into the kernel
> package. In a long term, we can put firmware files into a separate,
> architecture independent noarch package, though. This will save the
> total package size, too.

I'm not familiar with the SuSE kernel specfile, but it was a fairly
minor change to the Fedora kernel to do exactly that 'long term' thing.
The patch is in the fedora-kernel-list archives.

We have to do the same thing with exported headers, anyway -- those are
built from the kernel too, but again we need to have only one copy
installed or we'd get conflicts.

> But, right now, it's difficult because the installation and build of
> firmware files depend on the kernel config. We'd need a make rule for
> installing the all firmware files for that purpose.

That's what 'make firmware_install' does. It's config- and arch-
independent, mostly because it was developed with distributors in mind.

--
dwmw2

2008-07-05 10:26:53

by maximilian attems

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 04 Jul 2008, Theodore Tso wrote:

> On Fri, Jul 04, 2008 at 04:24:03PM +0200, maximilian attems wrote:
> > yes i'd call them severly broken.
> > as it is quite easy to pick up the modinfo firmware module output.
> >
> > their trouble is that they sync initramfs-tools from Debian only
> > once every 2 years or so.
>
> Well, Takashi just mentioned that SuSE's mkinitrd may not handle
> /lib/firmware correctly either....

i have good news: in intrepid they synced initramfs-tools,
so it is fixed for their upcoming release. as it is fixed
in Debian for Lenny. they went for the /lib/firmware/${version}
choice.

--
maks

2008-07-05 10:53:35

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, Jul 05, 2008 at 09:41:56AM +0200, Takashi Iwai wrote:
> Yes, it will, if the firmware blobs are packed into the kernel
> package. In a long term, we can put firmware files into a separate,
> architecture independent noarch package, though. This will save the
> total package size, too.

That could be interestingly hard, actually. Right now the kernel
package is one of these packages designed so that multiple versions
can be installed together. When the version of one of the firmwares
changes, the firmware package will have to be updated. But will it
keep the previous version? If it doesn't, the possibly still
installed older kernels won't work anymore. If it does, it will
accumulate a lot of files over time...

OG.

2008-07-05 11:22:36

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Olivier Galibert wrote:
> On Sat, Jul 05, 2008 at 09:41:56AM +0200, Takashi Iwai wrote:
>> Yes, it will, if the firmware blobs are packed into the kernel
>> package. In a long term, we can put firmware files into a separate,
>> architecture independent noarch package, though. This will save the
>> total package size, too.
>
> That could be interestingly hard, actually. Right now the kernel
> package is one of these packages designed so that multiple versions
> can be installed together. When the version of one of the firmwares
> changes, the firmware package will have to be updated. But will it
> keep the previous version? If it doesn't, the possibly still
> installed older kernels won't work anymore. If it does, it will
> accumulate a lot of files over time...

Many distribution have some way for separate kernel module packages.
It's essentially the same problem so it should be already solved
in some way. Also there are already drivers who rely on separate
firmware so they already should have this problem (and hopefully
a solution). Of course these existing solutions might not be good
enough. And a lot of people ignored them because they didn't use
these external packages and drivers with those requirements.

I agree with you that doing this for more drivers will be a further complication
and the rationale why this complication is needed for drivers like tg3 or
e100 so far didn't sound very convincing to me. If I read it correctly it
was "some other drivers do it this way so let's complicate everybody and
put them on the same level". Perhaps I'm dense, but I failed
to see the convincing reason in that.

On the other hand for my personal use it this change should be
transparent, at least as long as "make firmware_install"
will be integrated into "make modules_install" and put the files somewhere where
they don't conflict with other versions.

-Andi

2008-07-05 12:02:34

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, Jul 05, 2008 at 01:22:20PM +0200, Andi Kleen wrote:
> Many distribution have some way for separate kernel module packages.
> It's essentially the same problem so it should be already solved
> in some way.

Errr, no. Modules go in /lib/modules/`uname -r`, so no conflict.

OG.

2008-07-05 12:10:18

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Olivier Galibert wrote:
> On Sat, Jul 05, 2008 at 01:22:20PM +0200, Andi Kleen wrote:
>> Many distribution have some way for separate kernel module packages.
>> It's essentially the same problem so it should be already solved
>> in some way.
>
> Errr, no. Modules go in /lib/modules/`uname -r`, so no conflict.

Well that's the only sane way to store the firmware anyways (otherwise
you could never keep kernel versions which need different firmware installed)

While the current code doesn't do that there have been proposals for that
and I assume/hope they will be acted on.

-Andi

2008-07-05 12:16:59

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 2008-07-05 at 14:09 +0200, Andi Kleen wrote:
> Olivier Galibert wrote:
> > On Sat, Jul 05, 2008 at 01:22:20PM +0200, Andi Kleen wrote:
> >> Many distribution have some way for separate kernel module packages.
> >> It's essentially the same problem so it should be already solved
> >> in some way.
> >
> > Errr, no. Modules go in /lib/modules/`uname -r`, so no conflict.
>
> Well that's the only sane way to store the firmware anyways (otherwise
> you could never keep kernel versions which need different firmware installed)

It almost never happens that you have kernel versions which _need_
different firmware installed. In almost all cases, the older driver will
continue to work just fine with the newer firmware (and its bug-fixes).

The ABI between driver and firmware rarely changes in such a fashion
that you have to update the driver in lock-step -- and even on the
occasions that it does, it's not hard to simply change the name of the
"new-style" firmware so that it doesn't stomp on the old one (Think of
it like an soname).

--
dwmw2

2008-07-05 12:23:35

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Sat, 2008-07-05 at 14:09 +0200, Andi Kleen wrote:
>> Olivier Galibert wrote:
>>> On Sat, Jul 05, 2008 at 01:22:20PM +0200, Andi Kleen wrote:
>>>> Many distribution have some way for separate kernel module packages.
>>>> It's essentially the same problem so it should be already solved
>>>> in some way.
>>> Errr, no. Modules go in /lib/modules/`uname -r`, so no conflict.
>> Well that's the only sane way to store the firmware anyways (otherwise
>> you could never keep kernel versions which need different firmware installed)
>
> It almost never happens that you have kernel versions which _need_
> different firmware installed. In almost all cases, the older driver will
> continue to work just fine with the newer firmware (and its bug-fixes).

That's a lot of "should" and "in most cases" and "in a ideal world". What happens
when the new firmware is buggy for example and prevents booting of the system?
And in the cases where it doesn't work like you describe?

Simply overwriting it means there would be no simple way back.

Even if it breaks for only 1% of the users that would be pretty bad for Linux.

Besides it means the possible combinations that have to be tested
increases significantly.

Doesn't sound like a good plan to me. Besides the problems
that it would causes for package management of course (you would
force separate packages on everybody versus just keeping the firmware
in the kernel package)

-Andi

2008-07-05 12:43:37

by Henrique de Moraes Holschuh

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 2008-07-05 at 14:23 +0200, Andi Kleen wrote:
> That's a lot of "should" and "in most cases" and "in a ideal world".

OK, let's phrase it differently:

It almost never happens, and it's trivial to handle it safely in the
extremely rare cases that it does. We don't need to start putting
firmware in /lib/firmware/`uname -r`/ to deal with it.

> What happens when the new firmware is buggy for example and prevents
> booting of the system?

If the firmware is required for booting the system, then it'll be
included in the initramfs. The one on the _real_ file system is
therefore irrelevant. When you select the last-known-good kernel from
your boot loader you'll actually get the old firmware anyway.

And given that we almost never update most of this firmware _either_, it
really isn't a problem we should be losing sleep over.

But distributors are free to shift it into /lib/firmware/`uname -r`/ if
they want to -- it's easy enough to override INSTALL_FW_PATH. For now,
though, that isn't compatible with upstream hotplug scripts and would be
a bad choice as a default.

And if a distribution which actually likes contributing its changes
upstream ever starts using /lib/firmware/`uname -r`/, then perhaps we
can discuss making it the default for the kernel too.

--
dwmw2

2008-07-05 13:09:06

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 05 Jul 2008, Bill Fink wrote:
> On Sat, 5 Jul 2008, Henrique de Moraes Holschuh wrote:
> > On Fri, 04 Jul 2008, Trent Piepho wrote:
> > > On Fri, 4 Jul 2008, Henrique de Moraes Holschuh wrote:
> > > > On Sat, 05 Jul 2008, Olivier Galibert wrote:
> > > >> Won't that break multiple kernel installs on any binary packaging
> > > >> system that cares about file collisions? Multiple kernel rpms
> > > >> providing the same /lib/firmware files would break things wouldn't
> > > >> they ?
> > > >
> > > > We will probably need per-kernel directories, exactly like what is done for
> > > > modules. And since there are (now) both kernel-version-specific, and
> > > > non-kernel-version-specific firmware, this means the firmware loader should
> > > > look first on the version-specific directory (say, /lib/firmware/$(uname
> > > > -r)/), then if not found, on the general directory (/lib/firmware).
> > >
> > > How about /lib/modules/`uname -r`/firmware
> >
> > I am fine with it, it certainly has a few advantages.
>
> Why not put it in the same /lib/modules directory as the foo.ko
> kernel module itself? Then those who like to scp kernel modules
> around (which I've done myself on occasion) just need to learn
> to scp foo.* instead of foo.ko. Why replicate a separate
> /lib/modules/`uname -r`/firmware directory?

Because a single new directory tree is easier, simpler, and less prone to
breakage to implement. This thing is way too complicated already, and
that's not good for something that must ALWAYS work right. Also, it doesn't
assume any sort of mapping between the firmware files and their users (so,
it won't ADD constraints to the firmware loading API that do not exist right
now). And it lets you version or un-version firmware files (if you *want*,
and in in *every* case), very easily, and without breaking the current ABI
(/lib/firmware/).

If I were to attempt to address your use case properly, I'd do it by
exporting the firmware dependency information on module metadata, and
add/modify userspace to tell you about it. This would let you do "scp
$(findmoduledeps --include-self themodule) foo:/tmp" and get the module, its
firmware files, its dependencies, the dependencies' firmware, and so on, so
that you'd get the entire module stack and all the firmware for the stack.
Or whatever else you want "findmoduledeps" to do, the required data would be
there for the tool to be quite versatile.

But I have zero interest on firmware loading, and I am currently taking care
of more kernel work than what I am confortable with already, so someone else
would have to do it. There are probably even better ways than the simple
one I described above, I bet...

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2008-07-05 13:57:31

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Sat, 2008-07-05 at 14:23 +0200, Andi Kleen wrote:
>> That's a lot of "should" and "in most cases" and "in a ideal world".
>
> OK, let's phrase it differently:
>
> It almost never happens, and it's trivial to handle it safely in the
> extremely rare cases that it does.

Ok. I'm not sure how you got to your conclusion (I assume you did significant
research on this), but please make sure driver writers and reviewers
are aware of this new requirement.

>> What happens when the new firmware is buggy for example and prevents
>> booting of the system?
>
> If the firmware is required for booting the system,

initramfs is only required for drivers required to
mount the root file system. But there's a lot more to completely booting
a system than just mounting root.

-Andi

2008-07-05 14:44:59

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, Jul 05, 2008 at 01:16:06PM +0100, David Woodhouse wrote:
> It almost never happens that you have kernel versions which _need_
> different firmware installed. In almost all cases, the older driver will
> continue to work just fine with the newer firmware (and its bug-fixes).

I'm not sure which planet you're from, but it's one without ipw2200
chips in it. And in any case, the file names change.

> The ABI between driver and firmware rarely changes in such a fashion
> that you have to update the driver in lock-step -- and even on the
> occasions that it does, it's not hard to simply change the name of the
> "new-style" firmware so that it doesn't stomp on the old one (Think of
> it like an soname).

Ah, I see, you just didn't read the thread you're replying to. Let's
do it again one more time.

The question is, how do you sanely distribute the kernel-tree
generated firmware in a binary distribution, knowing that you want to
be able to have multiple working kernels installed simultaneously?

Solution 1: in the kernel package
-> You get file conflicts on the firmware files that do not change
between kernel versions

Solution 2: in a package by itself
-> You either break compatibility with kernel versions that happened
before a firmware change, or you accumulate tons of files over
time. The accumulated form gets hard to create from source.

Solution 3: in the kernel package or in a kernel-specific package, but
the files are in a kernel version-specific directory
(/lib/firmware/`uname -r`, /lib/modules/`uname -r`/firmware)
-> Incompatible with current userspace

Solution 4: in one package per firmware file, with appropriate
dependencies on the kernel package
-> A number of kernel package maintainers just took a hit on you

Any other solution you can see?

OG.

2008-07-05 15:11:22

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 2008-07-05 at 16:44 +0200, Olivier Galibert wrote:
> On Sat, Jul 05, 2008 at 01:16:06PM +0100, David Woodhouse wrote:
> > It almost never happens that you have kernel versions which _need_
> > different firmware installed. In almost all cases, the older driver will
> > continue to work just fine with the newer firmware (and its bug-fixes).
>
> I'm not sure which planet you're from, but it's one without ipw2200
> chips in it. And in any case, the file names change.

I was speaking of the firmware which is currently in-kernel. ipw2200 is
a recent driver and uses request_firmware() already, so isn't affected
at all when I update other, older drivers. As such, it's not
particularly relevant to this discussion.

The drivers which we're updating to use request_firmware() have _not_
changed their firmware very often at all -- and even _less_ frequently
have they done so in an incompatible fashion.

> > The ABI between driver and firmware rarely changes in such a fashion
> > that you have to update the driver in lock-step -- and even on the
> > occasions that it does, it's not hard to simply change the name of the
> > "new-style" firmware so that it doesn't stomp on the old one (Think of
> > it like an soname).
>
> Ah, I see, you just didn't read the thread you're replying to. Let's
> do it again one more time.
>
> The question is, how do you sanely distribute the kernel-tree
> generated firmware in a binary distribution, knowing that you want to
> be able to have multiple working kernels installed simultaneously?

<...>

> Solution 2: in a package by itself

Probably this one. That package can be seeded from a git repo which is
automatically derived from the contents of the firmware/ directory in
Linus' tree, and can add the other firmware blobs which are available in
various places -- the ones that the owners won't let us include in the
kernel tree due to the GPL, but _will_ allow us to distribute in a
separate firmware repository.

> -> You either break compatibility with kernel versions that happened
> before a firmware change, or you accumulate tons of files over
> time. The accumulated form gets hard to create from source.

On the rare occasions that a firmware changes incompatibly, you'd want
to keep both old and new versions in the firmware tree for a reasonable
period of time. But since that doesn't happen very often, it isn't a
particularly difficult issue to handle. I strongly believe that you are
overestimating the scale of the problem -- and it would only be a
problem for the person maintaining the firmware repository anyway. I'm
perfectly content to do that job.

--
dwmw2

2008-07-05 17:13:55

by Christoph Hellwig

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

2008-07-05 20:55:47

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, 2008-07-05 at 13:13 -0400, Christoph Hellwig wrote:
> On Sat, Jul 05, 2008 at 01:16:06PM +0100, David Woodhouse wrote:
> > It almost never happens that you have kernel versions which _need_
> > different firmware installed. In almost all cases, the older driver will
> > continue to work just fine with the newer firmware (and its bug-fixes).
> >
> > The ABI between driver and firmware rarely changes in such a fashion
> > that you have to update the driver in lock-step -- and even on the
> > occasions that it does, it's not hard to simply change the name of the
> > "new-style" firmware so that it doesn't stomp on the old one (Think of
> > it like an soname).
>
> That's unfortunately not true. There are a lot of drivers that rely
> on specific firmware versions.

Do you have examples of such?

--
dwmw2

2008-07-06 10:03:23

by Christoph Hellwig

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sat, Jul 05, 2008 at 09:55:11PM +0100, David Woodhouse wrote:
> > That's unfortunately not true. There are a lot of drivers that rely
> > on specific firmware versions.
>
> Do you have examples of such?

The worst examples are aic7xx/aic79xx and the symbios family of drivers
where the firmware / driver interface is entirely defined by the driver.
But as we have opensource firmware for these and build it as part of
the kernel build I suspect you don't want to convert them to external
firmware either.

aic94xx has a very similar firmware to aic7xx/aic79xx but it's only
available as blob. We've alredy required specific firmware versions
there.

b43 has two totally different firmware major revisions that even require
different drivers.

2008-07-06 10:56:18

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 2008-07-06 at 06:02 -0400, Christoph Hellwig wrote:
> The worst examples are aic7xx/aic79xx and the symbios family of drivers
> where the firmware / driver interface is entirely defined by the driver.
> But as we have opensource firmware for these and build it as part of
> the kernel build I suspect you don't want to convert them to external
> firmware either.

The fact that they're open source changes the technical situation
somewhat, mostly because it means that the firmware is much more likely
to change in step with the driver, and not have a stable ABI. So it
might make a little more sense to ship the firmware _with_ the driver in
those cases. For aic7.xx it's also a bit harder to load it as a discrete
blob, given that we generate C code in the driver which has intimate
knowledge of its internals.

There's still something to be said for keeping it in userspace and
loading it on demand though if we can, though, rather than keeping it in
kernel memory at all times.

I haven't yet come to a firm conclusion about what to do when we get to
those drivers; they're a bit of a special case.

> aic94xx has a very similar firmware to aic7xx/aic79xx but it's only
> available as blob. We've alredy required specific firmware versions
> there.

I don't believe we were going to touch that; it already uses
request_firmware(), doesn't it?

And what I think you're referring to is this:

[SCSI] aic94xx: tie driver to the major number of the sequencer firmware

The sequencer firmware file has both a string (currently showing
V17/10c6) and a number (currently set to 1.1). It has become apparent
that Adaptec may issue sequencer firmware in the future which could be
incompatible with the current driver. Therefore, the driver will be
tied to the particular major number of the firmware (i.e. the current
driver will load any 1.x firmware).

That's quite a good example. It hasn't happened yet but we know that
_if_ the major version changes, we can treat that like an soname bump.
The new firmware will have a new name, and the old drivers can continue
loading the old firmware.

> b43 has two totally different firmware major revisions that even require
> different drivers.

Another one we're not touching because it already uses request_firmware.
And this is one where we've _already_ used the soname trick -- there are
two versions of the firmware, and they each have different paths
in /lib/firmware. So the old and new drivers can happily coexist.

--
dwmw2

2008-07-06 11:50:56

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> I haven't yet come to a firm conclusion about what to do when we get to
> those drivers; they're a bit of a special case.

You could just keep them as they are? iirc they work just fine.

-Andi

2008-07-06 12:23:38

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 2008-07-06 at 13:50 +0200, Andi Kleen wrote:
> > I haven't yet come to a firm conclusion about what to do when we get to
> > those drivers; they're a bit of a special case.
>
> You could just keep them as they are? iirc they work just fine.

Yes, that makes a certain amount of sense.

--
dwmw2

2008-07-06 20:17:41

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Fri, 4 Jul 2008, Alan Cox wrote:

>> External firmware is by design an error prone system, even with
>> versioning. But by being built and linked into the driver, it
>> is fool proof.
>>
>> On a technical basis alone, we would never disconnect a crucial
>> component such as firmware, from the driver. The only thing
>> charging these transoformations, from day one, is legal concerns.
>
> As I said: We had this argument ten years ago (more than that now
> actually). People said the same thing about modules.
>

and they were right then as well. Fortunantly,at that time the kernel
developers listened and retained the possibility to not use modules.

if David W were to make it possible to not use the load_firmware() call to
userspace and build the firmware into the driver (be it in a monolithic
kernel or the module that contains the driver) this would not be a
problem. the default could be to build in the firmware (avoiding breakage)
and those people and distros that see a reason to seperate the firmware
would be able to by changing that setting.

we have also had the same argument about initrd/initramfs where people
have wanted to make them mandatory by moving things (like partition
detection) out of the kernel. so far this hasn't happened, and I hope it
doesn't.

David Lang

2008-07-06 20:27:55

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
> if David W were to make it possible to not use the load_firmware() call to
> userspace and build the firmware into the driver (be it in a monolithic
> kernel or the module that contains the driver)

You _can_ build the firmware into the kernel.

--
dwmw2

2008-07-06 20:52:18

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
>> if David W were to make it possible to not use the load_firmware() call to
>> userspace and build the firmware into the driver (be it in a monolithic
>> kernel or the module that contains the driver)
>
> You _can_ build the firmware into the kernel.

Which is a problem for those rare situations, like oh say vendor
kernels, where you can ship a driver update but not update the main kernel.

Just like with modules, we were all given the _choice_ to use the new
regime (modules) or stick with the old (100% monolithic kernel).

Under any new system, firmware should be able to be compiled into the
driver module itself -- as it is today.

Jeff

2008-07-06 20:52:32

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 6 Jul 2008, David Woodhouse wrote:

> On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
>> if David W were to make it possible to not use the load_firmware() call to
>> userspace and build the firmware into the driver (be it in a monolithic
>> kernel or the module that contains the driver)
>
> You _can_ build the firmware into the kernel.

right, but not into a module. you have half of the answer in place, but
not all of it.

David Lang

2008-07-06 20:57:31

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 2008-07-06 at 13:52 -0700, [email protected] wrote:
> On Sun, 6 Jul 2008, David Woodhouse wrote:
>
> > On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
> >> if David W were to make it possible to not use the load_firmware() call to
> >> userspace and build the firmware into the driver (be it in a monolithic
> >> kernel or the module that contains the driver)
> >
> > You _can_ build the firmware into the kernel.
>
> right, but not into a module. you have half of the answer in place, but
> not all of it.

The useful half. If you have userspace to load modules, you have
userspace to load firmware too.

--
dwmw2

2008-07-06 21:03:42

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 6 Jul 2008, David Woodhouse wrote:

> On Sun, 2008-07-06 at 13:52 -0700, [email protected] wrote:
>> On Sun, 6 Jul 2008, David Woodhouse wrote:
>>
>>> On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
>>>> if David W were to make it possible to not use the load_firmware() call to
>>>> userspace and build the firmware into the driver (be it in a monolithic
>>>> kernel or the module that contains the driver)
>>>
>>> You _can_ build the firmware into the kernel.
>>
>> right, but not into a module. you have half of the answer in place, but
>> not all of it.
>
> The useful half. If you have userspace to load modules, you have
> userspace to load firmware too.

it's the half where there isn't a work-around (and therefor the most
critical part), but it's also the half that is used less, so in terms of
user impact it could be argued that the part not yet done will cause more
pain.

David Lang

2008-07-06 21:39:34

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Sun, 2008-07-06 at 13:52 -0700, [email protected] wrote:
>> On Sun, 6 Jul 2008, David Woodhouse wrote:
>>
>>> On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
>>>> if David W were to make it possible to not use the load_firmware() call to
>>>> userspace and build the firmware into the driver (be it in a monolithic
>>>> kernel or the module that contains the driver)
>>> You _can_ build the firmware into the kernel.
>> right, but not into a module. you have half of the answer in place, but
>> not all of it.
>
> The useful half. If you have userspace to load modules, you have
> userspace to load firmware too.

Existing examples have already been provided where this logic fails.

Jeff

2008-07-06 22:10:53

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Sun, 2008-07-06 at 17:38 -0400, Jeff Garzik wrote:
> David Woodhouse wrote:
> > On Sun, 2008-07-06 at 13:52 -0700, [email protected] wrote:
> >> On Sun, 6 Jul 2008, David Woodhouse wrote:
> >>
> >>> On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
> >>>> if David W were to make it possible to not use the load_firmware() call to
> >>>> userspace and build the firmware into the driver (be it in a monolithic
> >>>> kernel or the module that contains the driver)
> >>> You _can_ build the firmware into the kernel.
> >> right, but not into a module. you have half of the answer in place, but
> >> not all of it.
> >
> > The useful half. If you have userspace to load modules, you have
> > userspace to load firmware too.
>
> Existing examples have already been provided where this logic fails.

I even provided such an example, where your script greps the module for
'request_firmware' and fails if there's a match. I don't think any of
the other provided examples were _much_ more sensible than that...

--
dwmw2

2008-07-06 23:23:08

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Woodhouse wrote:
> On Sun, 2008-07-06 at 17:38 -0400, Jeff Garzik wrote:
>> David Woodhouse wrote:
>>> On Sun, 2008-07-06 at 13:52 -0700, [email protected] wrote:
>>>> On Sun, 6 Jul 2008, David Woodhouse wrote:
>>>>
>>>>> On Sun, 2008-07-06 at 13:17 -0700, [email protected] wrote:
>>>>>> if David W were to make it possible to not use the load_firmware() call to
>>>>>> userspace and build the firmware into the driver (be it in a monolithic
>>>>>> kernel or the module that contains the driver)
>>>>> You _can_ build the firmware into the kernel.
>>>> right, but not into a module. you have half of the answer in place, but
>>>> not all of it.
>>> The useful half. If you have userspace to load modules, you have
>>> userspace to load firmware too.
>> Existing examples have already been provided where this logic fails.
>
> I even provided such an example, where your script greps the module for
> 'request_firmware' and fails if there's a match. I don't think any of
> the other provided examples were _much_ more sensible than that...

That makes the false presumption that scripts have been updated to avoid
the obvious case -- non-working drivers.

Why is it so difficult to follow a simple principle:

Keep things working tomorrow, that work today.

hmmm?

That's how kernel module support was integrated, so many years ago.

Jeff

2008-07-06 23:41:27

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
> On Fri, 04 Jul 2008 10:07:23 -0400
> Jeff Garzik <[email protected]> wrote:
>
>> David Woodhouse wrote:
>>> On Fri, 2008-07-04 at 09:39 -0400, Jeff Garzik wrote:
>>>> You have been told repeatedly that cp(1) and scp(1) are commonly used
>>>> to transport the module David and I care about -- tg3. It's been a
>>>> single file module since birth, and people take advantage of that
>>>> fact.
>>> And you can _continue_ to do that. You'd need to install the firmware
>>> just once, and that's all. It's a non-issue, and it isn't _worth_ the
>>> added complexity of building the firmware into the module.
>> We can stop there. Real-world examples are apparently non-issues not
>> worth your time.
>
> Jeff - real world issues and gains are measured on a scale across the
> userbase not "Jeff's having a tantrum because his specific usage case
> requires one extra copy once"

Please look at the example as an example of an entire _class_ of usage
that dwmw2 wishes to hand-wave away.

There are many cases where you have the freedom to update the module,
but not the kernel itself -- see vendor kernels and driver update disks,
or the typical usage of the upstream kernel's out-of-tree-module build
support.

It is trivial to think for five minutes and come up with other examples,
because the simple fact is, there is a failure to follow a basic
principle of kernel development: no regressions.

All I'm asking for: what works today, should continue to work tomorrow.

Why is that so wrong?

We applied that principle with libata -- if you did not turn it on, you
didn't even have to know it was there.

We applied that principle with kernel modules -- if you do not turn them
on, you have the static kernel you've always had.

> And we had the same argument over ten years ago about those evil module
> things which stopped you just using scp to copy the kernel in one go.
> Fortunately the nay sayers lost so we have modules.

Broken analogy.

When modules were added, you were given the option to use them, or not.

dwmw2's conversion is not providing analogous choices.

Jeff

2008-07-07 16:27:58

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> > And we had the same argument over ten years ago about those evil module
> > things which stopped you just using scp to copy the kernel in one go.
> > Fortunately the nay sayers lost so we have modules.
>
> Broken analogy.
>
> When modules were added, you were given the option to use them, or not.

You can still choose to compile firmware in. Did you read the patches ?

2008-07-07 17:25:25

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
>>> And we had the same argument over ten years ago about those evil module
>>> things which stopped you just using scp to copy the kernel in one go.
>>> Fortunately the nay sayers lost so we have modules.
>> Broken analogy.
>>
>> When modules were added, you were given the option to use them, or not.
>
> You can still choose to compile firmware in. Did you read the patches ?

You cannot compile the firmware into the modules themselves, which is a
regression from current behavior.

Its a problem for cases where you cannot as readily update the kernel
image, such as vendor kernel + driver disk situations, or other examples
already cited.

When the firmware travels with the module, as it does today in tg3, bnx2
and others, is the most reliable system available. The simplest, the
least amount of "parts", the easiest to upgrade, the best method to
guarantee driver/firmware version matches. It works wonderfully today.

Is it difficult to see why someone might want to keep the same attributes?

Compiled-in firmware wastes memory and isn't upgradable -- just like
static kernel vs. kernel modules debate -- but it IS far more reliable
than any system where the firmware is separated from the kernel module
itself.

I'd heartily support David's efforts if it was done in a regression-free
manner. But it is just so easy to build and package a _silently_
non-working driver, simply because the firmware got missed somewhere.

The best path to this new system is to (a) ensure the old system still
works, and then (b) make it easy (transparent?) to adopt the new system.

Jeff

2008-07-07 17:52:35

by Rick Jones

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

David Miller wrote:
> From: Alan Cox <[email protected]>
> Date: Fri, 4 Jul 2008 14:27:53 +0100
>
>
>>There are good sound reasons for having a firmware tree, the fact tg3 is
>>a bit of dinosaur in this area doesn't make it wrong.
>
>
> And bnx2, and bnx2x, and e100's ucode (hope David caught that one!).
>
> It isn't just tg3.

Ah bnx2 - it may be "fixed" now, but trying to install Debian Lenny on a
system with "core" bnx2 driven interfaces has been "fun" for a while now
thanks to the firmware being exiled.

rick jones

2008-07-07 18:48:20

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> When the firmware travels with the module, as it does today in tg3, bnx2
> and others, is the most reliable system available. The simplest, the
> least amount of "parts", the easiest to upgrade, the best method to
> guarantee driver/firmware version matches. It works wonderfully today.
>
> Is it difficult to see why someone might want to keep the same attributes?

No I can see that, should be a simple matter of sending David patches.

Alan

2008-07-07 18:59:18

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
>> When the firmware travels with the module, as it does today in tg3, bnx2
>> and others, is the most reliable system available. The simplest, the
>> least amount of "parts", the easiest to upgrade, the best method to
>> guarantee driver/firmware version matches. It works wonderfully today.
>>
>> Is it difficult to see why someone might want to keep the same attributes?
>
> No I can see that, should be a simple matter of sending David patches.

Isn't it David's obligation not to remove a highly reliable, working system?

Jeff

2008-07-07 19:03:48

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

On Mon, 07 Jul 2008 14:57:56 -0400
Jeff Garzik <[email protected]> wrote:

> Alan Cox wrote:
> >> When the firmware travels with the module, as it does today in tg3, bnx2
> >> and others, is the most reliable system available. The simplest, the
> >> least amount of "parts", the easiest to upgrade, the best method to
> >> guarantee driver/firmware version matches. It works wonderfully today.
> >>
> >> Is it difficult to see why someone might want to keep the same attributes?
> >
> > No I can see that, should be a simple matter of sending David patches.
>
> Isn't it David's obligation not to remove a highly reliable, working system?

I don't see why it should be David's job to add every conceivable feature
to the code. And this is the pot calling the kettle black. You badly
broke Marvell PATA support by setting the Marvell SATA devices to AHCI. I
note you've still not fixed that after some months.

Alan

2008-07-07 19:16:50

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
> On Mon, 07 Jul 2008 14:57:56 -0400
> Jeff Garzik <[email protected]> wrote:
>
>> Alan Cox wrote:
>>>> When the firmware travels with the module, as it does today in tg3, bnx2
>>>> and others, is the most reliable system available. The simplest, the
>>>> least amount of "parts", the easiest to upgrade, the best method to
>>>> guarantee driver/firmware version matches. It works wonderfully today.
>>>>
>>>> Is it difficult to see why someone might want to keep the same attributes?
>>> No I can see that, should be a simple matter of sending David patches.
>> Isn't it David's obligation not to remove a highly reliable, working system?
>
> I don't see why it should be David's job to add every conceivable feature
> to the code.

Just whose job is it, exactly, to avoid regressions?

Why is it unfair to ask a patch author not to break stuff?

> And this is the pot calling the kettle black. You badly
> broke Marvell PATA support by setting the Marvell SATA devices to AHCI. I
> note you've still not fixed that after some months.

Even if we accept that at face value, which I don't (it's more a driver
load order issue), that is no excuse for further regressions.

Jeff

2008-07-07 19:19:37

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> > And this is the pot calling the kettle black. You badly
> > broke Marvell PATA support by setting the Marvell SATA devices to AHCI. I
> > note you've still not fixed that after some months.
>
> Even if we accept that at face value, which I don't (it's more a driver
> load order issue), that is no excuse for further regressions.

So you are allowed to break stuff without fixing it (and driver load
order issue is not as far as I can tell the case - the AHCI stuff means
you lose the PATA port) but David isnt ?

How about we revert all the marvell changes - or would in truth be
another case where the good done for most (SATA AHCI support) outweighs
the bad for a few (PATA port problems) ?

Sorry Jeff but you don't get to jump up and down on David without being
reminded that your own actions are not consistent with your words.

Alan

2008-07-07 19:48:40

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

Alan Cox wrote:
>>> And this is the pot calling the kettle black. You badly
>>> broke Marvell PATA support by setting the Marvell SATA devices to AHCI. I
>>> note you've still not fixed that after some months.
>> Even if we accept that at face value, which I don't (it's more a driver
>> load order issue), that is no excuse for further regressions.
>
> So you are allowed to break stuff without fixing it (and driver load
> order issue is not as far as I can tell the case - the AHCI stuff means
> you lose the PATA port)

It is trivial to see -- both drivers compete for the same PCI IDs,
0x6145 and 0x6121, but with different capabilities. Load pata_marvell
first, and it claims those PCI IDs first.

> How about we revert all the marvell changes - or would in truth be
> another case where the good done for most (SATA AHCI support) outweighs
> the bad for a few (PATA port problems) ?

What load order would you suggest? pata_marvell-first order preserves
the behavior that existed before the PCI IDs appeared in ahci, by
ensuring it claims PCI IDs 0x6145 and 0x6121 first.

> Sorry Jeff but you don't get to jump up and down on David without being
> reminded that your own actions are not consistent with your words.

Your sidebar here doesn't change the fact that David's current firmware
implementation takes away a tool currently in use, replacing it with
another less-reliable tool.

Jeff

2008-07-07 20:48:44

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Alan Cox <[email protected]>
Date: Mon, 7 Jul 2008 19:30:08 +0100

> I don't see why it should be David's job to add every conceivable feature
> to the code.

David's changes prevent something from working, which works today.

Usually we refer to that as a regression.

And usually, we rely on the patch author to fix regressions they add,
and failing that we revert their work.

Bringing up this SATA scarecrow and trying to make Jeff look
inconsistent is not winning your arguments any extra points.
Especially not with me.

You also mentioned something about how similar arguments as ours
were made when modules were proposed as a feature. Well, I can
say only two things about that:

1) I could still build a static kernel image and use it as-is after
the changes to support modules were added to the kernel. In fact I
still largely do not use modules at all during my own kernel work.

This is completely unlike what David is doing here, where the
previous status quo will cease working.

2) You cannot deny the fine mess we have with proprietary modules and
such these days. It has been quite the pandora's box over time.

2008-07-07 21:16:01

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> 2) You cannot deny the fine mess we have with proprietary modules and
> such these days. It has been quite the pandora's box over time.

You seem to be trying to conflate legal and technical issues here.

Alan

2008-07-07 21:45:27

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Alan Cox <[email protected]>
Date: Mon, 7 Jul 2008 21:42:18 +0100

> > 2) You cannot deny the fine mess we have with proprietary modules and
> > such these days. It has been quite the pandora's box over time.
>
> You seem to be trying to conflate legal and technical issues here.

Exactly like the patches we are current discussing.

Thanks for walking right into that. :-)

2008-07-07 21:48:32

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> > You seem to be trying to conflate legal and technical issues here.
>
> Exactly like the patches we are current discussing.
>
> Thanks for walking right into that. :-)

No - the patches are for technical reasons, certain people seem to be
trying to attach legal stuff to them - you included, including mad
conspiracy theories that David was somehow being told by two different
employers to do this while one of them was also mysteriously not telling
you to do the same.

2008-07-07 21:58:57

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

From: Alan Cox <[email protected]>
Date: Mon, 7 Jul 2008 22:14:27 +0100

> > > You seem to be trying to conflate legal and technical issues here.
> >
> > Exactly like the patches we are current discussing.
> >
> > Thanks for walking right into that. :-)
>
> No - the patches are for technical reasons,

Which are? Consistent use of request_firmware()?

That's pure bullox as far as I can see. Why provide the means to
do something nobody has had a need for in 6+ years? Who needs
to load different firmware for the tg3 driver?

Who needs that capability? Distribution vendors? What for?
In what case will they need to load different firmware from
what the driver maintainer tested as a unit?

Rather, they want separation. I can see no other real impetus.

And, btw, who has the right to enforce this new burdon upon driver
maintainers when they have had a working and maintainable system for
so long?

I can only see it being about separation, pure and simple.

2008-07-08 07:10:50

by Alan

[permalink] [raw]

Subject: Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"

> That's pure bullox as far as I can see. Why provide the means to
> do something nobody has had a need for in 6+ years? Who needs
> to load different firmware for the tg3 driver?

Who needs modules, nobody needed it for years ... you are repeating
historically failed arguments still.

> Who needs that capability? Distribution vendors? What for?
> In what case will they need to load different firmware from
> what the driver maintainer tested as a unit?

For some drivers yes. Maybe not tg3.

> And, btw, who has the right to enforce this new burdon upon driver
> maintainers when they have had a working and maintainable system for
> so long?

The module argument again - see my comment about the sound driver history.

> I can only see it being about separation, pure and simple.

Separation - of firmware that can be paged from code that cannot. Of
stuff that doesn't change from stuff that does. That happens to be good
engineering.

Alan

2008-07-08 08:57:19