From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm Date: Mon, 6 Jul 2015 16:34:35 -0500 Message-ID: <1436218475.2658.14.camel@freescale.com> References: <1433917639-31699-1-git-send-email-wenweitaowenwei@gmail.com> <1433917639-31699-7-git-send-email-wenweitaowenwei@gmail.com> <1435873760.10531.11.camel@freescale.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Izik Eidus , , , Hugh Dickins , , , , , , , , , , , To: wenwei tao Return-path: In-Reply-To: Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org