Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220629150102.1582425-2-hch@lst.de> <Yrx5Lt7jrk5BiHXx@zx2c4.com>
 <20220629161020.GA24891@lst.de> <Yrx6EVHtroXeEZGp@zx2c4.com>
 <20220629161527.GA24978@lst.de> <Yrx8/Fyx15CTi2zq@zx2c4.com>
 <20220629163007.GA25279@lst.de> <Yrx/8UOY+J8Ao3Bd@zx2c4.com>
 <YryNQvWGVwCjJYmB@zx2c4.com> <Yryic4YG9X2/DJiX@google.com> <Yry6XvOGge2xKx/n@zx2c4.com>
In-Reply-To: <Yry6XvOGge2xKx/n@zx2c4.com>
From:   Kalesh Singh <kaleshsingh@google.com>
Date:   Wed, 29 Jun 2022 15:26:33 -0700
Message-ID: <CAC_TJve_Jk0+XD7VeSJVvJq4D9ZofnH69B4QZv2LPT4X3KNfeg@mail.gmail.com>
Subject: Re: [PATCH] remove CONFIG_ANDROID
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc:     Christoph Hellwig <hch@lst.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= <arve@android.com>,
        Todd Kjos <tkjos@android.com>,
        Martijn Coenen <maco@android.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Christian Brauner <brauner@kernel.org>,
        Hridya Valsaraju <hridya@google.com>,
        Suren Baghdasaryan <surenb@google.com>,
        "Theodore Ts'o" <tytso@mit.edu>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>,
        Paolo Abeni <pabeni@redhat.com>, Rob Herring <robh@kernel.org>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        Frederic Weisbecker <frederic@kernel.org>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Shuah Khan <shuah@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, wireguard@lists.zx2c4.com,
        netdev@vger.kernel.org, rcu <rcu@vger.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>, sultan@kerneltoast.com,
        android-kernel-team <android-kernel-team@google.com>,
        John Stultz <jstultz@google.com>,
        Saravana Kannan <saravanak@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jun 29, 2022 at 1:47 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Kalesh,
>
> On Wed, Jun 29, 2022 at 12:05:23PM -0700, Kalesh Singh wrote:
> > Thanks for raising this.
> >
> > Android no longer uses PM_AUTOSLEEP, is correct. libsuspend is
> > also now deprecated. Android autosuspend is initiatiated from the
> > userspace system suspend service [1].
> >
> > A runtime config sounds more reasonable since in the !PM_AUTOSLEEP
> > case, it is userspace which decides the suspend policy.
> >
> > [1] https://cs.android.com/android/platform/superproject/+/bf3906ecb33c98ff8edd96c852b884dbccb73295:system/hardware/interfaces/suspend/1.0/default/SystemSuspend.cpp;l=265
>
> Bingo, thanks for the pointer. So looking at this, I'm trying to tease
> out some heuristic that wouldn't require any changes, but I don't really
> see anything _too_ perfect. One fragment of an idea would be that the
> kernel treats things in autosuspending mode if anybody is holding open a
> fd to /sys/power/state. But I worry this would interact with
> non-autosuspending userspaces that also hold open the file. So barring
> that, I'm not quite sure.
>
> If you also can't think of something, maybe we should talk about adding
> something that requires code changes. In that line of thinking, how
> would you feel about opening /sys/power/userspace_autosuspender and
> keeping that fd open. Then the kernel API would have
> `bool pm_has_userspace_autosuspender(void)` that code could check.
> Alternatively, if you don't want refcounting fd semantics for that, just
> writing a "1" into a similar file seems fine?
>
> Any strong opinions about it? Personally it doesn't make much of a
> difference to me. The important thing is just that it'd be something
> you're willing to implement in that SystemSuspend.cpp file.

Hi Jason,

Thanks for taking a look. I'm concerned holding the sys/power/state
open would have unintentional side effects. Adding the
/sys/power/userspace_autosuspender seems more appropriate. We don't
have a use case for the refcounting, so would prefer the simpler
writing '0' / '1' to toggle semantics.

Thanks,
Kalesh

>
> Jason