Re: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on DEBUG_FS

From: John Hubbard
Date: Sat Nov 07 2020 - 22:22:14 EST


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.

thanks,
--
John Hubbard
NVIDIA