linux-kernel - Re: [tip:x86/apic] x86: Add NumaChip support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAELBVzBPcARsNSDhv0TUfSmXAu3+k5_KK-Aqv2JMaHX+gjOL+w@mail.gmail.com>
Date:	Fri, 9 Dec 2011 22:28:29 -0400
From:	Kevin Winchester <kjwinchester@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Steffen Persvold <sp@...ascale.com>, mingo@...hat.com,
	hpa@...or.com, linux-kernel@...r.kernel.org,
	jbarnes@...tuousgeek.org, tglx@...utronix.de,
	daniel@...ascale-asia.com, linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/apic] x86: Add NumaChip support

On 9 December 2011 21:52, Kevin Winchester <kjwinchester@...il.com> wrote:
> On 9 December 2011 03:22, Ingo Molnar <mingo@...e.hu> wrote:
>>
>> * Kevin Winchester <kjwinchester@...il.com> wrote:
>>
>>> On 6 December 2011 01:50, Ingo Molnar <mingo@...e.hu> wrote:
>>> >
>>> >
>>> > Ideally i think c->phys_proc_id should should be available
>>> > regardless of CONFIG_SMP or CONFIG_NUMA considerations - but
>>> > that would be a wider change. (feel free to have a shot at it
>>> > though, in addition to the fix above)
>>> >
>>>
>>> If Steffen does not plan to do this additional cleanup, I
>>> would give it a try.  You would likely prefer the changes
>>> against -tip, correct?
>>
>> On a second thought, the !SMP block in processor.h::cpuinfo_x86
>> is pretty self-contained and making it unconditional would
>> increase UP kernel size by 4x5==20 bytes.
>>
>> I have not checked how many further simplifications this allows
>> - if it's a really nice cleanup then i guess we could do it and
>> keep the all-zeroes-and-ones default value on UP.
>>
>> The fields *do* make sense on UP as well.
>>
>> So it's a "try and see how it ends up" thing.
>>
>
> So I get the following (gmail-mangled) patch as a cleanup if the
> fields are made available on UP.  Is it worth it?  I'll let you be the
> judge.  One other thing I noticed that prevented a few more #ifdef
> removals was the global smp_num_siblings variable defined in
> smpboot.c.  This variable appears related to hyperthreading siblings
> from what I can tell, but it gets used in ways related to the cpu
> info.  Would it be possible to move this into a new field in struct
> cpuinfo_x86?  It seems like a cpu-related property to me, not that I
> can imagine there will ever be a processor architecture where
> different cpus have different numbers of HT threads.
>
> In any case, if you like the simple cleanup, I can resend with sign
> off and no whitespace damage.  If you want me to go further, let me
> know and I'll give it a shot.
>
>  arch/x86/include/asm/processor.h     |    2 --
>  arch/x86/kernel/amd_nb.c             |    8 ++------
>  arch/x86/kernel/cpu/amd.c            |    2 --
>  arch/x86/kernel/cpu/common.c         |    7 -------
>  arch/x86/kernel/cpu/intel.c          |    2 --
>  arch/x86/kernel/cpu/mcheck/mce.c     |    2 --
>  arch/x86/kernel/cpu/mcheck/mce_amd.c |    7 +------
>  arch/x86/kernel/cpu/proc.c           |    4 +---
>  drivers/edac/sb_edac.c               |    2 --
>  drivers/hwmon/coretemp.c             |    6 ------
>  10 files changed, 4 insertions(+), 38 deletions(-)
>

A couple of other notes:

- For my config, converted to !SMP, the size of the kernel text grew
by 267 bytes:

   text	   data	    bss	    dec	    hex	filename
4734483	 510391	 972040	6216914	 5edcd2	vmlinux.o.before
4734750	 510391	 972040	6217181	 5edddd	vmlinux.o.after

Maybe that makes it no longer worthwhile for removing 34 lines of code...

- The booted_cores field is only set in smpboot.c, and only used there
and in cpu/proc.c in a SMP-specific block of code.  I wasn't sure
where the best place would be to set it for the UP case, so I left it
alone.  That technically makes it wrong since it will be 0 when there
really is one core booted, but no one will notice.  If you have a
suggestion for initializing it, let me know and I can include it.

And in case anyone is wondering, I am aware that this is pretty basic
in nature and not necessarily worth anyone's time to code/review.  But
it gives me a little more insight into the kernel codebase, so I'm
happy to do it anyway.

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