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;
Date:   Thu, 30 Jun 2022 02:24:26 +0200
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     John Stultz <jstultz@google.com>
Cc:     Kalesh Singh <kaleshsingh@google.com>,
        Christoph Hellwig <hch@lst.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Arve =?utf-8?B?SGrDuG5uZXbDpWc=?= <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>,
        Saravana Kannan <saravanak@google.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH] remove CONFIG_ANDROID
Message-ID: <YrztOqBBll66C2/n@zx2c4.com>
References: <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>
 <CAC_TJve_Jk0+XD7VeSJVvJq4D9ZofnH69B4QZv2LPT4X3KNfeg@mail.gmail.com>
 <YrzaCRl9rwy9DgOC@zx2c4.com>
 <CANDhNCpRzzULaGmEGCbbJgVinA0pJJB-gOP9AY0Hy488n9ZStA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CANDhNCpRzzULaGmEGCbbJgVinA0pJJB-gOP9AY0Hy488n9ZStA@mail.gmail.com>
Precedence: bulk

Hi John,

On Wed, Jun 29, 2022 at 04:52:05PM -0700, John Stultz wrote:
> Jason: Thanks for raising this issue and sharing this patch to avoid
> breakage! I really appreciate it.
> 
> My only concern with this change introducting a userspace knob set at
> runtime, vs a (hopefully more specific than _ANDROID) kernel config is

I'd also be okay with a compile-time knob. The details make no
difference to me, so long as there's just *something* there.

> that it's not exactly clear what the flag really means (which is the
> same issue CONFIG_ANDROID has). And more problematic, with this it
> would be an ABI.
> 
> So for this we probably need to have a very clear description of what
> userland is telling the kernel. Because I'm sure userlands behavior
> will drift and shift and we'll end up litigating what kind of behavior
> is really userspace_autosleeping vs userspace_sortof_autosleeping. :)

I guess what I have in mind is the answer to these being "yes":
- "Is it very common to be asleep for only 2 seconds before being woken?"
- "Is it very common to be awake for only 2 seconds before sleeping?"

I think it'd be easiest to have a knob somewhere (compiletime,
runtime, wherever) that describes a device that exhibits those
properties. Then wireguard and other things will make a decision on how
to handle the crypto during relevant events.

> Alternatively, maybe we should switch it to describe what behavior
> change we are wanting the kernel take (instead of it hinting to the
> kernel what to expect from userland's behavior)? That way it might be
> more specific.

As a general rule, I don't expose knobs like that in wireguard /itself/,
but wireguard has no problem with adapting to whatever machine properties
it finds itself on. And besides, this *is* a very definite device
property, something really particular and peculiar about the machine
the kernel is running on. It's a concrete thing that the kernel should
know about. So let's go with your "very clear description idea", above,
instead.

So taken together, I guess there are two approaches to this:

1) Introduce a simple CONFIG_PM_CONTINUOUS_AUTOSLEEPING Kconfig thing
   with lots of discouraging help text.

2) Go with the /sys/power tunable and bikeshed the naming of that a bit
   to get it to something that reflects this better, and document it as
   being undesirable except for Android phones.

It seems like in both cases, the key will be getting the naming right.

Jason