From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752570AbdFVOiA (ORCPT ); Thu, 22 Jun 2017 10:38:00 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:48576 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751221AbdFVOh7 (ORCPT ); Thu, 22 Jun 2017 10:37:59 -0400 Subject: Re: [PATCH] sparc64: Fix gup_huge_pmd To: Julian Calaby References: <1498081816-179369-1-git-send-email-nitin.m.gupta@oracle.com> Cc: "David S. Miller" , Lorenzo Stoakes , Jan Kara , Michal Hocko , sparclinux , "linux-kernel@vger.kernel.org" From: Nitin Gupta Message-ID: <34836bf2-2cef-6fc0-52e5-92767883eabf@oracle.com> Date: Thu, 22 Jun 2017 07:37:45 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Julian, On 6/22/17 3:53 AM, Julian Calaby wrote: > On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta wrote: >> The function assumes that each PMD points to head of a >> huge page. This is not correct as a PMD can point to >> start of any 8M region with a, say 256M, hugepage. The >> fix ensures that it points to the correct head of any PMD >> huge page. >> >> Signed-off-by: Nitin Gupta >> --- >> arch/sparc/mm/gup.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c >> index cd0e32b..9116a6f 100644 >> --- a/arch/sparc/mm/gup.c >> +++ b/arch/sparc/mm/gup.c >> @@ -80,6 +80,8 @@ static int gup_huge_pmd(pmd_t *pmdp, pmd_t pmd, unsigned long addr, >> refs = 0; >> head = pmd_page(pmd); >> page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT); >> + if (PageTail(head)) >> + head = compound_head(head); > Stupid question: shouldn't this go before the page calculation? No, it should be after page calculation: First, 'head' points to base of the PMD page, then 'page' points to an offset within that page. Finally, we make sure that head variable points to head of the compound page which contains the addr. I think confusion comes from the use of 'head' for pointing to a non-head page. So, maybe it would be more clear to write that part of the function this way: page = pmd_page(pmd) + ((addr & ~PMD_MASK) >> PAGE_SHIFT); head = compound_head(page); Thanks, Nitin