Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on DEBUG_FS
To:     Barry Song <song.bao.hua@hisilicon.com>,
        <akpm@linux-foundation.org>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>
CC:     <linuxarm@huawei.com>, Ralph Campbell <rcampbell@nvidia.com>,
        John Garry <john.garry@huawei.com>
References: <20201104100552.20156-1-song.bao.hua@hisilicon.com>
From:   John Hubbard <jhubbard@nvidia.com>
Message-ID: <e8ecbf3e-438e-934e-0335-ec9b3e097022@nvidia.com>
Date:   Fri, 6 Nov 2020 16:12:49 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <20201104100552.20156-1-song.bao.hua@hisilicon.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@nvidia.com>
> Cc: Ralph Campbell <rcampbell@nvidia.com>
> Inspired-by: John Garry <john.garry@huawei.com>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
>   * inspired by John's comment in this patch:
>   https://lore.kernel.org/linux-iommu/184797b8-512e-e3da-fae7-25c7d662648b@huawei.com/
> 
>   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