* Re: + mm-gup-track-foll_pin-pages-fix-2.patch added to -mm tree [not found] <20200307232517.0eSNFUWfN%akpm@linux-foundation.org> @ 2020-03-11 1:46 ` John Hubbard 2020-03-11 10:13 ` Claudio Imbrenda 0 siblings, 1 reply; 4+ messages in thread From: John Hubbard @ 2020-03-11 1:46 UTC (permalink / raw) To: Andrew Morton, imbrenda, mm-commits, Linux-MM On 3/7/20 3:25 PM, Andrew Morton wrote: > > The patch titled > Subject: mm/gup: fixup for 9947ea2c1e608e32 "mm/gup: track FOLL_PIN pages" > has been added to the -mm tree. Its filename is > mm-gup-track-foll_pin-pages-fix-2.patch > This appears to be causing a build failure in mm/gup.c, in mmotm, as of today anyway: commit 5d9ee416b570 ("pci: test for unexpectedly disabled bridges") I vaguely recall reviewing an earlier version that moved put_compound_head() up higher in gup.c, in order to avoid this. In any case, that's what this needs in order to build. Sorry for missing it in the review. Claudio, did you compile-test the patch? I'm not sure how this made it all the way to mmotm without it triggering something, either, but I can't see anything obviously wrong with my tree, which has no local changes applied. Here's what I'm seeing, and it makes sense because put_compound_head() is actually at line 2323: mm/gup.c: In function ‘gup_pte_range’: mm/gup.c:2177:4: error: implicit declaration of function ‘put_compound_head’; did you mean ‘set_compound_head’? [-Werror=implicit-function-declaration] 2177 | put_compound_head(head, 1, flags); | ^~~~~~~~~~~~~~~~~ | set_compound_head mm/gup.c: At top level: mm/gup.c:2323:13: warning: conflicting types for ‘put_compound_head’ 2323 | static void put_compound_head(struct page *page, int refs, unsigned int flags) | ^~~~~~~~~~~~~~~~~ mm/gup.c:2323:13: error: static declaration of ‘put_compound_head’ follows non-static declaration mm/gup.c:2177:4: note: previous implicit declaration of ‘put_compound_head’ was here 2177 | put_compound_head(head, 1, flags); | ^~~~~~~~~~~~~~~~~ And here's a compile-tested (only) proposed fix: diff --git a/mm/gup.c b/mm/gup.c index 722fcdd32ab8..6344252f0632 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2116,6 +2116,28 @@ static inline pte_t gup_get_pte(pte_t *ptep) } #endif /* CONFIG_GUP_GET_PTE_LOW_HIGH */ +static void put_compound_head(struct page *page, int refs, unsigned int flags) +{ + if (flags & FOLL_PIN) { + mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_RELEASED, + refs); + + if (hpage_pincount_available(page)) + hpage_pincount_sub(page, refs); + else + refs *= GUP_PIN_COUNTING_BIAS; + } + + VM_BUG_ON_PAGE(page_ref_count(page) < refs, page); + /* + * Calling put_page() for each ref is unnecessarily slow. Only the last + * ref needs a put_page(). + */ + if (refs > 1) + page_ref_sub(page, refs - 1); + put_page(page); +} + static void __maybe_unused undo_dev_pagemap(int *nr, int nr_start, unsigned int flags, struct page **pages) @@ -2320,28 +2342,6 @@ static int record_subpages(struct page *page, unsigned long addr, return nr; } -static void put_compound_head(struct page *page, int refs, unsigned int flags) -{ - if (flags & FOLL_PIN) { - mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_RELEASED, - refs); - - if (hpage_pincount_available(page)) - hpage_pincount_sub(page, refs); - else - refs *= GUP_PIN_COUNTING_BIAS; - } - - VM_BUG_ON_PAGE(page_ref_count(page) < refs, page); - /* - * Calling put_page() for each ref is unnecessarily slow. Only the last - * ref needs a put_page(). - */ - if (refs > 1) - page_ref_sub(page, refs - 1); - put_page(page); -} - #ifdef CONFIG_ARCH_HAS_HUGEPD static unsigned long hugepte_addr_end(unsigned long addr, unsigned long end, unsigned long sz) thanks, -- John Hubbard NVIDIA > This patch should soon appear at > http://ozlabs.org/~akpm/mmots/broken-out/mm-gup-track-foll_pin-pages-fix-2.patch > and later at > http://ozlabs.org/~akpm/mmotm/broken-out/mm-gup-track-foll_pin-pages-fix-2.patch > > Before you just go and hit "reply", please: > a) Consider who else should be cc'ed > b) Prefer to cc a suitable mailing list as well > c) Ideally: find the original patch on the mailing list and do a > reply-to-all to that, adding suitable additional cc's > > *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** > > The -mm tree is included into linux-next and is updated > there every 3-4 working days > > ------------------------------------------------------ > From: Claudio Imbrenda <imbrenda@linux.ibm.com> > Subject: mm/gup: fixup for 9947ea2c1e608e32 "mm/gup: track FOLL_PIN pages" > > In case pin fails, we need to unpin, a simple put_page will not be enough > > fixup for commit 9947ea2c1e608e32 ("mm/gup: track FOLL_PIN pages") > > it can be simply squashed in > > Link: http://lkml.kernel.org/r/20200306132537.783769-2-imbrenda@linux.ibm.com > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > Reviewed-by: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > mm/gup.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/mm/gup.c~mm-gup-track-foll_pin-pages-fix-2 > +++ a/mm/gup.c > @@ -2065,7 +2065,7 @@ static int gup_pte_range(pmd_t pmd, unsi > goto pte_unmap; > > if (unlikely(pte_val(pte) != pte_val(*ptep))) { > - put_page(head); > + put_compound_head(head, 1, flags); > goto pte_unmap; > } > > _ > > Patches currently in -mm which might be from imbrenda@linux.ibm.com are > > mm-gup-track-foll_pin-pages-fix-2.patch > mm-gup-writeback-add-callbacks-for-inaccessible-pages.patch > ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: + mm-gup-track-foll_pin-pages-fix-2.patch added to -mm tree 2020-03-11 1:46 ` + mm-gup-track-foll_pin-pages-fix-2.patch added to -mm tree John Hubbard @ 2020-03-11 10:13 ` Claudio Imbrenda 2020-03-12 0:14 ` John Hubbard 2020-03-12 0:51 ` Andrew Morton 0 siblings, 2 replies; 4+ messages in thread From: Claudio Imbrenda @ 2020-03-11 10:13 UTC (permalink / raw) To: John Hubbard; +Cc: Andrew Morton, mm-commits, Linux-MM On Tue, 10 Mar 2020 18:46:40 -0700 John Hubbard <jhubbard@nvidia.com> wrote: > On 3/7/20 3:25 PM, Andrew Morton wrote: > > > > The patch titled > > Subject: mm/gup: fixup for 9947ea2c1e608e32 "mm/gup: track > > FOLL_PIN pages" has been added to the -mm tree. Its filename is > > mm-gup-track-foll_pin-pages-fix-2.patch > > > > This appears to be causing a build failure in mm/gup.c, in mmotm, as > of today anyway: > > commit 5d9ee416b570 ("pci: test for unexpectedly disabled > bridges") > > > I vaguely recall reviewing an earlier version that moved > put_compound_head() up higher in gup.c, in order to avoid this. In yes, that is what I had sent > any case, that's what this needs in order to build. Sorry for missing > it in the review. I don't think you missed it, because the patch I sent out to be squashed did move it up > Claudio, did you compile-test the patch? I'm not sure how this made not only compile tested, I even actually tested-tested it. > it all the way to mmotm without it triggering something, either, but > I can't see anything obviously wrong with my tree, which has no local > changes applied. > > Here's what I'm seeing, and it makes sense because > put_compound_head() is actually at line 2323: the patch I had sent out did move put_compound_head to around line 116 > mm/gup.c: In function ‘gup_pte_range’: > mm/gup.c:2177:4: error: implicit declaration of function > ‘put_compound_head’; did you mean ‘set_compound_head’? > [-Werror=implicit-function-declaration] 2177 | > put_compound_head(head, 1, flags); | ^~~~~~~~~~~~~~~~~ | > set_compound_head mm/gup.c: At top level: > mm/gup.c:2323:13: warning: conflicting types for ‘put_compound_head’ > 2323 | static void put_compound_head(struct page *page, int refs, > unsigned int flags) | ^~~~~~~~~~~~~~~~~ > mm/gup.c:2323:13: error: static declaration of ‘put_compound_head’ > follows non-static declaration mm/gup.c:2177:4: note: previous > implicit declaration of ‘put_compound_head’ was here 2177 | > put_compound_head(head, 1, flags); | ^~~~~~~~~~~~~~~~~ > > And here's a compile-tested (only) proposed fix: I think the most appropriate place would be right after try_grab_compound_head, which is where I had put it. > diff --git a/mm/gup.c b/mm/gup.c > index 722fcdd32ab8..6344252f0632 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -2116,6 +2116,28 @@ static inline pte_t gup_get_pte(pte_t *ptep) > } > #endif /* CONFIG_GUP_GET_PTE_LOW_HIGH */ > > +static void put_compound_head(struct page *page, int refs, unsigned > int flags) +{ > + if (flags & FOLL_PIN) { > + mod_node_page_state(page_pgdat(page), > NR_FOLL_PIN_RELEASED, > + refs); > + > + if (hpage_pincount_available(page)) > + hpage_pincount_sub(page, refs); > + else > + refs *= GUP_PIN_COUNTING_BIAS; > + } > + > + VM_BUG_ON_PAGE(page_ref_count(page) < refs, page); > + /* > + * Calling put_page() for each ref is unnecessarily slow. > Only the last > + * ref needs a put_page(). > + */ > + if (refs > 1) > + page_ref_sub(page, refs - 1); > + put_page(page); > +} > + > static void __maybe_unused undo_dev_pagemap(int *nr, int nr_start, > unsigned int flags, > struct page **pages) > @@ -2320,28 +2342,6 @@ static int record_subpages(struct page *page, > unsigned long addr, return nr; > } > > -static void put_compound_head(struct page *page, int refs, unsigned > int flags) -{ > - if (flags & FOLL_PIN) { > - mod_node_page_state(page_pgdat(page), > NR_FOLL_PIN_RELEASED, > - refs); > - > - if (hpage_pincount_available(page)) > - hpage_pincount_sub(page, refs); > - else > - refs *= GUP_PIN_COUNTING_BIAS; > - } > - > - VM_BUG_ON_PAGE(page_ref_count(page) < refs, page); > - /* > - * Calling put_page() for each ref is unnecessarily slow. > Only the last > - * ref needs a put_page(). > - */ > - if (refs > 1) > - page_ref_sub(page, refs - 1); > - put_page(page); > -} > - > #ifdef CONFIG_ARCH_HAS_HUGEPD > static unsigned long hugepte_addr_end(unsigned long addr, unsigned > long end, unsigned long sz) > > > > thanks, ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + mm-gup-track-foll_pin-pages-fix-2.patch added to -mm tree 2020-03-11 10:13 ` Claudio Imbrenda @ 2020-03-12 0:14 ` John Hubbard 2020-03-12 0:51 ` Andrew Morton 1 sibling, 0 replies; 4+ messages in thread From: John Hubbard @ 2020-03-12 0:14 UTC (permalink / raw) To: Claudio Imbrenda; +Cc: Andrew Morton, mm-commits, Linux-MM On 3/11/20 3:13 AM, Claudio Imbrenda wrote: > On Tue, 10 Mar 2020 18:46:40 -0700 > John Hubbard <jhubbard@nvidia.com> wrote: > >> On 3/7/20 3:25 PM, Andrew Morton wrote: >>> >>> The patch titled >>> Subject: mm/gup: fixup for 9947ea2c1e608e32 "mm/gup: track >>> FOLL_PIN pages" has been added to the -mm tree. Its filename is >>> mm-gup-track-foll_pin-pages-fix-2.patch >>> >> >> This appears to be causing a build failure in mm/gup.c, in mmotm, as >> of today anyway: >> >> commit 5d9ee416b570 ("pci: test for unexpectedly disabled >> bridges") >> >> >> I vaguely recall reviewing an earlier version that moved >> put_compound_head() up higher in gup.c, in order to avoid this. In > > yes, that is what I had sent > >> any case, that's what this needs in order to build. Sorry for missing >> it in the review. > > I don't think you missed it, because the patch I sent out to be > squashed did move it up > >> Claudio, did you compile-test the patch? I'm not sure how this made > > not only compile tested, I even actually tested-tested it. Andrew, do you want Claudio to resend that patch? It seems like part of it got lost, somehow. thanks, -- John Hubbard NVIDIA ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + mm-gup-track-foll_pin-pages-fix-2.patch added to -mm tree 2020-03-11 10:13 ` Claudio Imbrenda 2020-03-12 0:14 ` John Hubbard @ 2020-03-12 0:51 ` Andrew Morton 1 sibling, 0 replies; 4+ messages in thread From: Andrew Morton @ 2020-03-12 0:51 UTC (permalink / raw) To: Claudio Imbrenda; +Cc: John Hubbard, mm-commits, Linux-MM On Wed, 11 Mar 2020 11:13:52 +0100 Claudio Imbrenda <imbrenda@linux.ibm.com> wrote: > > any case, that's what this needs in order to build. Sorry for missing > > it in the review. > > I don't think you missed it, because the patch I sent out to be > squashed did move it up <figures out what happened> hm, OK, http://lkml.kernel.org/r/20200306132537.783769-2-imbrenda@linux.ibm.com is billed as a fixup for 9947ea2c1e608e32 "mm/gup: track FOLL_PIN pages" but it actually applies after everything else. I messed up untangling this. Redone, should be OK now. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-12 0:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200307232517.0eSNFUWfN%akpm@linux-foundation.org> 2020-03-11 1:46 ` + mm-gup-track-foll_pin-pages-fix-2.patch added to -mm tree John Hubbard 2020-03-11 10:13 ` Claudio Imbrenda 2020-03-12 0:14 ` John Hubbard 2020-03-12 0:51 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).