From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.zx2c4.com (lists.zx2c4.com [165.227.139.114]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 57371C433EF for ; Thu, 30 Jun 2022 10:06:08 +0000 (UTC) Received: by lists.zx2c4.com (OpenSMTPD) with ESMTP id b7c1b2d4; Thu, 30 Jun 2022 10:06:06 +0000 (UTC) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lists.zx2c4.com (OpenSMTPD) with ESMTPS id 48cef7eb (TLSv1.2:ECDHE-ECDSA-AES256-GCM-SHA384:256:NO) for ; Thu, 30 Jun 2022 10:06:03 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id F0FE562198; Thu, 30 Jun 2022 10:06:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 14DCCC34115; Thu, 30 Jun 2022 10:05:57 +0000 (UTC) Authentication-Results: smtp.kernel.org; dkim=pass (1024-bit key) header.d=zx2c4.com header.i=@zx2c4.com header.b="ACkRU+Ds" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zx2c4.com; s=20210105; t=1656583556; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=lpetoj7kI9rRA+ZeAoTE77fi6zOrRL2bKq1/FPJ526c=; b=ACkRU+DstmTaTG/qZ0irEBLPkULFIDpmiMuCN9oQNUCoD/n8pqTayt4kE4T3q3fKiM6hfE 1kdRZnJIBzQ8fCYp9BJHZ7IO5rxsJ5v1LZ3Q18QgIcZRPREkUJDOIDkZj3Lq1fTNRQTDM3 0klFcqdwCZnEM1ZY8LMAJrzSICKkUiQ= Received: by mail.zx2c4.com (ZX2C4 Mail Server) with ESMTPSA id 2d14dac7 (TLSv1.3:AEAD-AES256-GCM-SHA384:256:NO); Thu, 30 Jun 2022 10:05:55 +0000 (UTC) Date: Thu, 30 Jun 2022 12:05:50 +0200 From: "Jason A. Donenfeld" To: Kalesh Singh Cc: John Stultz , Christoph Hellwig , Greg Kroah-Hartman , Arve =?utf-8?B?SGrDuG5uZXbDpWc=?= , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Hridya Valsaraju , Suren Baghdasaryan , Theodore Ts'o , "David S. Miller" , Eric Dumazet , Jakub Kicinski , "Alex Xu (Hello71)" , Paolo Abeni , Rob Herring , "Paul E. McKenney" , Frederic Weisbecker , Neeraj Upadhyay , Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Shuah Khan , LKML , wireguard@lists.zx2c4.com, netdev@vger.kernel.org, rcu , "open list:KERNEL SELFTEST FRAMEWORK" , sultan@kerneltoast.com, android-kernel-team , Saravana Kannan , "Rafael J. Wysocki" Subject: Re: [PATCH] remove CONFIG_ANDROID Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-BeenThere: wireguard@lists.zx2c4.com X-Mailman-Version: 2.1.30rc1 Precedence: list List-Id: Development discussion of WireGuard List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: wireguard-bounces@lists.zx2c4.com Sender: "WireGuard" Hi Kalesh, On Wed, Jun 29, 2022 at 09:25:32PM -0700, Kalesh Singh wrote: > On Wed, Jun 29, 2022 at 5:30 PM Jason A. Donenfeld wrote: > > > > Hey again, > > > > On Thu, Jun 30, 2022 at 2:24 AM Jason A. Donenfeld wrote: > > > 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. > > > > One other quick thought, which I had mentioned earlier to Kalesh: > > > > 3) Make the semantics a process holding open a file descriptor, rather > > than writing 0/1 into a file. It'd be called /sys/power/ > > userspace_autosleep_ctrl, or something, and it'd enable this behavior > > while it's opened. And maybe down the line somebody will want to add > > ioctls to it for a different purpose. This way it's less of a tunable > > and more of an indication that there's a userspace app doing/controlling > > something. > > > > This idea (3) may be a lot of added complexity for basically nothing, > > but it might fit the usage semantics concerns a bit better than (2). But > > anyway, just an idea. Any one of those three are fine with me. > > Two concerns John raised: > 1) Adding new ABI we need to maintain > 2) Having unclear config options > > Another idea, I think, is to add the Kconfig option as > CONFIG_SUSPEND_SKIP_RNG_RESEED? Similar to existing > CONFIG_SUSPEND_SKIP_SYNC and I think it would address those concerns. I mentioned in my reply to him that this doesn't really work for me: | 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. IOW, we're not going to add a tunable on every possible place this is used. Anyway if you don't want a runtime switch, make a compiletime switch called CONFIG_PM_CONTINUOUS_RAPID_AUTOSLEEPING or whatever, write some very discouraging help text, and call it a day. And this way you don't have to worry about ABI and we can change this later on and do the whole thing as a no-big-deal change that somebody can tweak later without issue. The below diff is some boiler plate to help you get started with that direction. Similar order of operations for this one: 1. You write a patch for Android's base config to enable this option and post it on Gerrit. 2. You take the diff below, clean it up or bikeshed the naming a bit or do whatever there, and submit it to the kernel, including as a `Link: ...` this thread and the Gerrit link. 3. When the patch lands, you submit the Gerrit CL. 4. When both have landed, Christoph moves forward with his CONFIG_ANDROID removal. So really, just pick an option here -- the runtime switch or the compiletime switch or the crazy fd thing I mentioned -- and run with it. Jason diff --git a/drivers/char/random.c b/drivers/char/random.c index e3dd1dd3dd22..5332236cb1ad 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -756,7 +756,7 @@ static int random_pm_notification(struct notifier_block *nb, unsigned long actio if (crng_ready() && (action == PM_RESTORE_PREPARE || (action == PM_POST_SUSPEND && - !IS_ENABLED(CONFIG_PM_AUTOSLEEP) && !IS_ENABLED(CONFIG_ANDROID)))) { + !IS_ENABLED(CONFIG_PM_AUTOSLEEP) && !IS_ENABLED(CONFIG_PM_RAPID_USERSPACE_AUTOSLEEP)))) { crng_reseed(); pr_notice("crng reseeded on system resumption\n"); } diff --git a/drivers/net/wireguard/device.c b/drivers/net/wireguard/device.c index aa9a7a5970fd..b93171f2e6c9 100644 --- a/drivers/net/wireguard/device.c +++ b/drivers/net/wireguard/device.c @@ -69,7 +69,7 @@ static int wg_pm_notification(struct notifier_block *nb, unsigned long action, v * its normal operation rather than as a somewhat rare event, then we * don't actually want to clear keys. */ - if (IS_ENABLED(CONFIG_PM_AUTOSLEEP) || IS_ENABLED(CONFIG_ANDROID)) + if (IS_ENABLED(CONFIG_PM_AUTOSLEEP) || IS_ENABLED(CONFIG_PM_RAPID_USERSPACE_AUTOSLEEP)) return 0; if (action != PM_HIBERNATION_PREPARE && action != PM_SUSPEND_PREPARE) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index a12779650f15..bcbfbeb39d4f 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -150,6 +150,25 @@ config PM_WAKELOCKS Allow user space to create, activate and deactivate wakeup source objects with the help of a sysfs-based interface. +config PM_RAPID_USERSPACE_AUTOSLEEP + bool "Tune for rapid and consistent userspace calls to sleep" + depends on PM_SLEEP + help + Change the behavior of various sleep-sensitive code to deal with + userspace autosuspend daemons that put the machine to sleep and wake it + up extremely often and for short periods of time. + + This option mostly disables code paths that most users really should + keep enabled. In particular, only enable this if: + + - It is very common to be asleep for only 2 seconds before being woken; and + - It is very common to be awake for only 2 seconds before sleeping. + + This likely only applies to Android devices, and not other machines. + Therefore, you should say N here, unless you're extremely certain that + this is what you want. The option otherwise has bad, undesirable + effects, and should not be enabled just for fun. + config PM_WAKELOCKS_LIMIT int "Maximum number of user space wakeup sources (0 = no limit)" range 0 100000