RE: [PATCH] mm/gup_benchmark: update the documentation in Kconfig

[Song Bao Hua (Barry Song)]

> -----Original Message-----
> From: linux-kselftest-owner@xxxxxxxxxxxxxxx
> [mailto:linux-kselftest-owner@xxxxxxxxxxxxxxx] On Behalf Of John Hubbard
> Sent: Friday, August 21, 2020 3:51 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>;
> akpm@xxxxxxxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx;
> linux-kselftest@xxxxxxxxxxxxxxx
> Cc: Linuxarm <linuxarm@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Keith
> Busch <keith.busch@xxxxxxxxx>; Ira Weiny <ira.weiny@xxxxxxxxx>; Kirill A .
> Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH] mm/gup_benchmark: update the documentation in
> Kconfig
> 
> On 8/20/20 8:25 PM, Barry Song wrote:
> > In the beginning, mm/gup_benchmark.c supported get_user_pages_fast()
> > only, but right now, it supports the benchmarking of a couple of
> > get_user_pages() related calls like:
> > * get_user_pages_fast()
> > * get_user_pages()
> > * pin_user_pages_fast()
> > * pin_user_pages()
> > The documentation is confusing and needs update.
> 
> hmmm, it's not that confusing, given that pin_user_pages() and
> get_user_pages() use the same underlying get_user_pages()
> implementation.

get_user_pages_fast() is not get_user_pages(). It is a specific function :-)

In the beginning, gup_benchmark was only for get_user_pages_fast():
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/mm/gup_benchmark.c?id=64c349f4ae78


> 
> >
> > Cc: John Hubbard <jhubbard@xxxxxxxxxx>
> > Cc: Keith Busch <keith.busch@xxxxxxxxx>
> > Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
> > Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > Signed-off-by: Barry Song <song.bao.hua@xxxxxxxxxxxxx>
> > ---
> >   mm/Kconfig | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 6c974888f86f..f7c9374da7b3 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -831,10 +831,10 @@ config PERCPU_STATS
> >   	  be used to help understand percpu memory usage.
> >
> >   config GUP_BENCHMARK
> > -	bool "Enable infrastructure for get_user_pages_fast() benchmarking"
> > +	bool "Enable infrastructure for get_user_pages() and related calls
> benchmarking"
> 
> If we really want to go to the trouble of tweaking this, then I'd go with
> something more like:
> 
> "Enable infrastructure for get_user_pages() and pin_user_pages benchmarking"
> 
> ...but I don't think it really warrants a patch just yet. *However*, my
> judgment is skewed right now, because I'm planning a small patchset to split
> up gup_benchmark a little bit, and to add some more testing and take
> advantage
> of parts of it to do a dump_page() test. At which point "related calls" would
> make more sense, but then it would be different enough that this patch would
> still need changing.

Ok. Fair enough. I was copying the "related calls" from:
Documentation/core-api/pin_user_pages.rst

> 
> So I'm inclined to just recommend leaving this alone for a bit, but if others
> want to put it in, I'm OK with that too.
> 
> >   	help
> >   	  Provides /sys/kernel/debug/gup_benchmark that helps with testing
> > -	  performance of get_user_pages_fast().
> > +	  performance of get_user_pages() and related calls.
> >
> >   	  See tools/testing/selftests/vm/gup_benchmark.c
> >

Thanks
Barry