Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: David VomLehn <dvomlehn@cisco.com>
To: David Daney <ddaney@caviumnetworks.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
	linux-mips@linux-mips.org, "Paoletti,
	Tomaso" <Tomaso.Paoletti@caviumnetworks.com>
Subject: Re: [PATCH] MIPS: Rewrite cpu_to_name so it has one statement per line.
Date: Tue, 14 Oct 2008 11:25:19 -0700	[thread overview]
Message-ID: <48F4E40F.1090808@cisco.com> (raw)
In-Reply-To: <48F4C580.5030702@caviumnetworks.com>

David Daney wrote:
> Ralf Baechle wrote:
>> On Mon, Oct 13, 2008 at 11:00:30AM -0700, David Daney wrote:
>>
>>> Rewrite cpu_to_name so it has one statement per line.
>>>
>>> Future changes can now pass checkpatch.pl
>>
>> It's been one of those changes where I found the Linux coding style in my
>> opinion at least, not to be optimal.  My plan was to rewrite it like 
>> below
>> incomplete patch for ages.  What do you think?
>>
>>   Ralf
>>
>> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>>
>> diff --git a/arch/mips/include/asm/cpu-info.h 
>> b/arch/mips/include/asm/cpu-info.h
>> index 744cd8f..6d0f891 100644
>> --- a/arch/mips/include/asm/cpu-info.h
>> +++ b/arch/mips/include/asm/cpu-info.h
>> @@ -75,6 +75,7 @@ struct cpuinfo_mips {
>>      unsigned int        watch_reg_use_cnt; /* Usable by ptrace */
>>  #define NUM_WATCH_REGS 4
>>      u16            watch_reg_masks[NUM_WATCH_REGS];
>> +    const char        *name;
>>  } __attribute__((aligned(SMP_CACHE_BYTES)));
>>  
> 
> It increases the size of the cpuinfo_mips structure by sizeof(char *)
> for data that is only ever used in /proc/cpuinfo, also it goes against
> my sense of data normalization.  So I think the current method of
> looking it up on demand is fine.
> 
> I am not enamored with my patch as it doubles the number of lines in
> the function.  So we will defer to you and follow which ever style you
> decide is best.
> 
> David Daney

This is a pretty trivial issue, but I would note that the /proc/cpuinfo code is 
not a performance critical path. Thus, adding redundant data isn't worth it. If 
we really care about checkpatch and, even in this case, I kinda do, we could 
shrink the code by doing a linear scan of a table that contains the CPU type and 
the name. If you really care about code and data size *and* want it fast you 
could demand that the CPU types be sequentially numbered values, possibly enums, 
that you use to index into a table of CPU names.

Okay, I'm done beating this dead horse...

David VomLehn






     - - - - -                              Cisco                            - - - - -         
This e-mail and any attachments may contain information which is confidential, 
proprietary, privileged or otherwise protected by law. The information is solely 
intended for the named addressee (or a person responsible for delivering it to 
the addressee). If you are not the intended recipient of this message, you are 
not authorized to read, print, retain, copy or disseminate this message or any 
part of it. If you have received this e-mail in error, please notify the sender 
immediately by return e-mail and delete it from your computer.

      reply	other threads:[~2008-10-14 18:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-13 18:00 [PATCH] MIPS: Rewrite cpu_to_name so it has one statement per line David Daney
2008-10-14  9:01 ` Ralf Baechle
2008-10-14 16:14   ` David Daney
2008-10-14 18:25     ` David VomLehn [this message]

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=48F4E40F.1090808@cisco.com \
    --to=dvomlehn@cisco.com \
    --cc=Tomaso.Paoletti@caviumnetworks.com \
    --cc=ddaney@caviumnetworks.com \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox