Messages in this thread | | | Subject | Re: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on DEBUG_FS | From | Randy Dunlap <> | Date | Sat, 7 Nov 2020 20:11:57 -0800 |
| |
On 11/7/20 7:22 PM, John Hubbard wrote: > On 11/7/20 7:14 PM, John Hubbard wrote: >> On 11/7/20 6:58 PM, Song Bao Hua (Barry Song) wrote: >>>> On 11/7/20 2:20 PM, Randy Dunlap wrote: >>>>> On 11/7/20 11:16 AM, John Hubbard wrote: >>>>>> On 11/7/20 11:05 AM, Song Bao Hua (Barry Song) wrote: >>>>>>>> From: John Hubbard [mailto:jhubbard@nvidia.com] >>>>>> ... >>>> But if you really disagree, then I'd go with, just drop the patch entirely, because >>>> it doesn't really make things better as written...IMHO anyway. :) >>> >>> Just imagine a case, we don't enable DEBUG_FS but we enable GUP_TEST, we will >>> get an image with totally useless code section since GUP_TEST depends on debugfs >>> entry to perform any useful functionality. >>> >> >> Looking at the choices, from the user's (usually kernel developer's) experience: >> >> a) The user enables GUP_TEST, then boots up, runs, and is briefly surprised by a >> runtime failure. But it's a very quick diagnosis: "open: No such file or directory", >> when trying to make that ioctl call. The path indicates that it's a debug fs path, >> so the solution is pretty clear, at least for the main audience. >> >> b) The other choice: the user *never even sees* GUP_TEST as a choice. This especially >> bothers me because sometimes you find things by poking around in the menu, although >> of course "you should already know about it"...but there's a lot to "already know" >> in a large kernel. >> >> From a user experience, it's way better to simply see what you want, and select it >> in the menu. Or, at least get some prompt that you need to pre-select something else. >> > > ...and again, this is all fallout from Kconfig. I might be missing some advanced > feature, because it seems surprising to only be allowed to choose between > missing dependencies (which this patch would correct), or a poorer user experience > (which I argue that this patch would also provide). > > Ideally, we'd just set up the dependencies, and then have some options for > visibility, but I'm not aware of any way to do that...and after a quick peek > at Documentation/kbuild/kconfig-macro-language.rst it looks pretty bare bones.
Look at kconfig-language.rst instead.
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
e.g. drivers/hid/usbhid/Kconfig:
comment "Input core support is needed for USB HID input layer or HIDBP support" depends on USB_HID && INPUT=n
-- ~Randy
| |