All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jaswinder Singh Rajput <jaswinder@kernel.org>
Cc: x86 maintainers <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [git-pull -tip] x86: cpu_debug patches
Date: Mon, 20 Apr 2009 13:16:19 +0200	[thread overview]
Message-ID: <20090420111619.GE6670@elte.hu> (raw)
In-Reply-To: <1240217428.3083.2.camel@ht.satnam>


* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

>  static DEFINE_PER_CPU(struct cpu_cpuX_base, cpu_arr[CPU_REG_ALL_BIT]);
>  static DEFINE_PER_CPU(struct cpu_private *, priv_arr[MAX_CPU_FILES]);
>  static DEFINE_PER_CPU(unsigned, cpu_modelflag);
>  static DEFINE_PER_CPU(int, cpu_priv_count);
> -static DEFINE_PER_CPU(unsigned, cpu_model);
> +
> +/* Storing vendor locally because it is used excessive in this code */
> +static unsigned cpu_vendor;

There's still no need to store it locally - what's wrong with 
cpu_data(cpu) or current_cpu_data?

Also, do we need the per-cpu cpu_modelflag variable too? I'd suggest 
to integrate that kind of enumeration into struct cpuinfo_x86 and 
cpu_info. We often have such kinds of constructs in x86 code:

	c->x86 <= 0x11

So extending your scheme to other code would benefit all code.

Plus this kind of enumeration:

        switch (model) {
        case 0x0501:
        case 0x0502:
        case 0x0504:
                flag = CPU_INTEL_PENTIUM;
                break;
        case 0x0601:
        case 0x0603:
        case 0x0605:
        case 0x0607:
        case 0x0608:
        case 0x060A:
        case 0x060B:
                flag = CPU_INTEL_P6;

The 0x05/0x06 there is already available as the family flag in 
cpuinfo_x86, as cpu_info::x86. The 01,02...0B model portion is also 
already available as cpu_info::x86_model.

[ there's also cpu_info::x86_mask, which gives the stepping. ]

This is what i meant when i said that you needlessly duplicate 
already existing information. You encode/decode it in some weird 
looking way instead of using the already existing, per CPU 
information of cpu_info.

	Ingo

  reply	other threads:[~2009-04-20 11:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-20  1:15 [git-pull -tip] x86: cpu_debug patches Jaswinder Singh Rajput
2009-04-20  1:35 ` Jaswinder Singh Rajput
2009-04-20  8:50   ` Jaswinder Singh Rajput
2009-04-20 11:16     ` Ingo Molnar [this message]
2009-04-28 15:43       ` Jaswinder Singh Rajput
2009-04-28 17:28         ` Ingo Molnar
2009-04-29  3:52           ` Jaswinder Singh Rajput
2009-04-29 10:50             ` Ingo Molnar
2009-04-29 12:14               ` Jaswinder Singh Rajput
2009-04-29 12:30                 ` Ingo Molnar
2009-05-03  9:09                 ` Ingo Molnar
2009-05-06  9:57                   ` [RFC][git-pull -tip] x86: cpu_debug and cpufeature patches Jaswinder Singh Rajput
2009-05-06 12:25                     ` Ingo Molnar
2009-05-06 12:49                       ` Jaswinder Singh Rajput
2009-05-08  0:39                     ` Jaswinder Singh Rajput
2009-05-09 18:36                     ` Jaswinder Singh Rajput
2009-05-11 14:07                       ` Ingo Molnar
2009-05-19 11:57                   ` [git-pull -tip] x86: cpu_debug patches Jaswinder Singh Rajput

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090420111619.GE6670@elte.hu \
    --to=mingo@elte.hu \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jaswinder@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.