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
