> -----Original Message----- > From: John Hubbard [mailto:jhubbard@xxxxxxxxxx] > Sent: Sunday, November 8, 2020 5:56 PM > To: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>; Song Bao Hua (Barry Song) > <song.bao.hua@xxxxxxxxxxxxx>; akpm@xxxxxxxxxxxxxxxxxxxx; > linux-mm@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Cc: Linuxarm <linuxarm@xxxxxxxxxx>; Ralph Campbell > <rcampbell@xxxxxxxxxx>; John Garry <john.garry@xxxxxxxxxx> > 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
