Re: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on DEBUG_FS

[John Hubbard]

On 11/4/20 2:05 AM, Barry Song wrote:
> Without DEBUG_FS, all the code in gup_benchmark becomes meaningless.
> For sure kernel provides debugfs stub while DEBUG_FS is disabled, but
> the point here is that GUP_BENCHMARK can do nothing without DEBUG_FS.
>
> Cc: John Hubbard <jhubbard@xxxxxxxxxx>
> Cc: Ralph Campbell <rcampbell@xxxxxxxxxx>
> Inspired-by: John Garry <john.garry@xxxxxxxxxx>
> Signed-off-by: Barry Song <song.bao.hua@xxxxxxxxxxxxx>
> ---
>   * inspired by John's comment in this patch:
>   https://lore.kernel.org/linux-iommu/184797b8-512e-e3da-fae7-25c7d662648b@xxxxxxxxxx/
>
>   mm/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index d42423f..91fa923 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -836,6 +836,7 @@ config PERCPU_STATS
>   
>   config GUP_BENCHMARK
>   	bool "Enable infrastructure for get_user_pages() and related calls benchmarking"
> +	depends on DEBUG_FS


I think "select DEBUG_FS" is better here. "depends on" has the obnoxious behavior
of hiding the choice from you, if the dependencies aren't already met. Whereas what
the developer *really* wants is a no-nonsense activation of the choice: "enable
GUP_BENCHMARK and the debug fs that it requires".

So depends on really on is better for things that you just can't control, such as
the cpu arch you're on, etc.

Also note that this will have some minor merge conflict with mmotm, Due to renaming
to GUP_TEST. No big deal though.


thanks,
--
John Hubbard
NVIDIA