| Message ID | 20200211001536.1027652-13-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | mm/gup: track FOLL_PIN pages | expand |
On Mon, Feb 10, 2020 at 04:15:36PM -0800, John Hubbard wrote: > As part of pin_user_pages() and related API calls, pages are > "dma-pinned". For the case of compound pages of order > 1, the per-page > accounting of dma pins is accomplished via the 3rd struct page in the > compound page. In order to support debugging of any pin_user_pages()- > related problems, enhance dump_page() so as to report the pin count > in that case. > > Documentation/core-api/pin_user_pages.rst is also updated accordingly. > > Cc: Jan Kara <jack@suse.cz> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Cc: Matthew Wilcox <willy@infradead.org> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > Documentation/core-api/pin_user_pages.rst | 7 +++++++ > mm/debug.c | 21 ++++++++++++++++----- > 2 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/Documentation/core-api/pin_user_pages.rst b/Documentation/core-api/pin_user_pages.rst > index 5c8a5f89756b..2e939ff10b86 100644 > --- a/Documentation/core-api/pin_user_pages.rst > +++ b/Documentation/core-api/pin_user_pages.rst > @@ -238,6 +238,13 @@ long-term [R]DMA pins in place, or during pin/unpin transitions. > (...unless it was already out of balance due to a long-term RDMA pin being in > place.) > > +Other diagnostics > +================= > + > +dump_page() has been enhanced slightly, to handle these new counting fields, and > +to better report on compound pages in general. Specifically, for compound pages > +with order > 1, the exact (hpage_pinned_refcount) pincount is reported. > + > References > ========== > > diff --git a/mm/debug.c b/mm/debug.c > index f5ffb0784559..2189357f0987 100644 > --- a/mm/debug.c > +++ b/mm/debug.c > @@ -85,11 +85,22 @@ void __dump_page(struct page *page, const char *reason) > mapcount = PageSlab(head) ? 0 : page_mapcount(page); > > if (compound) > - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " > - "index:%#lx head:%px order:%u compound_mapcount:%d\n", > - page, page_ref_count(head), mapcount, > - mapping, page_to_pgoff(page), head, > - compound_order(head), compound_mapcount(page)); > + if (hpage_pincount_available(page)) { > + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " > + "index:%#lx head:%px order:%u " > + "compound_mapcount:%d compound_pincount:%d\n", > + page, page_ref_count(head), mapcount, > + mapping, page_to_pgoff(page), head, > + compound_order(head), compound_mapcount(page), > + compound_pincount(page)); > + } else { > + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " > + "index:%#lx head:%px order:%u " > + "compound_mapcount:%d\n", > + page, page_ref_count(head), mapcount, > + mapping, page_to_pgoff(page), head, > + compound_order(head), compound_mapcount(page)); > + } Have you considered using pr_cont() here. I guess it would be easier to read. You can use my Ack anyway. > else > pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", > page, page_ref_count(page), mapcount, > -- > 2.25.0 >
On 2/11/20 5:21 AM, Kirill A. Shutemov wrote: ... >> diff --git a/mm/debug.c b/mm/debug.c >> index f5ffb0784559..2189357f0987 100644 >> --- a/mm/debug.c >> +++ b/mm/debug.c >> @@ -85,11 +85,22 @@ void __dump_page(struct page *page, const char *reason) >> mapcount = PageSlab(head) ? 0 : page_mapcount(page); >> >> if (compound) >> - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " >> - "index:%#lx head:%px order:%u compound_mapcount:%d\n", >> - page, page_ref_count(head), mapcount, >> - mapping, page_to_pgoff(page), head, >> - compound_order(head), compound_mapcount(page)); >> + if (hpage_pincount_available(page)) { >> + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " >> + "index:%#lx head:%px order:%u " >> + "compound_mapcount:%d compound_pincount:%d\n", >> + page, page_ref_count(head), mapcount, >> + mapping, page_to_pgoff(page), head, >> + compound_order(head), compound_mapcount(page), >> + compound_pincount(page)); >> + } else { >> + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " >> + "index:%#lx head:%px order:%u " >> + "compound_mapcount:%d\n", >> + page, page_ref_count(head), mapcount, >> + mapping, page_to_pgoff(page), head, >> + compound_order(head), compound_mapcount(page)); >> + } > > Have you considered using pr_cont() here. I guess it would be easier to > read. Yes, and it does have the advantage of removing some of the code duplication above. On the other hand, though, it leaves the end result (the long lines being printed) the same, and introduces a window in which the output can get garbled by another thread that is printk'-ing. And actually, what I'd really like is to shorten the printed output lines, as I mentioned in [1]. So overall, given that this series has been fairly difficult to get finalized, and it's now in Andrew's tree at last, I'd *really* like to leave it as-is right now, and build on top of it. So I will submit a follow-on patch to formally propose shortening the printed lines, and that can live or die independently of this series, which is hopefully over now. > > You can use my Ack anyway. Thanks, and I appreciate all of your reviews and bug spotting and ideas for improvements on this series, it's been really helpful. > > >> else >> pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", >> page, page_ref_count(page), mapcount, >> -- >> 2.25.0 >> > [1] https://lore.kernel.org/r/96e1f693-0e7b-2817-f13d-1946ff7654a1@nvidia.com thanks,
diff --git a/Documentation/core-api/pin_user_pages.rst b/Documentation/core-api/pin_user_pages.rst index 5c8a5f89756b..2e939ff10b86 100644 --- a/Documentation/core-api/pin_user_pages.rst +++ b/Documentation/core-api/pin_user_pages.rst @@ -238,6 +238,13 @@ long-term [R]DMA pins in place, or during pin/unpin transitions. (...unless it was already out of balance due to a long-term RDMA pin being in place.) +Other diagnostics +================= + +dump_page() has been enhanced slightly, to handle these new counting fields, and +to better report on compound pages in general. Specifically, for compound pages +with order > 1, the exact (hpage_pinned_refcount) pincount is reported. + References ========== diff --git a/mm/debug.c b/mm/debug.c index f5ffb0784559..2189357f0987 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -85,11 +85,22 @@ void __dump_page(struct page *page, const char *reason) mapcount = PageSlab(head) ? 0 : page_mapcount(page); if (compound) - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " - "index:%#lx head:%px order:%u compound_mapcount:%d\n", - page, page_ref_count(head), mapcount, - mapping, page_to_pgoff(page), head, - compound_order(head), compound_mapcount(page)); + if (hpage_pincount_available(page)) { + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " + "index:%#lx head:%px order:%u " + "compound_mapcount:%d compound_pincount:%d\n", + page, page_ref_count(head), mapcount, + mapping, page_to_pgoff(page), head, + compound_order(head), compound_mapcount(page), + compound_pincount(page)); + } else { + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " + "index:%#lx head:%px order:%u " + "compound_mapcount:%d\n", + page, page_ref_count(head), mapcount, + mapping, page_to_pgoff(page), head, + compound_order(head), compound_mapcount(page)); + } else pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", page, page_ref_count(page), mapcount,
As part of pin_user_pages() and related API calls, pages are "dma-pinned". For the case of compound pages of order > 1, the per-page accounting of dma pins is accomplished via the 3rd struct page in the compound page. In order to support debugging of any pin_user_pages()- related problems, enhance dump_page() so as to report the pin count in that case. Documentation/core-api/pin_user_pages.rst is also updated accordingly. Cc: Jan Kara <jack@suse.cz> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Matthew Wilcox <willy@infradead.org> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- Documentation/core-api/pin_user_pages.rst | 7 +++++++ mm/debug.c | 21 ++++++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-)