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
0001-KVM-IA64-Add-API-for-allocating-Dynamic-TR-resource.patch
Description: 0001-KVM-IA64-Add-API-for-allocating-Dynamic-TR-resource.patch
------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
