Re: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe()

From: Yinghai Lu
Date: Wed Feb 06 2013 - 12:10:48 EST


On Wed, Feb 6, 2013 at 5:16 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
> * Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>
>> On Tue, Feb 5, 2013 at 4:24 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>> >
>> > * Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>> >
>> >> From: Stoney Wang <song-bo.wang@xxxxxx>
>> >>
>> >> When HP ProLiant DL980 G7 Server boot without x2apic_phys, there will be
>> >> intermittent lost interrupts and could result in a hang or data loss.
>> >
>> > What does 'boot without x2apic_phys' mean?
>> >
>> > Does it mean x2apic_phys=0 boot command line? Or, because
>> > x2apic_phys is off by default, does it simply mean that if it's
>> > booted with a default kernel, without any workaround specified
>> > on the boot command line?
>>
>> means that user does not append "x2apic_phys" in boot command line.
>
> The user does not append a whole lot of other
> behavior-modification command line options either! There's no
> apic=0 line either. Nor smp=0.
>
> Adding this essentially irrelevant piece of information to the
> *FIRST*, most important sentence of the changelog is thus not
> just confusing but utterly misleading. Communications 101.

Those system should not be out of the door with buggy bios.
Assume that HP would have release notes that require user to append
"x2apic_phys"

>
> Instead it should say something like:
>
> When a HP ProLiant DL980 G7 Server boots a regular kernel,
> there will be intermittent lost interrupts which could
> result in a hang or (in extreme cases) data loss.
>
> The reason is that this system only supports x2apic physical
> mode, while the kernel boots with a logical-cluster default
> setting.
>
> This bug can be worked around by specifying the x2apic_phys=1
> boot option, but we want to handle this sytem without
> requiring manual workarounds.
>
> Right? Writing a clean changelog is like writing clean code -
> you have to learn it if you want to contribute to the kernel
> smoothly. Think of it as an engineering task: the other required
> half of modifying kernel code.

Yes. that is clean. just want to update the workaround words to
"x2apic_phys" or "nox2apic"

This bug can be worked around by specifying "x2apic_phys" or
"nox2apic" boot option, but we want to handle this system without
requiring manual workarounds.

Will continue to work on writing clean changelog.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/