lkml.org 
[lkml]   [2020]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on DEBUG_FS
Date


> -----Original Message-----
> From: John Hubbard [mailto:jhubbard@nvidia.com]
> Sent: Sunday, November 8, 2020 5:56 PM
> To: Randy Dunlap <rdunlap@infradead.org>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; akpm@linux-foundation.org;
> linux-mm@kvack.org; linux-kernel@vger.kernel.org
> Cc: Linuxarm <linuxarm@huawei.com>; Ralph Campbell
> <rcampbell@nvidia.com>; John Garry <john.garry@huawei.com>
> Subject: Re: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on
> DEBUG_FS
>
> On 11/7/20 8:11 PM, Randy Dunlap wrote:
> ...
> > Look at kconfig-language.rst instead.
>
> aha, yes.
>
> >
> > One thing that could be done (and is done in a few places for other reasons)
> is to add
> > a Kconfig comment if DEBUG_FS is not enabled:
> >
> > comment "GUP_TEST needs to have DEBUG_FS enabled"
> > depends on !GUP_TEST && !DEBUG_FS
> >
>
> Sweet--I just applied that here, and it does exactly what I wanted: puts a nice
> clear
> message on the "make menuconfig" screen. No more hidden item. Brilliant!
>
> Let's go with that, shall we?

Do you want this

(Code A)

diff --git a/mm/Kconfig b/mm/Kconfig
index 01b0ae0cd9d3..d80839d1fad8 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -853,6 +853,9 @@ config GUP_TEST

See tools/testing/selftests/vm/gup_test.c

+comment "GUP_TEST needs to have DEBUG_FS enabled"
+ depends on !GUP_TEST && !DEBUG_FS
+
config GUP_GET_PTE_LOW_HIGH
bool

Or do you want this ?

(Code B)
diff --git a/mm/Kconfig b/mm/Kconfig
index 01b0ae0cd9d3..a7ff0d31afd5 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -836,6 +836,7 @@ config PERCPU_STATS

config GUP_TEST
bool "Enable infrastructure for get_user_pages()-related unit tests"
+ depends on DEBUG_FS
help
Provides /sys/kernel/debug/gup_test, which in turn provides a way
to make ioctl calls that can launch kernel-based unit tests for
@@ -853,6 +854,9 @@ config GUP_TEST

See tools/testing/selftests/vm/gup_test.c

+comment "GUP_TEST needs to have DEBUG_FS enabled"
+ depends on !GUP_TEST && !DEBUG_FS
+
config GUP_GET_PTE_LOW_HIGH
bool

To be honest, I am not a big fan of both of code A and B. I think "depends on" has
clearly said everything the redundant comment wants to say.

│ Symbol: GUP_TEST [=]
│ Type : bool
│ Defined at mm/Kconfig:837
│ Prompt: Enable infrastructure for get_user_pages()-related unit tests
│ Depends on: DEBUG_FS [=n]
│ Location:
│ (1) -> Memory Management option
Menuconfig shows GUP_TEST depends on DEBUG_FS and right now DEBUG_FS is
"n". so it is impossible to enable GUP_TEST.

"comment" is a good thing, but it is more likely to be used for a menu or a group
of configurations to extend a bundle of things.

On the other hand, If this particular case needs this comment, so do countless
other configurations in hundreds of Kconfig files.

My favorite order is
1. depends on
2. select
3. depends on + comment(code B)
4. comment only(code A)

I won't accept 4, but it seems we can't get agreement on 1 which is my favorite.
So if you want either one of 2 and 3, I am happy to send v2 for that. So which one
is your favorite? 2 or 3?

>
> thanks,
> --
> John Hubbard
> NVIDIA

Thanks
Barry
\
 
 \ /
  Last update: 2020-11-08 08:36    [W:0.054 / U:1.444 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site