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 CF5FBC6FA8F for ; Wed, 30 Aug 2023 08:34:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 64E898E004A; Wed, 30 Aug 2023 04:34:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5FE248E0009; Wed, 30 Aug 2023 04:34:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4C5FD8E004A; Wed, 30 Aug 2023 04:34:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 3ADDC8E0009 for ; Wed, 30 Aug 2023 04:34:28 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 06EDB8029B for ; Wed, 30 Aug 2023 08:34:28 +0000 (UTC) X-FDA: 81180109416.24.3CB91E8 Received: from mail-lj1-f179.google.com (mail-lj1-f179.google.com [209.85.208.179]) by imf11.hostedemail.com (Postfix) with ESMTP id 1F92C40007 for ; Wed, 30 Aug 2023 08:34:25 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="GYnbP1/1"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of elver@google.com designates 209.85.208.179 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693384466; 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=Pt0PhngddEXeQF55w8XLKCuMlgv9f/y2sGmaKeHMR1g=; b=F0BpMeYEWJBjNyUTA7KwxFXi5d2fl0N1R/3xKIDpnVEtDQegXmk9kcSI5qP058xZIEs+9b iaESkXzUwv6ScVRPjUX/s11lfdkluoSnPdrD2DBTxcMggrx2L8XSPFhpTU/beJhdxOaVXd +pbOEw/pxwsKAyQssFx5RsRB2HGrcIY= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="GYnbP1/1"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of elver@google.com designates 209.85.208.179 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693384466; a=rsa-sha256; cv=none; b=sJD8mRFsy239BhVTctT7QK58zS3XDbzVcejzQdcIsMeYFJwXzeYuvkC+3H+z8TOGsX5th4 aXCfzDe4uQ2Nf4hnDPtoVGVN/5FFfLowSrsXNB0ZUuTOiEgGIRC8DgASDIncVPbiisDI+S kEFh3LT5A0H4L/QZaFN+VlO3FVCvGaw= Received: by mail-lj1-f179.google.com with SMTP id 38308e7fff4ca-2b9f0b7af65so78143581fa.1 for ; Wed, 30 Aug 2023 01:34:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1693384464; x=1693989264; darn=kvack.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=Pt0PhngddEXeQF55w8XLKCuMlgv9f/y2sGmaKeHMR1g=; b=GYnbP1/1g0bkItExhjsrFoGhV4kl7pxK0g674ZE3S3xfHsxiyO3P/P9Nu9sL2K0v87 G8SK7syLBoF7pSHmpjcHY2lbn9q/97Q+qBA9mY/uLT0Eqmn9oewIIqjYwfcJfeEoMSvZ gm64jci+Jb1vnT7/CNNnT7o6ZuinaUKlLTpwE+wtPQL8EMNNCpcd9M3tqxxukV8FMH6S /emmaeCPKQMuzJ/l9HLydlTTLkCFg1aPe1uF+LVVchS4ZqPpctXRE1RCcYPx3SXf24W2 Y64YghLezvT5lhEYR+UGYVVfetio393TaI8KFRwfZHRYSqUWWfPm5NV+XcSBg9yPAiBq zgDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693384464; x=1693989264; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Pt0PhngddEXeQF55w8XLKCuMlgv9f/y2sGmaKeHMR1g=; b=B6lr+J7JBwauw42Mf0LfJnNvZY3g1HO2ihXeu/WHnDK2rcCnA8gsfT9kBw/ZL8PTvx kjtxKIgTg9iMXk0bAa0ggkdO73kYCkrIRdiVyeYqnjzPcj57lzhhZ9HWXj6yPRcbFyV0 JZs2d4k1xVz0b6+KWyATtYCzNsHYasGYvJg6OTfUW6EaVAHsspcG9tq1UCD2qCMYTh1h xbwVcDlCwNkObBV2+IQSWoAzmcDz06WuzE8idrLrPIj2Oan1c7ULvo6HwsUrxQDhjDgE hkaBYq2VIcpFGCFZ55Ud5nTh7RltN37qZGX2o+hFauDTBEfvv9V2cD8y4E5GprTYNmby wuAg== X-Gm-Message-State: AOJu0YyqmCqP/JrJL3nz0Q3Y7eipS+JfVDY1ffLsExbaBTrhH31zPQ3d /fGOhzW3e19o1Su0L0wYP9lDvQ== X-Google-Smtp-Source: AGHT+IF5mj6CqUUVIWOYpJZkiR0PfqRBvcfVsy6nzRFSq/akc1Lk4stw6BD+EnUfeCleyyR0MIOHyg== X-Received: by 2002:a2e:b60e:0:b0:2bc:d3a8:974a with SMTP id r14-20020a2eb60e000000b002bcd3a8974amr1259289ljn.24.1693384463909; Wed, 30 Aug 2023 01:34:23 -0700 (PDT) Received: from elver.google.com ([2a00:79e0:9c:201:3380:af04:1905:46a]) by smtp.gmail.com with ESMTPSA id o18-20020a05600c379200b003fee65091fdsm1505056wmr.40.2023.08.30.01.34.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Aug 2023 01:34:23 -0700 (PDT) Date: Wed, 30 Aug 2023 10:34:18 +0200 From: Marco Elver To: andrey.konovalov@linux.dev Cc: Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vlastimil Babka , kasan-dev@googlegroups.com, Evgenii Stepanov , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrey Konovalov Subject: Re: [PATCH 06/15] stackdepot: fix and clean-up atomic annotations Message-ID: References: <8ad8f778b43dab49e4e6214b8d90bed31b75436f.1693328501.git.andreyknvl@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8ad8f778b43dab49e4e6214b8d90bed31b75436f.1693328501.git.andreyknvl@google.com> User-Agent: Mutt/2.2.9 (2022-11-12) X-Rspamd-Queue-Id: 1F92C40007 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: wfdpx6htyj8634bmdriy5rf51b5sxddg X-HE-Tag: 1693384465-499678 X-HE-Meta: U2FsdGVkX18ZTptWEN0FlGJxeKn5FRz5vh3VavIX76Ll8Hi8Z0zEF0IYiFV1ol9PlpIT75tgsC6COTeh5oFy7ph0RB2mtUbh8oHt8tOVuzyeUHhbiNXCa8Hg8jmzh2zTTmT8Z39gdJlRLWl8jCxoiPC+0R7KbaBZqNhzGFUyBFMDlwkaM3Gz+vqirdxy43laZ1viLJmwaOK05O2HCX8Pwqn0pIiFAzBquNX8dOV4skvlBjTG+C3G6H+ZK1yoMLXPLGTeV+zdWLPrueJ9g4lEOHoEHoTuC0rd2NN0gD/6YLRFCi0l/8Zq6t0h3qhMBe6aOFvlQXj5cyZRxKRBkb62P6Y6RcuMen5rCHSS/cnxbi3K1m+MyL9RYqMEg7xWfg7IDRQUX8TVTZQiL22P1JmPhBweLLVTWDQsVNhrii1wVpgegGzzdsmo4lXWg3FFvnL7BpWY3BfNYmTz4GTghJv96rJrrXJ72OGB88C8z0d2vZ9gaZGK569G5FYJENWdQFT1GexKE6a33gdJM7XuHXl3Rp7AjNA8amE7Maje40JB14rjK+6q0ZKHpna+ayRf3Jwt8gvEMqvBFYHUUmrWyFB25i092iwceOsvGdTlW6NxY0vnqVPPQfw9gzEIDDpdqSEsAWVP2p+homlD+xlnLtF12eGwA5/MGqaTn/MOksvjMEmfnWTMLNNlxTGxyaWmuTnbaaOMLDInC5qeQeTZ+ZZ1Z3d4/uTPljufbL64sPVyvw27ptAZg6jWXBW6uq0YcQ7ivx6yn8Jbl6KIhyr2Yp8Q6VRLsC2xznqKHUqiQhrqeO6UkpboyAzzQZrE7Z7vELXHPhFUWgfoCQaRIQq3qYaNXj1bDPMeymlTDmQ1TWdxIkk7fWzOrxcvXUWM2mAK30nhdcTm7/Py8MPVkRhOOkqwGv8VnBcKZU8kFpKOvSAoYtqPBQyugoTewxmwl/u4SrzyFy2iChxThrofWL1qd4G v4GQBEyT B1U3RW4PoH1VpBMHxnv2HqbuIO4u8um1RGTmZL2lP8MdRJjZr5ZTEhYFiEUiehlo2xjS3mtg07kimHMNv1lmSnmokW0LEbD8Y+t8vXOd8y7LubDWjamJHi0pc8/jCZFuh5OW+AxtEm12Pce4wo3sz3n0wmowbO0hb1zcvs8KS3zAydL3de4u2r2SFA+SDlT0i4DuCID9UcaJtcB+rpTBGx0/24HdCiivbnxzxroc/adw7TZ9eLxA8lSxENUMb5OZfc1xAVw1rPVjIcbzzGt2Dp+1ZsOnVvM8PbhdKOQx+k62oz/GiZswnkxQx0ryKU2uhyvzRV9joFiqD8nU+kvtQxfIw2wHGxYJL4/naZBsnNopYYm6SBGuQN2JdxUW2zaPW6GLDdal5ahrVwMq1Nd8OYQPDSA== 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: On Tue, Aug 29, 2023 at 07:11PM +0200, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov > > Simplify comments accompanying the use of atomic accesses in the > stack depot code. > > Also turn smp_load_acquire from next_pool_required in depot_init_pool > into READ_ONCE, as both depot_init_pool and the all smp_store_release's > to this variable are executed under the stack depot lock. > > Signed-off-by: Andrey Konovalov > > --- > > This patch is not strictly required, as the atomic accesses are fully > removed in one of the latter patches. However, I decided to keep the > patch just in case we end up needing these atomics in the following > iterations of this series. > --- > lib/stackdepot.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 93191ee70fc3..9ae71e1ef1a7 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -226,10 +226,10 @@ static void depot_init_pool(void **prealloc) > /* > * If the next pool is already initialized or the maximum number of > * pools is reached, do not use the preallocated memory. > - * smp_load_acquire() here pairs with smp_store_release() below and > - * in depot_alloc_stack(). > + * READ_ONCE is only used to mark the variable as atomic, > + * there are no concurrent writes. This doesn't make sense. If there are no concurrent writes, we should drop the marking, so that if there are concurrent writes, tools like KCSAN can tell us about it if our assumption was wrong. > */ > - if (!smp_load_acquire(&next_pool_required)) > + if (!READ_ONCE(next_pool_required)) > return; > > /* Check if the current pool is not yet allocated. */ > @@ -250,8 +250,8 @@ static void depot_init_pool(void **prealloc) > * At this point, either the next pool is initialized or the > * maximum number of pools is reached. In either case, take > * note that initializing another pool is not required. > - * This smp_store_release pairs with smp_load_acquire() above > - * and in stack_depot_save(). > + * smp_store_release pairs with smp_load_acquire in > + * stack_depot_save. > */ > smp_store_release(&next_pool_required, 0); > } > @@ -275,15 +275,15 @@ depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc) > /* > * Move on to the next pool. > * WRITE_ONCE pairs with potential concurrent read in > - * stack_depot_fetch(). > + * stack_depot_fetch. > */ > WRITE_ONCE(pool_index, pool_index + 1); > pool_offset = 0; > /* > * If the maximum number of pools is not reached, take note > * that the next pool needs to initialized. > - * smp_store_release() here pairs with smp_load_acquire() in > - * stack_depot_save() and depot_init_pool(). > + * smp_store_release pairs with smp_load_acquire in > + * stack_depot_save. > */ > if (pool_index + 1 < DEPOT_MAX_POOLS) > smp_store_release(&next_pool_required, 1); > @@ -414,8 +414,7 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > > /* > * Fast path: look the stack trace up without locking. > - * The smp_load_acquire() here pairs with smp_store_release() to > - * |bucket| below. > + * smp_load_acquire pairs with smp_store_release to |bucket| below. > */ > found = find_stack(smp_load_acquire(bucket), entries, nr_entries, hash); > if (found) > @@ -425,8 +424,8 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > * Check if another stack pool needs to be initialized. If so, allocate > * the memory now - we won't be able to do that under the lock. > * > - * The smp_load_acquire() here pairs with smp_store_release() to > - * |next_pool_inited| in depot_alloc_stack() and depot_init_pool(). > + * smp_load_acquire pairs with smp_store_release > + * in depot_alloc_stack and depot_init_pool. Reflow comment to match 80 cols used by comments elsewhere. > */ > if (unlikely(can_alloc && smp_load_acquire(&next_pool_required))) { > /* > @@ -452,8 +451,8 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries, > if (new) { > new->next = *bucket; > /* > - * This smp_store_release() pairs with > - * smp_load_acquire() from |bucket| above. > + * smp_store_release pairs with smp_load_acquire > + * from |bucket| above. > */ > smp_store_release(bucket, new); > found = new; > -- > 2.25.1 >