From patchwork Tue Oct 18 08:22:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin He X-Patchwork-Id: 4015 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp1843018wrs; Tue, 18 Oct 2022 01:40:38 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5rtn7UdYSWTEOGDItFUkVs2i8K8uwK5DD0KlbahSXId0P3SMSosan6ERHhCTu3Xi4MHA19 X-Received: by 2002:a17:902:da83:b0:185:5837:f43a with SMTP id j3-20020a170902da8300b001855837f43amr2224178plx.26.1666082438135; Tue, 18 Oct 2022 01:40:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666082438; cv=none; d=google.com; s=arc-20160816; b=w/a7uUk8hCm7s1XvaWdcjemezgYCO08DBZxmPygM7MtmrFMg5lf0As0J0nMc8oPM81 c3gmws3ZBNeeNsYkeg4iQiWWzUoXDb7O0rafysPM38D5Oc7fycjwo5vRNe6R6m6PtWv9 SvKZwXRSg9DlUuYFIShuG5laKowJviE1rF8xuUn1LTLFDeBBYLfNUt6Yj4ByoSQyO+iA R8jjclu+FDfwPkYVITEv5ICXKVgY905PuuBQ+VBskSME35hY8uGZJNBZZMWeZOfulmkE VzloUOJbOoWZD2d2PJIrgQ5NkVi73C2XCwVd8mJN18dPLuubonBtR1kjhLp4f/S9cKPd ttDg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=XgBr/Exf9jetnEHto4I4Vp5TTeJmxoN5RaqemhG/pak=; b=zUvlvWy5f0tA9UZHD6W70Y+wRSTfL4putPrhrJ2wUCbIrep/9PcxjoqXmTWv3DL2g7 99vWlt0s33aPycdBmxqj/0/fozuGxYf+0qqV8kbP8nZdO+Jq2zq0b6MocUW8WQJNWwBm jJUmc4G+sjSfJ+ZO7d7CGCBWQZPRQqciYtcn7f0YfiQru3KtrISxlswXbKUnotJnDi1v xOjNwVys5vJOFMe7C2Malu/InOjQRljqrdLCYv8NVCpTdUNkjD9usY1Xnf+5dSzcTPdH n6OADXx1p9f0yLXyMd2DTadMlOR1AMAXuriazkdDPxQ5Z9Jc8wsSU9/7yg8jwEr2lEJ+ M6fA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id z12-20020a170903018c00b00177f35ce11fsi4146698plg.22.2022.10.18.01.40.19; Tue, 18 Oct 2022 01:40:38 -0700 (PDT) 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; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230287AbiJRI3A (ORCPT + 99 others); Tue, 18 Oct 2022 04:29:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53578 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231176AbiJRI2c (ORCPT ); Tue, 18 Oct 2022 04:28:32 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2475E9AFC2; Tue, 18 Oct 2022 01:27:59 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 63354175D; Tue, 18 Oct 2022 01:28:01 -0700 (PDT) Received: from entos-ampere-02.shanghai.arm.com (entos-ampere-02.shanghai.arm.com [10.169.212.212]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 773723F7D8; Tue, 18 Oct 2022 01:27:48 -0700 (PDT) From: Jia He To: Ard Biesheuvel , Len Brown , Tony Luck , Borislav Petkov , Mauro Carvalho Chehab , Robert Richter , Robert Moore , Qiuxu Zhuo , Yazen Ghannam , Jan Luebbe , Khuong Dinh , Kani Toshi Cc: James Morse , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org, devel@acpica.org, "Rafael J . Wysocki" , Shuai Xue , Jarkko Sakkinen , linux-efi@vger.kernel.org, nd@arm.com, Peter Zijlstra , Jia He Subject: [PATCH v10 6/7] apei/ghes: Use xchg_release() for updating new cache slot instead of cmpxchg() Date: Tue, 18 Oct 2022 08:22:13 +0000 Message-Id: <20221018082214.569504-7-justin.he@arm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221018082214.569504-1-justin.he@arm.com> References: <20221018082214.569504-1-justin.he@arm.com> MIME-Version: 1.0 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW, SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747014058398716257?= X-GMAIL-MSGID: =?utf-8?q?1747014058398716257?= From: Ard Biesheuvel From: Ard Biesheuvel ghes_estatus_cache_add() selects a slot, and either succeeds in replacing its contents with a pointer to a new cached item, or it just gives up and frees the new item again, without attempting to select another slot even if one might be available. Since only inserting new items is needed, the race can only cause a failure if the selected slot was updated with another new item concurrently, which means that it is arbitrary which of those two items gets dropped. This means the cmpxchg() and the special case are not necessary, and hence just drop the existing item unconditionally. Note that this does not result in loss of error events, it simply means we might cause a false cache miss, and report the same event one additional time in quick succession even if the cache should have prevented that. Move the xchg_release() and call_rcu out of rcu_read_lock/unlock section since there is no actually dereferencing the pointer at all. Co-developed-by: Jia He Signed-off-by: Jia He Signed-off-by: Ard Biesheuvel --- drivers/acpi/apei/ghes.c | 49 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 27c72b175e4b..3ade61e6b7a5 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -150,7 +150,7 @@ struct ghes_vendor_record_entry { static struct gen_pool *ghes_estatus_pool; static unsigned long ghes_estatus_pool_size_request; -static struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; +static struct ghes_estatus_cache __rcu *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE]; static atomic_t ghes_estatus_cache_alloced; static int ghes_panic_timeout __read_mostly = 30; @@ -785,31 +785,26 @@ static struct ghes_estatus_cache *ghes_estatus_cache_alloc( return cache; } -static void ghes_estatus_cache_free(struct ghes_estatus_cache *cache) +static void ghes_estatus_cache_rcu_free(struct rcu_head *head) { + struct ghes_estatus_cache *cache; u32 len; + cache = container_of(head, struct ghes_estatus_cache, rcu); len = cper_estatus_len(GHES_ESTATUS_FROM_CACHE(cache)); len = GHES_ESTATUS_CACHE_LEN(len); gen_pool_free(ghes_estatus_pool, (unsigned long)cache, len); atomic_dec(&ghes_estatus_cache_alloced); } -static void ghes_estatus_cache_rcu_free(struct rcu_head *head) -{ - struct ghes_estatus_cache *cache; - - cache = container_of(head, struct ghes_estatus_cache, rcu); - ghes_estatus_cache_free(cache); -} - static void ghes_estatus_cache_add( struct acpi_hest_generic *generic, struct acpi_hest_generic_status *estatus) { int i, slot = -1, count; unsigned long long now, duration, period, max_period = 0; - struct ghes_estatus_cache *cache, *slot_cache = NULL, *new_cache; + struct ghes_estatus_cache *cache, *new_cache; + struct ghes_estatus_cache __rcu *victim; new_cache = ghes_estatus_cache_alloc(generic, estatus); if (new_cache == NULL) @@ -820,13 +815,11 @@ static void ghes_estatus_cache_add( cache = rcu_dereference(ghes_estatus_caches[i]); if (cache == NULL) { slot = i; - slot_cache = NULL; break; } duration = now - cache->time_in; if (duration >= GHES_ESTATUS_IN_CACHE_MAX_NSEC) { slot = i; - slot_cache = cache; break; } count = atomic_read(&cache->count); @@ -835,18 +828,30 @@ static void ghes_estatus_cache_add( if (period > max_period) { max_period = period; slot = i; - slot_cache = cache; } } - /* new_cache must be put into array after its contents are written */ - smp_wmb(); - if (slot != -1 && cmpxchg(ghes_estatus_caches + slot, - slot_cache, new_cache) == slot_cache) { - if (slot_cache) - call_rcu(&slot_cache->rcu, ghes_estatus_cache_rcu_free); - } else - ghes_estatus_cache_free(new_cache); rcu_read_unlock(); + + if (slot != -1) { + /* + * Use release semantics to ensure that ghes_estatus_cached() + * running on another CPU will see the updated cache fields if + * it can see the new value of the pointer. + */ + victim = xchg_release(&ghes_estatus_caches[slot], + RCU_INITIALIZER(new_cache)); + + /* + * At this point, victim may point to a cached item different + * from the one based on which we selected the slot. Instead of + * going to the loop again to pick another slot, let's just + * drop the other item anyway: this may cause a false cache + * miss later on, but that won't cause any problems. + */ + if (victim) + call_rcu(&unrcu_pointer(victim)->rcu, + ghes_estatus_cache_rcu_free); + } } static void __ghes_panic(struct ghes *ghes,