On Wed, Sep 28, 2022 at 9:01 AM Sebastian Andrzej Siewior
<[email protected]> wrote:
>
> On 2022-09-28 06:49:34 [+0000], Ren Zhijie wrote:
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1273,6 +1273,7 @@ endif # NAMESPACES
> >
> > config CHECKPOINT_RESTORE
> > bool "Checkpoint/restore support"
> > + select PROC_FS
>
> Couldn't this become a depends?
>
It could also be a depends (to resolve the warning).
It is just the question whether:
When PROC_FS is not set, should the CHECKPOINT_RESTORE still be
visible as a config option to add (and then automatically add
PROC_FS)? Then select is right here.
or:
When PROC_FS is not set, should the CHECKPOINT_RESTORE not be visible
as a config option to add? Instead the user first needs to add
PROC_FS, then CHECKPOINT_RESTORE becomes visible as an option to add,
and then the user can add it. Then depends would be right.
For me, both seem reasonable. So, I assume Ren considered select the
better choice.
But maybe Ren can confirm.
A kernel build configuration without PROC_FS is quite special
anyway... and then being interested in CHECKPOINT_ RESTORE for such a
system is really really special. I wonder if that user then really
knows what he or she is configuring at that point.
Lukas
On 2022-09-28 09:20:42 [+0200], Lukas Bulwahn wrote:
> > Couldn't this become a depends?
> It could also be a depends (to resolve the warning).
…
> It is just the question whether:
>
> When PROC_FS is not set, should the CHECKPOINT_RESTORE still be
> visible as a config option to add (and then automatically add
> PROC_FS)? Then select is right here.
then CHECKPOINT_RESTORE is the only option selecting PROC_FS while
everyone else depends on it _or_ avoids using it in the absence of
PROC_FS.
Sebastian
On Wed, Sep 28, 2022, at 11:14 AM, Sebastian Andrzej Siewior wrote:
> On 2022-09-28 09:20:42 [+0200], Lukas Bulwahn wrote:
>> > Couldn't this become a depends?
>> It could also be a depends (to resolve the warning).
> …
>> It is just the question whether:
>>
>> When PROC_FS is not set, should the CHECKPOINT_RESTORE still be
>> visible as a config option to add (and then automatically add
>> PROC_FS)? Then select is right here.
>
> then CHECKPOINT_RESTORE is the only option selecting PROC_FS while
> everyone else depends on it _or_ avoids using it in the absence of
> PROC_FS.
Right, we should not mix 'select' and 'depends on' for the same
symbol, as that leads to circular dependencies and general
confusion.
If there is no way to use CHECKPOINT_RESTORE without procfs,
then the symbol should just not be visible (it will still show
up with the dependency when one searches in menuconfig).
Force-enabling a major subsystem like procfs from another
symbol is not a good solution.
Arnd
On Wed, Sep 28, 2022 at 11:32 AM Arnd Bergmann <[email protected]> wrote:
>
> On Wed, Sep 28, 2022, at 11:14 AM, Sebastian Andrzej Siewior wrote:
> > On 2022-09-28 09:20:42 [+0200], Lukas Bulwahn wrote:
> >> > Couldn't this become a depends?
> >> It could also be a depends (to resolve the warning).
> > …
> >> It is just the question whether:
> >>
> >> When PROC_FS is not set, should the CHECKPOINT_RESTORE still be
> >> visible as a config option to add (and then automatically add
> >> PROC_FS)? Then select is right here.
> >
> > then CHECKPOINT_RESTORE is the only option selecting PROC_FS while
> > everyone else depends on it _or_ avoids using it in the absence of
> > PROC_FS.
>
> Right, we should not mix 'select' and 'depends on' for the same
> symbol, as that leads to circular dependencies and general
> confusion.
>
> If there is no way to use CHECKPOINT_RESTORE without procfs,
> then the symbol should just not be visible (it will still show
> up with the dependency when one searches in menuconfig).
> Force-enabling a major subsystem like procfs from another
> symbol is not a good solution.
>
Agree. I retract my Reviewed-by.
The arguments are clear to make this depend on PROC_FS.
Lukas