Re: 2.6.26, PAT and AMD family 6

[Adrian Bunk]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/