Re: [PATCH v3 10/12] mm/gup: /proc/vmstat: pin_user_pages (FOLL_PIN) reporting

On 2/3/20 1:34 PM, John Hubbard wrote:

On 2/3/20 1:30 PM, Kirill A. Shutemov wrote:

quoted

On Mon, Feb 03, 2020 at 01:04:04PM -0800, John Hubbard wrote:

quoted

On 2/3/20 5:53 AM, Kirill A. Shutemov wrote:

quoted

On Fri, Jan 31, 2020 at 07:40:27PM -0800, John Hubbard wrote:

quoted

diff --git a/mm/gup.c b/mm/gup.c
index c10d0d051c5b..9fe61d15fc0e 100644
--- a/mm/gup.c
+++ b/mm/gup.c

@@ -29,6 +29,19 @@ struct follow_page_context {
 	unsigned int page_mask;
 };
 
+#ifdef CONFIG_DEBUG_VM

Why under CONFIG_DEBUG_VM? There's nothing about this in the cover letter.

Early on, gup_benchmark showed a really significant slowdown from using these 
counters. And I don't doubt that it's still the case.

I'll re-measure and add a short summary and a few numbers to the patch commit
description, and to the v4 cover letter.

Looks like you'll show zeros for these counters if debug is off. It can be
confusing to the user. I think these counters should go away if you don't
count them.

OK, that's a good point. (And in fact, the counters==0 situation already led me
astray briefly while debugging with Leon R, even. heh.) I'll remove them entirely for
the !CONFIG_DEBUG_VM case.

On second thought, let me do some more careful performance testing. I don't recall
now if I was just removing every possible perf slowdown item, when I made this decision.
It could be that the perf is not affected, and I could just leave this feature enabled
at all times, which would be nicer.

And after all, these counters were designed for pretty hot-path items. I'll report back
with results...


thanks,
-- 
John Hubbard
NVIDIA