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 <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Signed-off-by: John Hubbard <[email protected]>
---
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));
+ }
else
pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n",
page, page_ref_count(page), mapcount,
--
2.25.0
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 <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Signed-off-by: John Hubbard <[email protected]>
> ---
> 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
>
--
Kirill A. Shutemov
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/[email protected]
thanks,
--
John Hubbard
NVIDIA