Re: [kvm-ia64-devel] [01/17]PATCH Add API for allocating dynamic TR resouce. V8

[Zhang, X. <xia...@in...>]

Jes Sorensen wrote:
> Hi Xiantao,
> 
> I general I think the code in this patch is fine. I have a couple of
> nit-picking comments:
> 
>> +	if (target_mask&0x1) {
> 
> The formatting here isn't quite what most of the kernel does. It would
> be better if you added spaces so it's a little easier to read, ie:

Good suggesion! 


> 	if (target_mask & 0x1) {
> 
>> +		p = &__per_cpu_idtrs[cpu][0][0];
>> +		for (i = IA64_TR_ALLOC_BASE; i <= per_cpu(ia64_tr_used,
cpu);
>> +								i++,
>> p++) {
>> +			if (p->pte&0x1)
> 
> Same thing here.
> 
>> +#define RR_TO_RID(rr) ((rr)<<32>>40)
> 
> I would prefer to have this one defined like this:
> 
> #define RR_TO_RID(rr)	(rr >> 8) & 0xffffff
> 
> It should generate the same code, but is more intuitive for the
> reader. 

Looks better :)


> Otherwise I think this patch is fine - this is really just cosmetics.
Thank you! 
Xiantao