On Wed, Aug 24, 2022 at 3:36 PM Juhyung Park <qkrwngud...@gmail.com> wrote:
>
> Hi Rafael,
>
> On Wed, Aug 24, 2022 at 10:11 PM Rafael J. Wysocki <raf...@kernel.org> wrote:
> >
> > On Wed, Aug 24, 2022 at 6:41 AM Juhyung Park <qkrwngud...@gmail.com> wrote:
> > >
> > > Commit 2fd77fff4b44 ("PM / suspend: make sync() on suspend-to-RAM 
> > > build-time
> > > optional") added an option to skip sync() on suspend entry to avoid heavy
> > > overhead on platforms with frequent suspends.
> > >
> > > Years later, commit 261e224d6a5c ("pm/sleep: Add PM_USERSPACE_AUTOSLEEP
> > > Kconfig") added a dedicated config for indicating that the kernel is 
> > > subject to
> > > frequent suspends.
> > >
> > > While SUSPEND_SKIP_SYNC is also available as a knob that the userspace can
> > > configure, it makes sense to enable this by default if 
> > > PM_USERSPACE_AUTOSLEEP
> > > is selected already.
> > >
> > > Signed-off-by: Juhyung Park <qkrwngud...@gmail.com>
> > > ---
> > >  kernel/power/Kconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > > index 60a1d3051cc7..5725df6c573b 100644
> > > --- a/kernel/power/Kconfig
> > > +++ b/kernel/power/Kconfig
> > > @@ -23,6 +23,7 @@ config SUSPEND_SKIP_SYNC
> > >         bool "Skip kernel's sys_sync() on suspend to RAM/standby"
> > >         depends on SUSPEND
> > >         depends on EXPERT
> > > +       default PM_USERSPACE_AUTOSLEEP
> >
> > Why is this better than selecting SUSPEND_SKIP_SYNC from 
> > PM_USERSPACE_AUTOSLEEP?
>
> That won't allow developers to opt-out from SUSPEND_SKIP_SYNC when
> they still want PM_USERSPACE_AUTOSLEEP.

I see.

It is not particularly clear, so at least please mention it in the changelog.


> (Can't think of a valid reason
> for this though, as PM_USERSPACE_AUTOSLEEP is only used by Android and
> probably Chromium, afaik.)
>
> I don't think SUSPEND_SKIP_SYNC is critical enough to enforce when
> PM_USERSPACE_AUTOSLEEP is enabled, but I don't have a strong opinion
> on this either.
> (We could do `imply SUSPEND_SKIP_SYNC` from PM_USERSPACE_AUTOSLEEP,
> but that doesn't look good semantically imho.)
>
> If you want, I can send a v2 with 'PM_USERSPACE_AUTOSLEEP select
> SUSPEND_SKIP_SYNC'.

Personally, I would use "select" and I would amend the
PM_USERSPACE_AUTOSLEEP help text to say that it will disable sync on
suspend by default explicitly.

IMV otherwise it is more confusing than it needs to be.

I'm also wondering about a particular use case addressed by this
change.  Is there any?


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to