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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DEB4AC5475B for ; Thu, 7 Mar 2024 01:30:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 709386B00CA; Wed, 6 Mar 2024 20:30:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 692146B00CB; Wed, 6 Mar 2024 20:30:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 559A16B00CC; Wed, 6 Mar 2024 20:30:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 46CE96B00CA for ; Wed, 6 Mar 2024 20:30:15 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 24A261C126B for ; Thu, 7 Mar 2024 01:30:15 +0000 (UTC) X-FDA: 81868512390.12.517970A Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf23.hostedemail.com (Postfix) with ESMTP id 9176A140005 for ; Thu, 7 Mar 2024 01:30:13 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Dt3vEybg; spf=pass (imf23.hostedemail.com: domain of jpoimboe@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jpoimboe@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709775013; a=rsa-sha256; cv=none; b=Q92vtwp6PptmCdHunazOsPAo744+9ZkDq5xec9s/t271jvWxIWPV09dbg6WtJdN8iDltRY ecXkcQa3s53bfI9kEUVQ5mQVMChDJ7mzlteq7b4KDYDLCIryVgr+QP4FBJZIK83HA0aqUS 5Z4qtPwbF+nauovNXF9LYLbUHnFEn54= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Dt3vEybg; spf=pass (imf23.hostedemail.com: domain of jpoimboe@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jpoimboe@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709775013; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HzCLu8BelOcK+5ludkOIggm0HsyJ16/qXqfKoR3su/A=; b=iKMDpQvdzS55MWObHY1nG7p1NbuA9kErsBs8kOh96tAcZCuPvxUNeZM0PHeiVlNYG68ivY ESO5n6pTw400fwLOC2Q+UDwXaSChfcgI3CKAze34+PvTPA6n5Bufr1cXC43neQbfyKBcZX ml2Cwp/jWpJnbCFUachvFBAQAfflnyo= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 9F52E61BB3; Thu, 7 Mar 2024 01:30:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 99348C433C7; Thu, 7 Mar 2024 01:30:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1709775012; bh=3iEyCAOIpIr+olvkgpU7l0MpcoipYtMImoe35an3NbM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Dt3vEybgbCDhmPIzPNp15wv/LSmpaIdJyanOUje6Kg2JNG0WyAqol6dA5vQqYgSbX ZfjujaGjut999mzEYrAZKINbQhKWRKT9u1Ca/qh8gEPqbfvVKemiYfUlMwvy2WubHO CoLqUEZ81VnfsxdDth9a76dSXYtrJkpP+MlSsHJIg+9R4+bMW3fN/fbSSHCmZD/8OD 0QUcGm4WjcS5WRYs8M0URCHkCuZAhEnx5wXD0Glz4lxGX9FnIYo+e7VVuTX8ltJb7E 2teRBj6ORHEXCNejKzyHW7vcqEsVUD5rzQO3iLO5uHXhwb2hHGuK9SHjGjMUGc54mV yAHYmDdE4bTqg== Date: Wed, 6 Mar 2024 17:30:09 -0800 From: Josh Poimboeuf To: Jason Baron Cc: Steven Rostedt , Sam Sun , linux-kernel@vger.kernel.org, syzkaller@googlegroups.com, xrivendell7@gmail.com, ardb@kernel.org, peterz@infradead.org, linux-mm@kvack.org, akpm@linux-foundation.org, Paolo Bonzini Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked Message-ID: <20240307013009.erbnug2mopx5yrnf@treble> References: <20240306105420.6a6bea2c@gandalf.local.home> <20240306193101.s2g33o4viqi2azf3@treble> <854e523c-c467-47f6-b977-933cbaadeb62@akamai.com> <20240306221650.sw3lha7kca2quv63@treble> <8f586bd2-c436-4334-92af-762a284e1101@akamai.com> <20240306234253.zporv6cypoc7yihs@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20240306234253.zporv6cypoc7yihs@treble> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 9176A140005 X-Stat-Signature: 45zop5c49z8tic1phb8k8kxa7wckx1wc X-Rspam-User: X-HE-Tag: 1709775013-317483 X-HE-Meta: U2FsdGVkX1/NSGfJDIvjyv8K0P1ABRG01ggXQFgOSzPsbu26sV0ox+iQrWVynaEQkdNkxQUJ+VAzpJJlAettQWYOBf2bgfr7TkVr8JgrMsnLpFR5ebcBWZcgV78tzYHH4pgh066m7+Ejn876ZyCm5Xplz8qtpL4ocZXjHnARXrUFhSafmJTF4hTpDJTG6Cy3hcmlwoOhE9o+jGJJ/o7JxPYtG2/jP015D3ZGQWrTJ9V3tpv2oWFi+Kuzx++cHD43XJNhU8+oQ/f64qSLVb5A/tSgoZ66CP+Q+XoIJh9JaVa0GvckTkw73VE+qDq8StVMrU0g8+FejY/c2k9uA1/3LMrq2/3XfyKoyabZcFPACEMmxlpD+e5bH9caD+nPX+FCkFF0mvLUBAsThgbVS/C67n/ZT9ExI9X4mphTAyPVzWsco+ABhWl4lMqw9VsjmuyYmNAWUNPFUmxmFdPlKRoD1YXUPZUuj2v8deAix0C9xjsjE5WDefSgqNtczqVHjOzryucWLNbPZqctbvvlb4yGlehtkf0M0NoRivxLNCU5LFonFHLgyZ/Kyf5Oyx4NDgISenK0KeSOnc9otHdMMxcdKm7Q85haFWxqFelLmxmVxrTt/jjB9QgV+ZAVPYajZOBylJYl8Hrx1Wet5TRoYkm5nevUqGwNxTSa0zqrVx2n0djEPlcDBLA4mZU/cN9YcRMbeH1yxD6rzGwvMw6ZDlG3n9bTovSZnENbSQR5XkNh2DpYtdgYuiVnQQe4BdUGAUnuJj3Hr0ZGx33wuEvsCJprpWCBTfIv0vR/eLAmqf+rH4Nt9guMNFlRdXuTQegErWvGbYXNT/+iMIGENmqhLhdv1PggNhXC2fCYCKdrwnJq9nNZqlWWRzBJ/vPG8+eMIjJ9yvE5k/zCaRbIAV7onIeEEI3By/LVo2b2gBVBZGK7drnJK1QENMDohw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Mar 06, 2024 at 03:42:55PM -0800, Josh Poimboeuf wrote: > On Wed, Mar 06, 2024 at 05:40:11PM -0500, Jason Baron wrote: > > On 3/6/24 5:16 PM, Josh Poimboeuf wrote: > > > On Wed, Mar 06, 2024 at 03:12:07PM -0500, Jason Baron wrote: > > > > On 3/6/24 2:31 PM, Josh Poimboeuf wrote: > > > > > On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote: > > > > > > Now I guess the question is, why is something trying to disable something > > > > > > that is not enabled? Is the above scenario OK? Or should the users of > > > > > > static_key also prevent this? > > > > > > > > > > Apparently that's an allowed scenario, as the jump label code seems to > > > > > be actively trying to support it. Basically the last one "wins". > > > > > > > > > > See for example: > > > > > > > > > > 1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()") > > > > > > > > > > Also the purpose of the first atomic_read() is to do a quick test before > > > > > grabbing the jump lock. So instead of grabbing the jump lock earlier, > > > > > it should actually do the first test atomically: > > > > > > > > Makes sense but the enable path can also set key->enabled to -1. > > > > > > Ah, this code is really subtle :-/ > > > > > > > So I think a concurrent disable could then see the -1 in tmp and still > > > > trigger the WARN. > > > > > > I think this shouldn't be possible, for the same reason that > > > static_key_slow_try_dec() warns on -1: key->enabled can only be -1 > > > during the first enable. And disable should never be called before > > > then. > > > > hmm, right but I think in this case the reproducer is writing to a sysfs > > file to enable/disable randomly so i'm not sure if there is anything that > > would enforce that ordering. I guess you could try the reproducer, I haven't > > really looked at it in any detail. > > > > The code in question here is in mm/vmscan.c which actually already takes the > > local 'state_mutex' for some cases. So that could be extended I think easily > > to avoid this warning. > > Hm, right... For now I'll just continue to allow "disable before enable" > (or "double disable") since it may be harmless and I don't want to > introduce any unnecessary constraints, unless we manage to convince > ourselves that it's the right thing to do. So, I think we can simplify this nicely by getting rid of the whole -1 thing altogether: diff --git a/kernel/jump_label.c b/kernel/jump_label.c index d9c822bbffb8..ef7eda7685b2 100644 --- a/kernel/jump_label.c +++ b/kernel/jump_label.c @@ -194,20 +194,15 @@ void static_key_enable_cpuslocked(struct static_key *key) STATIC_KEY_CHECK_USE(key); lockdep_assert_cpus_held(); - if (atomic_read(&key->enabled) > 0) { - WARN_ON_ONCE(atomic_read(&key->enabled) != 1); + if (atomic_read(&key->enabled) == 1) return; - } - jump_label_lock(); - if (atomic_read(&key->enabled) == 0) { - atomic_set(&key->enabled, -1); + + if (atomic_cmpxchg(&key->enabled, 0, 1) == 0) jump_label_update(key); - /* - * See static_key_slow_inc(). - */ - atomic_set_release(&key->enabled, 1); - } + else + WARN_ON_ONCE(atomic_read(&key->enabled) != 1); + jump_label_unlock(); } EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked); @@ -225,14 +220,16 @@ void static_key_disable_cpuslocked(struct static_key *key) STATIC_KEY_CHECK_USE(key); lockdep_assert_cpus_held(); - if (atomic_read(&key->enabled) != 1) { - WARN_ON_ONCE(atomic_read(&key->enabled) != 0); + if (atomic_read(&key->enabled) == 0) return; - } jump_label_lock(); - if (atomic_cmpxchg(&key->enabled, 1, 0)) + + if (atomic_cmpxchg(&key->enabled, 1, 0) == 1) jump_label_update(key); + else + WARN_ON_ONCE(atomic_read(&key->enabled) != 0); + jump_label_unlock(); } EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);