[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8eaa47c0-a62d-46da-4fd6-93f2b5b2910d@nvidia.com>
Date: Sat, 7 Nov 2020 19:22:11 -0800
From: John Hubbard <jhubbard@...dia.com>
To: "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>,
Randy Dunlap <rdunlap@...radead.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Linuxarm <linuxarm@...wei.com>,
Ralph Campbell <rcampbell@...dia.com>,
John Garry <john.garry@...wei.com>
Subject: Re: [PATCH] mm/gup_benchmark: GUP_BENCHMARK depends on DEBUG_FS
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@...dia.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.
thanks,
--
John Hubbard
NVIDIA
Powered by blists - more mailing lists