Re: [GIT PULL] x86 fixes for 2.6.30-rc8

["H. Peter Anvin" <hpa@zytor.com>]

[Replying to add Venki back to the Cc: list since I typed his name]

Linus Torvalds wrote:
> 
> On Mon, 25 May 2009, H. Peter Anvin wrote:
>> +static void wbinvd_local(void *unused)
>> +{
>> +	wbinvd();
>> +}
>> +
>>  static void cpa_flush_array(unsigned long *start, int numpages, int cache,
>>  			    int in_flags, struct page **pages)
>>  {
>> @@ -218,8 +223,9 @@ static void cpa_flush_array(unsigned long *start, int numpages, int cache,
>>  
>>  	/* 4M threshold */
>>  	if (numpages >= 1024) {
>> -		if (boot_cpu_data.x86_model >= 4)
>> -			wbinvd();
>> +		if (boot_cpu_data.x86 >= 4)
>> +			on_each_cpu(wbinvd_local, NULL, 1);
>> +
> 
> This looks a bit wrong.
> 
> Just above this, we've done
> 
> 	on_each_cpu(__cpa_flush_range, NULL, 1);
> 
> and quite frankly, it seems to be that what we _should_ have done is to 
> instead change that to
> 
> 	long do_wbinvd = cache && numpages >= 1024;
> 
> 	on_each_cpu(__cpa_flush_all, (void *)do_wbinvd, 1);
> 	if (!cache || do_wbinvd)
> 		return;
> 
> 	.. do the cflush dance .. 
> 
> instead.
> 
> Now you made it do two different "on_each_cpu" things. Maybe it doesn't 
> matter, but it just seems wrong.
> 
> 		Linus


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.