All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Robert Richter" <robert.richter@amd.com>
To: "David Rientjes" <rientjes@google.com>
Cc: "Stephane Eranian" <eranian@hpl.hp.com>,
	"Andi Kleen" <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [patch 4/8] 2.6.22-rc3 perfmon2 : IBS implementation for AMD64
Date: Wed, 20 Jun 2007 20:36:32 +0200	[thread overview]
Message-ID: <20070620183632.GG24544@erda.amd.com> (raw)
In-Reply-To: <alpine.DEB.0.99.0706151149480.10812@chino.kir.corp.google.com>

David,

> Please don't include mixed cases of hex digits.  This entire file has all 
> hex digits in lowercase type, so please conform to that.

I fixed this in the 2nd version of the patch.

> > +#define K8_MTRRFIXRANGE_DRAM_ENABLE	0x00040000 /* MtrrFixDramEn bit    */
> > +#define K8_MTRRFIXRANGE_DRAM_MODIFY	0x00080000 /* MtrrFixDramModEn bit */
> > +#define K8_MTRR_RDMEM_WRMEM_MASK	0x18181818 /* Mask: RdMem|WrMem    */
> 
> Masks like K8_MTRR_RDMEM_WRMEM_MASK are prone to bugs when the values they 
> are testing change and somebody forgets to update the mask.  Can you make 
> K8_MTRR_RDMEM_WRMEM_MASK defined to be the result of another preprocessor 
> macro expression?  Or, even better, get rid of it completely and modify 
> set_fixed_range()?

This is existing code, won't change that.

Thanks,
-Robert

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: robert.richter@amd.com




  reply	other threads:[~2007-06-20 18:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-15 16:56 [patch 0/8] 2.6.22-rc3 perfmon2 : IBS implementation for AMD64 Robert Richter
2007-06-15 16:57 ` [patch 1/8] " Robert Richter
2007-06-15 18:54   ` David Rientjes
2007-06-20 18:36     ` Robert Richter
2007-06-19 12:39   ` Stephane Eranian
2007-06-15 16:58 ` [patch 2/8] " Robert Richter
2007-06-19 12:39   ` Stephane Eranian
2007-06-15 16:58 ` [patch 3/8] " Robert Richter
2007-06-19 12:40   ` Stephane Eranian
2007-06-15 16:59 ` [patch 4/8] " Robert Richter
2007-06-15 18:52   ` David Rientjes
2007-06-20 18:36     ` Robert Richter [this message]
2007-06-19 13:34   ` Stephane Eranian
2007-06-15 17:00 ` [patch 5/8] " Robert Richter
2007-06-15 17:00 ` [patch 6/8] " Robert Richter
2007-06-15 20:15   ` David Rientjes
2007-06-20 18:38     ` Robert Richter
2007-06-15 17:01 ` [patch 7/8] " Robert Richter
2007-06-15 17:02 ` [patch 8/8] " Robert Richter

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=20070620183632.GG24544@erda.amd.com \
    --to=robert.richter@amd.com \
    --cc=andi@firstfloor.org \
    --cc=eranian@hpl.hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rientjes@google.com \
    /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.