Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 12 Apr 2022 10:13:15 -0700
From:   Martin KaFai Lau <kafai@fb.com>
To:     KP Singh <kpsingh@kernel.org>
Cc:     Neeraj Upadhyay <quic_neeraju@quicinc.com>, paulmck@kernel.org,
        Eric Dumazet <edumazet@google.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Stanislav Fomichev <sdf@google.com>, andrii@kernel.org,
        ast@kernel.org, bpf <bpf@vger.kernel.org>
Subject: Re: [BUG] rcu-tasks : should take care of sparse cpu masks
Message-ID: <20220412171315.i6e6n3b6v7xclje6@kafai-mbp.dhcp.thefacebook.com>
References: <20220401130114.GC4285@paulmck-ThinkPad-P17-Gen-1>
 <CANn89iLicuKS2wDjY1D5qNT4c-ob=D2n1NnRnm5fGg4LFuW1Kg@mail.gmail.com>
 <20220401152037.GD4285@paulmck-ThinkPad-P17-Gen-1>
 <20220401152814.GA2841044@paulmck-ThinkPad-P17-Gen-1>
 <20220401154837.GA2842076@paulmck-ThinkPad-P17-Gen-1>
 <7a90a9b5-df13-6824-32d1-931f19c96cba@quicinc.com>
 <CACYkzJ4FzbFu5NfdRMParp3Ome=ygVAqQPs2v6UGzRDt2LC6iw@mail.gmail.com>
 <20220405203818.qsi7j74jpsex7oky@kafai-mbp.dhcp.thefacebook.com>
 <CACYkzJ5PkwidPAomc-+js=OTFdzwf38hMO01Q_rbsPM-HZTTkg@mail.gmail.com>
 <CACYkzJ77k7=UwAve-DBkhO7cmBh7W9e6JgFrc3HoHQ3AU1Xfsg@mail.gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACYkzJ77k7=UwAve-DBkhO7cmBh7W9e6JgFrc3HoHQ3AU1Xfsg@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?rXQee8Mx993YAp2Amdd9FWM6JzSY1XV0uoVtr0fW/6Rr4SvMKF1cZnLjRdoV?=
 =?us-ascii?Q?sw6enhG819u0RQNOgO4f7DU5F80CcZxMZHF3lCuL1sLaW1GRaRDp6+ALSyXu?=
 =?us-ascii?Q?op2D4WEDSgrjpCW53etU8Wq0Lf27Vs860l9nsSzEHqj5nr49vP3RF+ae6Iuw?=
 =?us-ascii?Q?0Kj57NghlWiuCMYa/bRachNjf014f19K3h69vUePlHT0iSFYkYB2Le5dYHXu?=
 =?us-ascii?Q?0TxjJya4JUwa9ErpLBjxO7fH+mVPfLoORC/e0l6JiUbarGyAbo5MJD1jbRTs?=
 =?us-ascii?Q?37rO+FalzcMwLA1334OdSCIOVV/BWlKb56bi7DIVgb8hUGS5uJ82A2sN0YZS?=
 =?us-ascii?Q?GK64ATGerUt9HRMclIXfHaiYNuTnEJvMymYDKD5c87A/xdoOiSnOVFiHJw32?=
 =?us-ascii?Q?qD66g6KDNIDR6yRC0rA3ZczMaQU5EzNPpVPxjSvJNNYBk5Nf1HGUlfgApI+Y?=
 =?us-ascii?Q?PilS0QeQGKuyKmlo4mA+fUpx2WIARp7UVcTvJY4aGgG/6PobDtfYNhGhs/iq?=
 =?us-ascii?Q?PsD/IgNVQ3cc8x7fd+8ntbAmxZgAxWM+2dymuk6+cc4PyWHgaLBUGchM4Z5G?=
 =?us-ascii?Q?ZZh4u1UB8UxpHswbvt2xjekrlVvUPaiziRMvSsA8OSF8lBFbLS+4tFevpUrd?=
 =?us-ascii?Q?iKXWUr8YxnTW0XU6t1O0bAGx9WXNiPj6QEx27GdJShYp+tkI0w7Z0BdJA5Sl?=
 =?us-ascii?Q?e3JRuK1WVo4pskOMLCXlv5KL+xvhyQ1gEXZOD2oZf1ErcTedJWCMAAt6kbPC?=
 =?us-ascii?Q?ZSp2aYo8gXctfVhiTHnSMjBDeR6QIQLgGsEbiNH5oBuR4sx7u33mi3zPDCl6?=
 =?us-ascii?Q?J9/3bltQ2fUkydTBsmoDIx6/+gGxEx+k+Up6O9QwX0c1prFxDORtek8fBDEC?=
 =?us-ascii?Q?XyNDzmi3QYqCtWjnIwYi1pUShe7d6BlgLNWCM1KloQZPGYtX+qgwdTA/gLXK?=
 =?us-ascii?Q?5ezM2jHO/2OnJGiXU7wOp7fWam2xRxblwDlK397eOOQSxtIVyIOmADplXXmX?=
 =?us-ascii?Q?nFol7O6ZBfNqShxzFt5GpxBBErx6yk3uay2rH5DOp3g67SULEetd5BJQxdqb?=
 =?us-ascii?Q?sPpStQqYIypK06AWtrtyKx5e/ej9gIFVQSKVfrVWfylF4CEgKYP2lnVu/7YC?=
 =?us-ascii?Q?TQXjoDfb8c3cnhfWGFjJuwCMI2kWYv+Qczmv9NPZSD+QupF9oD6tWwKy+18x?=
 =?us-ascii?Q?tyMa0Gsswucdcx3d1R9ivZhfnHsp2kA81giOuU12XtcaBiXngCoyjmwlG7S2?=
 =?us-ascii?Q?pB9Vtffni2vQxsjBeN/qY6CLTicAaBnVZulGprfXcX4jOY+pwtT8ZRbByvxg?=
 =?us-ascii?Q?6s03bwaZpa6XT+Vsjv4LMQIGssDKJNANkuLw4bVaiLikxLtOeryIyJckDrWL?=
 =?us-ascii?Q?ttJOE/uHfGGc/w0jRJmGLxLFb9vCIDSXN/GAP23Xf6JYEUnG8iMIIBhNnORp?=
 =?us-ascii?Q?D7tHv13IIcUMdWuSymmN7nMDEnnnqH/BrIi4tUQOzDVgK12GnbvCP2WPf0a6?=
 =?us-ascii?Q?rTLci+tXt2u1KlBh6vW3tsj5txkRUi4U9s2OJbsInExjulLcUmrQsE/L3UgI?=
 =?us-ascii?Q?kJdAwtR1HcQXx+AIvtXhvVMIab9DCXJKmJMxwS+C5nNhtDu9lPHRj0y8sqsT?=
 =?us-ascii?Q?QY7Rn7u3Oq5qv5HU7t5ezZ2O3ICuy8qJsu5yyZLC/XeYHdCIGS21R9LMofNo?=
 =?us-ascii?Q?UMTbOMllqBVgBshsbdhum7OAFKE+ORaVrclxW+VBzODVcpXwSiAUi8Bo5aRa?=
 =?us-ascii?Q?/xSwkMtjxSlKCIKiVnUf6EwU+ymypOs=3D?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 770f5a91-0e27-49fb-04a9-08da1ca7bc5e
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5016.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2022 17:13:17.4240
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JMeRxsk7hrCno3viHlwSEAkppzRalH8NJBc3jNPvxsk9OPAJfw0U6v0FfZGDw5nC
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR15MB4792
X-Proofpoint-ORIG-GUID: Jhy6_aaxOZ6MfTaFgRzo92aHYK9YMs7i
X-Proofpoint-GUID: Jhy6_aaxOZ6MfTaFgRzo92aHYK9YMs7i
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.858,Hydra:6.0.486,FMLib:17.11.64.514
 definitions=2022-04-12_06,2022-04-12_02,2022-02-23_01
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no 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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Apr 12, 2022 at 08:28:07AM +0200, KP Singh wrote:
> On Wed, Apr 6, 2022 at 5:44 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > On Tue, Apr 5, 2022 at 10:38 PM Martin KaFai Lau <kafai@fb.com> wrote:
> > >
> > > On Tue, Apr 05, 2022 at 02:04:34AM +0200, KP Singh wrote:
> > > > > >>> Either way, how frequently is call_rcu_tasks_trace() being invoked in
> > > > > >>> your setup?  If it is being invoked frequently, increasing delays would
> > > > > >>> allow multiple call_rcu_tasks_trace() instances to be served by a single
> > > > > >>> tasklist scan.
> > > > > >>>
> > > > > >>>> Given that, I do not think bpf_sk_storage_free() can/should use
> > > > > >>>> call_rcu_tasks_trace(),
> > > > > >>>> we probably will have to fix this soon (or revert from our kernels)
> > > > > >>>
> > > > > >>> Well, you are in luck!!!  This commit added call_rcu_tasks_trace() to
> > > > > >>> bpf_selem_unlink_storage_nolock(), which is invoked in a loop by
> > > > > >>> bpf_sk_storage_free():
> > > > > >>>
> > > > > >>> 0fe4b381a59e ("bpf: Allow bpf_local_storage to be used by sleepable programs")
> > > > > >>>
> > > > > >>> This commit was authored by KP Singh, who I am adding on CC.  Or I would
> > > > > >>> have, except that you beat me to it.  Good show!!!  ;-)
> > > >
> > > > Hello :)
> > > >
> > > > Martin, if this ends up being an issue we might have to go with the
> > > > initial proposed approach
> > > > of marking local storage maps explicitly as sleepable so that not all
> > > > maps are forced to be
> > > > synchronized via trace RCU.
> > > >
> > > > We can make the verifier reject loading programs that try to use
> > > > non-sleepable local storage
> > > > maps in sleepable programs.
> > > >
> > > > Do you think this is a feasible approach we can take or do you have
> > > > other suggestions?
> > > bpf_sk_storage_free() does not need to use call_rcu_tasks_trace().
> > > The same should go for the bpf_{task,inode}_storage_free().
> > > The sk at this point is being destroyed.  No bpf prog (sleepable or not)
> > > can have a hold on this sk.  The only storage reader left is from
> > > bpf_local_storage_map_free() which is under rcu_read_lock(),
> > > so a 'kfree_rcu(selem, rcu)' is enough.
> > > A few lines below in bpf_sk_storage_free(), 'kfree_rcu(sk_storage, rcu)'
> > > is currently used instead of call_rcu_tasks_trace() for the same reason.
> > >
> > > KP, if the above makes sense, can you make a patch for it?
> > > The bpf_local_storage_map_free() code path also does not need
> > > call_rcu_tasks_trace(), so may as well change it together.
> > > The bpf_*_storage_delete() helper and the map_{delete,update}_elem()
> > > syscall still require the call_rcu_tasks_trace().
> >
> > Thanks, I will send a patch.
> 
> Martin, does this work? (I can send it as a patch later today)
LGTM. Thanks.