RE: [01/17]PATCH Add API for allocating dynamic TR resouce. V8
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
- To: "Jes Sorensen" <jes@xxxxxxx>
- Subject: RE: [01/17]PATCH Add API for allocating dynamic TR resouce. V8
- From: "Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>
- Date: Mon, 31 Mar 2008 17:53:07 +0800
- Cc: "Avi Kivity" <avi@xxxxxxxxxxxx>, "Luck, Tony" <tony.luck@xxxxxxxxx>, "Xu, Anthony" <anthony.xu@xxxxxxxxx>, "Akio Takebe" <takebe_akio@xxxxxxxxxxxxxx>, <kvm-devel@xxxxxxxxxxxxxxxxxxxxx>, <kvm-ia64-devel@xxxxxxxxxxxxxxxxxxxxx>, <linux-ia64@xxxxxxxxxxxxxxx>, <virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx>, "Carsten Otte" <cotte@xxxxxxxxxx>
- In-reply-to: <47F0A8F1.2090109@xxxxxxx>
- List-id: <linux-ia64.vger.kernel.org>
- References: <42DFA526FC41B1429CE7279EF83C6BDC0104823F@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <47F0A8F1.2090109@xxxxxxx>
- Thread-index: AciTDjWUVWbKKSkiSU6AFBzXQynnVwABiNmA
- Thread-topic: [01/17]PATCH Add API for allocating dynamic TR resouce. V8
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
Attachment:
0001-KVM-IA64-Add-API-for-allocating-Dynamic-TR-resource.patch
Description: 0001-KVM-IA64-Add-API-for-allocating-Dynamic-TR-resource.patch
[Index of Archives]
[Linux Kernel]
[Sparc Linux]
[DCCP]
[Linux ARM]
[Yosemite News]
[Linux SCSI]
[Linux x86_64]
[Linux for Ham Radio]