[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080507220942.GD14951@cs181133002.pp.htv.fi>
Date: Thu, 8 May 2008 01:09:42 +0300
From: Adrian Bunk <bunk@...sta.de>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Yinghai Lu <yhlu.kernel@...il.com>,
Rene Herman <rene.herman@...access.nl>,
Ingo Molnar <mingo@...e.hu>,
Linux Kernel <linux-kernel@...r.kernel.org>, hpa@...or.com,
torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
Pavel Machek <pavel@...e.cz>
Subject: Re: 2.6.26, PAT and AMD family 6
On Wed, May 07, 2008 at 11:54:19PM +0200, Thomas Gleixner wrote:
> On Thu, 8 May 2008, Adrian Bunk wrote:
> > On Wed, May 07, 2008 at 10:52:36PM +0200, Thomas Gleixner wrote:
>...
> > > Also there is no downside on these older systems. They are fine with
> > > MTRRs and have been for years. We can revisit that once PAT support
> > > has stabilized.
> >
> > My comment was about:
> > "please try attach patch to see if duron support PAT."
>
> I know.
>
> > That information is for sure available.
>
> So you know where it is and you can confirm that it works fine ?
>
> Pointers please.
Rene testing the patch Yinghai Lu sent would can reveal reliably whether
it works fine for all Duron steppings?
> > > This feature and detection code is hard to clean up and definitely out
> > > of the scope of this patch.
> >
> > Did you even look at the commit we are discussing?
> >
> > It ***adds*** exactly the same code at three different places.
>
> Yes, I did. And it adds it for a fscking good reason.
>
> 1) two times in common.c due to the existing detection logic mess
> 2) once in the 64 bit version
>
> Again, this code is inherited and fragile mess, which can not be
> changed at will.
Please explain why having this code twice in arch/x86/kernel/cpu/common.c
should be safer than doing the logical thing of putting it into an own
function:
void clear_set_pat(struct cpuinfo_x86 *c)
{
clear_cpu_cap(c, X86_FEATURE_PAT);
switch (c->x86_vendor) {
case X86_VENDOR_AMD:
if (c->x86 >= 0xf && c->x86 <= 0x11)
set_cpu_cap(c, X86_FEATURE_PAT);
break;
case X86_VENDOR_INTEL:
if (c->x86 == 0xF || (c->x86 == 6 && c->x86_model >= 15))
set_cpu_cap(c, X86_FEATURE_PAT);
break;
}
}
> > >...
> > > > And this patch (by the author of the code himself) is the first time
> > > > where it breaks.
> > >
> > > Very interesting analysis. What broke ? This CPU was never in the set
> > > of supported ones at all.
> >
> > The patch in the email I answered to was broken since the author did
>
> "the author" has a name.
>
> > fall into his own trap by patching only one copy of his duplicated code.
>
> That's not a real good reason to yell at him.
>
> Yinghai wanted to help out and check with the reporter whether this CPU
> works with PAT or not.
>
> Yinghai made a mistake.
>
> You come along and ride a crusade against him for that. Very helpful.
I'm actually on a crusade against:
- the many empty or unusable commit descriptions that recently came
through the x86 tree
- the fact that Yinghai Lu ignored Pavel's valid commit to not do
copy&paste programming
> Thanks,
>
> tglx
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
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/
Powered by blists - more mailing lists