From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Mon, 06 Jul 2015 21:34:35 +0000 Subject: Re: [RFC PATCH 6/6] powerpc/kvm: change the condition of identifying hugetlb vm Message-Id: <1436218475.2658.14.camel@freescale.com> List-Id: 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> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: wenwei tao Cc: Izik Eidus , aarcange@redhat.com, chrisw@sous-sol.org, Hugh Dickins , tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, x86@kernel.org, viro@zeniv.linux.org.uk, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org, kvm-ppc@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