* [PATCH] sparc64: Fix gup_huge_pmd @ 2017-06-21 21:50 ` Nitin Gupta 0 siblings, 0 replies; 8+ messages in thread From: Nitin Gupta @ 2017-06-21 21:50 UTC (permalink / raw) To: David S. Miller Cc: David S. Miller, Nitin Gupta, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel 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 <nitin.m.gupta@oracle.com> --- 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); do { VM_BUG_ON(compound_head(page) != head); pages[*nr] = page; -- 2.9.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] sparc64: Fix gup_huge_pmd @ 2017-06-21 21:50 ` Nitin Gupta 0 siblings, 0 replies; 8+ messages in thread From: Nitin Gupta @ 2017-06-21 21:50 UTC (permalink / raw) To: David S. Miller Cc: Nitin Gupta, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel 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 <nitin.m.gupta@oracle.com> --- 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); do { VM_BUG_ON(compound_head(page) != head); pages[*nr] = page; -- 2.9.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] sparc64: Fix gup_huge_pmd 2017-06-21 21:50 ` Nitin Gupta @ 2017-06-22 10:53 ` Julian Calaby -1 siblings, 0 replies; 8+ messages in thread From: Julian Calaby @ 2017-06-22 10:53 UTC (permalink / raw) To: Nitin Gupta Cc: David S. Miller, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel Hi Nitin, On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gupta@oracle.com> 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 <nitin.m.gupta@oracle.com> > --- > 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? > do { > VM_BUG_ON(compound_head(page) != head); > pages[*nr] = page; Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparc64: Fix gup_huge_pmd @ 2017-06-22 10:53 ` Julian Calaby 0 siblings, 0 replies; 8+ messages in thread From: Julian Calaby @ 2017-06-22 10:53 UTC (permalink / raw) To: Nitin Gupta Cc: David S. Miller, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel Hi Nitin, On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gupta@oracle.com> 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 <nitin.m.gupta@oracle.com> > --- > 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? > do { > VM_BUG_ON(compound_head(page) != head); > pages[*nr] = page; Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparc64: Fix gup_huge_pmd 2017-06-22 10:53 ` Julian Calaby @ 2017-06-22 14:37 ` Nitin Gupta -1 siblings, 0 replies; 8+ messages in thread From: Nitin Gupta @ 2017-06-22 14:37 UTC (permalink / raw) To: Julian Calaby Cc: David S. Miller, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel Hi Julian, On 6/22/17 3:53 AM, Julian Calaby wrote: > On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gupta@oracle.com> 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 <nitin.m.gupta@oracle.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparc64: Fix gup_huge_pmd @ 2017-06-22 14:37 ` Nitin Gupta 0 siblings, 0 replies; 8+ messages in thread From: Nitin Gupta @ 2017-06-22 14:37 UTC (permalink / raw) To: Julian Calaby Cc: David S. Miller, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel Hi Julian, On 6/22/17 3:53 AM, Julian Calaby wrote: > On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gupta@oracle.com> 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 <nitin.m.gupta@oracle.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparc64: Fix gup_huge_pmd 2017-06-22 14:37 ` Nitin Gupta @ 2017-06-22 14:45 ` Julian Calaby -1 siblings, 0 replies; 8+ messages in thread From: Julian Calaby @ 2017-06-22 14:45 UTC (permalink / raw) To: Nitin Gupta Cc: David S. Miller, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel Hi Nitin, On Fri, Jun 23, 2017 at 12:37 AM, Nitin Gupta <nitin.m.gupta@oracle.com> wrote: > Hi Julian, > > > On 6/22/17 3:53 AM, Julian Calaby wrote: >> >> On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gupta@oracle.com> >> 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 <nitin.m.gupta@oracle.com> >>> --- >>> 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. Thanks for the explanation, that makes a bit more sense to me. > 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); More verbose variable names would help too. =) Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] sparc64: Fix gup_huge_pmd @ 2017-06-22 14:45 ` Julian Calaby 0 siblings, 0 replies; 8+ messages in thread From: Julian Calaby @ 2017-06-22 14:45 UTC (permalink / raw) To: Nitin Gupta Cc: David S. Miller, Lorenzo Stoakes, Jan Kara, Michal Hocko, sparclinux, linux-kernel Hi Nitin, On Fri, Jun 23, 2017 at 12:37 AM, Nitin Gupta <nitin.m.gupta@oracle.com> wrote: > Hi Julian, > > > On 6/22/17 3:53 AM, Julian Calaby wrote: >> >> On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gupta@oracle.com> >> 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 <nitin.m.gupta@oracle.com> >>> --- >>> 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. Thanks for the explanation, that makes a bit more sense to me. > 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); More verbose variable names would help too. =) Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-06-22 14:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-21 21:50 [PATCH] sparc64: Fix gup_huge_pmd Nitin Gupta 2017-06-21 21:50 ` Nitin Gupta 2017-06-22 10:53 ` Julian Calaby 2017-06-22 10:53 ` Julian Calaby 2017-06-22 14:37 ` Nitin Gupta 2017-06-22 14:37 ` Nitin Gupta 2017-06-22 14:45 ` Julian Calaby 2017-06-22 14:45 ` Julian Calaby
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.