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@xxxxxxxxxx] >>>>>> ... >>>> 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
