Received: by 2002:a05:6358:11c7:b0:104:8066:f915 with SMTP id i7csp2249484rwl; Thu, 30 Mar 2023 07:57:34 -0700 (PDT) X-Google-Smtp-Source: AKy350ble+SqbJY4yLbGTIkQkqlwgCl97/8zMTsFoci5/MXOP+ik9uOoGR4Mfq1lgnuCBLxmC2bh X-Received: by 2002:a17:906:74c3:b0:944:4d7:d3d9 with SMTP id z3-20020a17090674c300b0094404d7d3d9mr2134277ejl.2.1680188254418; Thu, 30 Mar 2023 07:57:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680188254; cv=none; d=google.com; s=arc-20160816; b=BolUo6iNsnEFeHOXj80spLIU1xzpFOZcqHP1kfxpXbGPUG3Ov3RGCNYX78/LZ4o/A7 vE8Jrw2nsyCzmuNEhw19yjfBZBylyiEEO77q9YXZPzba/C0xYLeRtJ0ZfzlAHF2BRrOU 4O1TzB4VkyYbLC8RkuEhe25CuPZYaK43mRdiXiGr1bgExwEAJXVuFuSG0oYpkm+LnYCz 1W41JNyoi6HKk2gTf1BLXOAhWmzuzsJJmE5IIGaao6fxYQcC3GFAEzzjxp+4gn3HgOBj 85wNfgzg6SBe7MTHxfzLYqt9VBLhgTG70gAVc1nElXARjZ2v1BZVHcJXl2nBADDvyIQN kjCQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=XVGijnsoCupexgnnwJKW3IEH3FOwV+mLfhhEUMBkbsw=; b=PuIbai9JZ6nAXpVdPmfwBGamBkEdFb1HybQ2bNH5Kye8dF5Qb+WMftdrssEHiAfqL1 G68nhKtJnO4arJa4QSC4X6AUv7vpAZa/mOVUOLDThPUH4Tho/c2GIRgCNLHbbPpRV2C9 Qq0uHzypAU9AzAO6yDv9nb/leqETEEeC9VxRB9lglEOY7w/Rg+ichjMNS3sZ/i2lJi7u TdVMEXte9Ptcr1AITTCA0Nj3T/FTxPX6avK+05NWz7YTtThrw3y8Cs/drfHS9+nqfej/ cnyoanaP5wAYD1/3tgyMeT51Lrv1MHI7Cr1Qcv0Qesap6K82NRXiPoJnaePqvyLDEoMi NGvQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=HRZrUIY+; 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=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id mj22-20020a170906af9600b009307d609e83si32138169ejb.905.2023.03.30.07.57.06; Thu, 30 Mar 2023 07:57:34 -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; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=HRZrUIY+; 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=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232565AbjC3O4X (ORCPT + 99 others); Thu, 30 Mar 2023 10:56:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37368 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232605AbjC3O4R (ORCPT ); Thu, 30 Mar 2023 10:56:17 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 64AB0CC1D; Thu, 30 Mar 2023 07:55:51 -0700 (PDT) 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 ams.source.kernel.org (Postfix) with ESMTPS id 70D96B82923; Thu, 30 Mar 2023 14:55:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0F371C433D2; Thu, 30 Mar 2023 14:55:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680188112; bh=FHLtPlDgAi9TMDXORFSOOGlYeZicSFqCxScdEUp354U=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=HRZrUIY+GC+w0z8tNpSlgNGGzxhVW+1DD0SdyLO9O+Z9+nkfgZeN+as9wLD3yZ0Iv 9jcroKlQrUwV0KU9/Qibp4dLwKzfQp5jyUGtB6k5mUHfQ6tCYZqxcnFNkFvBYtqYoE ga3+EB6ogbl6sKDrwHmUTXSb/Zca9d8mP2/UCh/K7B7uQ+VC3YNj0aWNYdCbuSwyHr TSR78FZzQJvlzJkwU+EhdHuLqgKIHXuao4fcto55jmYXiD4WZIOcXu8gnxnEnReinO IUhJqNgN0E2UJgAg5LKNQaTF29UXFh81pPMT+pnQCV48TFxEei33R+2zfX0D8PVLHD Rs1rwrDVrkmYw== Received: by paulmck-ThinkPad-P72.home (Postfix, from userid 1000) id 865E01540476; Thu, 30 Mar 2023 07:55:11 -0700 (PDT) Date: Thu, 30 Mar 2023 07:55:11 -0700 From: "Paul E. McKenney" To: "Zhang, Qiang1" Cc: "rcu@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "kernel-team@meta.com" , "rostedt@goodmis.org" , "hch@lst.de" , "qiang.zhang1211@gmail.com" Subject: Re: [PATCH RFC rcu 02/19] srcu: Use static init for statically allocated in-module srcu_struct Message-ID: Reply-To: paulmck@kernel.org References: <3db82572-f156-4a5d-b711-841aa28bd996@paulmck-laptop> <20230324001938.3443499-2-paulmck@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-5.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,SPF_HELO_NONE, SPF_PASS autolearn=unavailable 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 On Thu, Mar 30, 2023 at 04:11:05AM +0000, Zhang, Qiang1 wrote: > > > >Further shrinking the srcu_struct structure is eased by requiring > >that in-module srcu_struct structures rely more heavily on static > >initialization. In particular, this preserves the property that > >a module-load-time srcu_struct initialization can fail only due > >to memory-allocation failure of the per-CPU srcu_data structures. > >It might also slightly improve robustness by keeping the number of memory > >allocations that must succeed down percpu_alloc() call. > > > >This is in preparation for splitting an srcu_usage structure out > >of the srcu_struct structure. > > > >[ paulmck: Fold in qiang1.zhang@intel.com feedback. ] > > > >Signed-off-by: Paul E. McKenney > >Cc: Christoph Hellwig > >--- > > include/linux/srcutree.h | 19 ++++++++++++++----- > > kernel/rcu/srcutree.c | 19 +++++++++++++------ > > 2 files changed, 27 insertions(+), 11 deletions(-) > > > >diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h > >index ac8af12f93b3..428480152375 100644 > >--- a/include/linux/srcutree.h > >+++ b/include/linux/srcutree.h > >@@ -121,15 +121,24 @@ struct srcu_struct { > > #define SRCU_STATE_SCAN1 1 > > #define SRCU_STATE_SCAN2 2 > > > >-#define __SRCU_STRUCT_INIT(name, pcpu_name) \ > >-{ \ > >- .sda = &pcpu_name, \ > >+#define __SRCU_STRUCT_INIT_COMMON(name) \ > > .lock = __SPIN_LOCK_UNLOCKED(name.lock), \ > > .srcu_gp_seq_needed = -1UL, \ > > .work = __DELAYED_WORK_INITIALIZER(name.work, NULL, 0), \ > >- __SRCU_DEP_MAP_INIT(name) \ > >+ __SRCU_DEP_MAP_INIT(name) > >+ > >+#define __SRCU_STRUCT_INIT_MODULE(name) \ > >+{ \ > >+ __SRCU_STRUCT_INIT_COMMON(name) \ > > } > > > >+#define __SRCU_STRUCT_INIT(name, pcpu_name) \ > >+{ \ > >+ .sda = &pcpu_name, \ > >+ __SRCU_STRUCT_INIT_COMMON(name) \ > >+} > >+ > >+ > > /* > > * Define and initialize a srcu struct at build time. > > * Do -not- call init_srcu_struct() nor cleanup_srcu_struct() on it. > >@@ -151,7 +160,7 @@ struct srcu_struct { > > */ > > #ifdef MODULE > > # define __DEFINE_SRCU(name, is_static) \ > >- is_static struct srcu_struct name; \ > >+ is_static struct srcu_struct name = __SRCU_STRUCT_INIT_MODULE(name); \ > > extern struct srcu_struct * const __srcu_struct_##name; \ > > struct srcu_struct * const __srcu_struct_##name \ > > __section("___srcu_struct_ptrs") = &name > >diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > >index cd46fe063e50..7a6d9452a5d0 100644 > >--- a/kernel/rcu/srcutree.c > >+++ b/kernel/rcu/srcutree.c > >@@ -1895,13 +1895,14 @@ void __init srcu_init(void) > > static int srcu_module_coming(struct module *mod) > > { > > int i; > >+ struct srcu_struct *ssp; > > struct srcu_struct **sspp = mod->srcu_struct_ptrs; > >- int ret; > > > > for (i = 0; i < mod->num_srcu_structs; i++) { > >- ret = init_srcu_struct(*(sspp++)); > >- if (WARN_ON_ONCE(ret)) > >- return ret; > >+ ssp = *(sspp++); > >+ ssp->sda = alloc_percpu(struct srcu_data); > >+ if (WARN_ON_ONCE(!ssp->sda)) > >+ return -ENOMEM; > > } > > return 0; > > } > >@@ -1910,10 +1911,16 @@ static int srcu_module_coming(struct module *mod) > > static void srcu_module_going(struct module *mod) > > { > > int i; > >+ struct srcu_struct *ssp; > > struct srcu_struct **sspp = mod->srcu_struct_ptrs; > > > >- for (i = 0; i < mod->num_srcu_structs; i++) > >- cleanup_srcu_struct(*(sspp++)); > >+ for (i = 0; i < mod->num_srcu_structs; i++) { > >+ ssp = *(sspp++); > >+ if (!rcu_seq_state(smp_load_acquire(&ssp->srcu_sup->srcu_gp_seq_needed)) && > >+ !WARN_ON_ONCE(!ssp->srcu_sup->sda_is_static)) > >+ cleanup_srcu_struct(ssp); > >+ free_percpu(ssp->sda); > > > Hi Paul > > About 037b80b8865fb ("srcu: Check for readers at module-exit time ") > > --- a/kernel/rcu/srcutree.c > +++ b/kernel/rcu/srcutree.c > @@ -1911,7 +1911,8 @@ static void srcu_module_going(struct module *mod) > if (!rcu_seq_state(smp_load_acquire(&ssp->srcu_sup->srcu_gp_seq_needed)) && > !WARN_ON_ONCE(!ssp->srcu_sup->sda_is_static)) > cleanup_srcu_struct(ssp); > - free_percpu(ssp->sda); > + else if (!WARN_ON(srcu_readers_active(ssp))) > + free_percpu(ssp->sda); > > Should the else statement be removed? like this: > > if (!WARN_ON(srcu_readers_active(ssp))) > free_percpu(ssp->sda); Mightn't that cause us to double-free ssp->sda? Once in free_percpu(), and before that in cleanup_srcu_struct()? Thanx, Paul > Thanks > Zqiang > > > >+ } > > } > > > > /* Handle one module, either coming or going. */ > >-- > >2.40.0.rc2 > >