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:jhubb...@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
