Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm
Hi Scott
I understand what you said.
I will use the function 'is_vm_hugetlb_page()' to hide the bit
combinations according to your comments in the next version of patch
set.
But for the situation like below, there isn't an obvious structure
'vma', using 'is_vm_hugetlb_page()' maybe costly or even not possible.
void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
unsigned long end, unsigned long vmflag)
{
...
if (end == TLB_FLUSH_ALL || tlb_flushall_shift == -1
|| vmflag VM_HUGETLB) {
local_flush_tlb();
goto flush_all;
}
...
}
Thank you
Wenwei
2015-07-07 5:34 GMT+08:00 Scott Wood scottw...@freescale.com:
On Fri, 2015-07-03 at 16:47 +0800, wenwei tao wrote:
Hi Scott
Thank you for your comments.
Kernel already has that function: is_vm_hugetlb_page() , but the
original code didn't use it,
in order to keep the coding style of the original code, I didn't use it
either.
For the sentence like: vma-vm_flags VM_HUGETLB , hiding it behind
'is_vm_hugetlb_page()' is ok,
but the sentence like: vma-vm_flags
(VM_LOCKED|VM_HUGETLB|VM_PFNMAP) appears in the patch 2/6,
is it better to hide the bit combinations behind the
is_vm_hugetlb_page() ? In my patch I just replaced it with
vma-vm_flags (VM_LOCKED|VM_PFNMAP) || (vma-vm_flags
(VM_HUGETLB|VM_MERGEABLE)) == VM_HUGETLB.
If you're going to do non-obvious things with the flags, it should be done in
one place rather than throughout the code. Why would you do the above and
not vma-vm_flags (VM_LOCKED | VM_PFNMAP) || is_vm_hugetlb_page(vma)?
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm
On Tue, 2015-07-07 at 16:05 +0800, wenwei tao wrote:
Hi Scott
I understand what you said.
I will use the function 'is_vm_hugetlb_page()' to hide the bit
combinations according to your comments in the next version of patch
set.
But for the situation like below, there isn't an obvious structure
'vma', using 'is_vm_hugetlb_page()' maybe costly or even not possible.
void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
unsigned long end, unsigned long vmflag)
{
...
if (end == TLB_FLUSH_ALL || tlb_flushall_shift == -1
|| vmflag VM_HUGETLB) {
local_flush_tlb();
goto flush_all;
}
...
}
Add a function that operates on the flags directly, then.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm
On Fri, 2015-07-03 at 16:47 +0800, wenwei tao wrote: Hi Scott Thank you for your comments. Kernel already has that function: is_vm_hugetlb_page() , but the original code didn't use it, in order to keep the coding style of the original code, I didn't use it either. For the sentence like: vma-vm_flags VM_HUGETLB , hiding it behind 'is_vm_hugetlb_page()' is ok, but the sentence like: vma-vm_flags (VM_LOCKED|VM_HUGETLB|VM_PFNMAP) appears in the patch 2/6, is it better to hide the bit combinations behind the is_vm_hugetlb_page() ? In my patch I just replaced it with vma-vm_flags (VM_LOCKED|VM_PFNMAP) || (vma-vm_flags (VM_HUGETLB|VM_MERGEABLE)) == VM_HUGETLB. If you're going to do non-obvious things with the flags, it should be done in one place rather than throughout the code. Why would you do the above and not vma-vm_flags (VM_LOCKED | VM_PFNMAP) || is_vm_hugetlb_page(vma)? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm
Hi Scott Thank you for your comments. Kernel already has that function: is_vm_hugetlb_page() , but the original code didn't use it, in order to keep the coding style of the original code, I didn't use it either. For the sentence like: vma-vm_flags VM_HUGETLB , hiding it behind 'is_vm_hugetlb_page()' is ok, but the sentence like: vma-vm_flags (VM_LOCKED|VM_HUGETLB|VM_PFNMAP) appears in the patch 2/6, is it better to hide the bit combinations behind the is_vm_hugetlb_page() ? In my patch I just replaced it with vma-vm_flags (VM_LOCKED|VM_PFNMAP) || (vma-vm_flags (VM_HUGETLB|VM_MERGEABLE)) == VM_HUGETLB. I am a newbie to Linux kernel, do you have any good suggestions on this situation? Thank you Wenwei 2015-07-03 5:49 GMT+08:00 Scott Wood scottw...@freescale.com: On Wed, 2015-06-10 at 14:27 +0800, Wenwei Tao wrote: Hugetlb VMAs are not mergeable, that means a VMA couldn't have VM_HUGETLB and VM_MERGEABLE been set in the same time. So we use VM_HUGETLB to indicate new mergeable VMAs. Because of that a VMA which has VM_HUGETLB been set is a hugetlb VMA only if it doesn't have VM_MERGEABLE been set in the same time. Eww. If you must overload such bit combinations, please hide it behind a vm_is_hugetlb() function. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm
On Wed, 2015-06-10 at 14:27 +0800, Wenwei Tao wrote: Hugetlb VMAs are not mergeable, that means a VMA couldn't have VM_HUGETLB and VM_MERGEABLE been set in the same time. So we use VM_HUGETLB to indicate new mergeable VMAs. Because of that a VMA which has VM_HUGETLB been set is a hugetlb VMA only if it doesn't have VM_MERGEABLE been set in the same time. Eww. If you must overload such bit combinations, please hide it behind a vm_is_hugetlb() function. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm
Hugetlb VMAs are not mergeable, that means a VMA couldn't have VM_HUGETLB and
VM_MERGEABLE been set in the same time. So we use VM_HUGETLB to indicate new
mergeable VMAs. Because of that a VMA which has VM_HUGETLB been set is a hugetlb
VMA only if it doesn't have VM_MERGEABLE been set in the same time.
Signed-off-by: Wenwei Tao wenweitaowen...@gmail.com
---
arch/powerpc/kvm/e500_mmu_host.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index cc536d4..d76f518 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -423,7 +423,8 @@ static inline int kvmppc_e500_shadow_map(struct
kvmppc_vcpu_e500 *vcpu_e500,
break;
}
} else if (vma hva = vma-vm_start
- (vma-vm_flags VM_HUGETLB)) {
+ ((vma-vm_flags (VM_HUGETLB | VM_MERGEABLE)) ==
+ VM_HUGETLB)) {
unsigned long psize = vma_kernel_pagesize(vma);
tsize = (gtlbe-mas1 MAS1_TSIZE_MASK)
--
1.7.9.5
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
