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 7183AC25B7C for ; Tue, 28 May 2024 14:29:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0CD646B009A; Tue, 28 May 2024 10:29:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 07EAC6B009B; Tue, 28 May 2024 10:29:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E879B6B009C; Tue, 28 May 2024 10:29:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id CA7986B009A for ; Tue, 28 May 2024 10:29:19 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 72E6540637 for ; Tue, 28 May 2024 14:29:19 +0000 (UTC) X-FDA: 82168037238.04.3377E79 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf26.hostedemail.com (Postfix) with ESMTP id 00988140008 for ; Tue, 28 May 2024 14:29:16 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=EGB5RhMK; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf26.hostedemail.com: domain of "SRS0=W8Pr=M7=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 145.40.73.55 as permitted sender) smtp.mailfrom="SRS0=W8Pr=M7=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716906557; h=from:from:sender:reply-to: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=XkWwhrAF4pRP4haGzKU/cfZFGbLKHhzigt0LInkvGrs=; b=RfAlgrdB6A6kPQH4BWChvfVlPIQU4OJazzII1RPKlPL8r6Ss7YqyV4s/Q0SDfUidvy3alF sEMPtokFtZx7HE9fIAbthDVg9g8NE/tMTBrnjdMAPHx/uCYhRkQt3lLMsXenSe4xD/H31w 5TYft553pmZPyjRrh+NQ9uxXnDPaXmo= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=EGB5RhMK; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf26.hostedemail.com: domain of "SRS0=W8Pr=M7=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 145.40.73.55 as permitted sender) smtp.mailfrom="SRS0=W8Pr=M7=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716906557; a=rsa-sha256; cv=none; b=AO7Pg88+jK1h7T2dvQ87Dbb20oY012KnWMUPFRT6Ne9Gdy+eaEpW3jfp+PIsoAKBJ9LH9l UKTBqbY+zfxB6LNTlFjB0oOJu0Kh7e1HPHuPxWz7bQC3f8k9NG5edCCGrAWpeEdLWy0syL p4Ba4Sha//QW9NrnJk4DI6CkKMGHIns= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id BF4F2CE12EA; Tue, 28 May 2024 14:29:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EBC2AC3277B; Tue, 28 May 2024 14:29:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1716906553; bh=NviLABIc30JqHrgbrB574ijIJfU2gfwyOcA6sTzzM6w=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=EGB5RhMKjXS6WAGU/Qa4j6/cyaQ3HWFcY3Lpnz6opVp3E2J9Og3gocMQphIj04RQr 1Xq9B9+7uNO6oFZQnkc+YaDWZbhp0kxoY8xew1WamavWyNqNFcH74gO/vh6Sp4DzWr lI8HvNu6yeS+JLjzD5sj/1+fXedZAmzDiYd30dWq4IdYIG5d8LFel6CqxFot6ZHOYn mtC5FKnwISnMprtodEm/f+9dsOvVL86k/d/RyEAc4MxMXr1eTIhbsHyEpC8Hy/AMJD HzON/6g7/PHQz8eaPouozD23UTdEXzZygq7lQs5JySyuirhs5DPeGCoOnOpcMnGVSg UY3z1JOMHsPCQ== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id 8F454CE0C56; Tue, 28 May 2024 07:29:12 -0700 (PDT) Date: Tue, 28 May 2024 07:29:12 -0700 From: "Paul E. McKenney" To: Uladzislau Rezki Cc: Julia Lawall , Vlastimil Babka , "linux-mm@kvack.org" , RCU , cocci@inria.fr Subject: Re: [cocci] patch idea: convert trivial call_rcu users to kfree_rcu Message-ID: <2ec7eb0f-0950-4d65-b33a-64df7c5f89c9@paulmck-laptop> Reply-To: paulmck@kernel.org References: <68b85796-c674-4691-936a-2051849f4692@suse.cz> <6b7e3ed9-d974-cb3b-6839-a6b7cf5f5a36@inria.fr> <2dd5bafb-ceb1-238d-2f51-83ebf135b73@inria.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 00988140008 X-Stat-Signature: c6ptr6ahe66sfzwbr8s9d4nd8mppoa6o X-HE-Tag: 1716906556-218390 X-HE-Meta: U2FsdGVkX18HQdNaVf/c3UIhcUML+IbL//y5e9/x7BfLZhEejlfF8UL5pffbSlCe2Eilqy2iR551ivf/YyX47DJKphTCpeMwgh+efUU0V4S8gPi2cqKggmAeEsTKGuDw/wqONw0luYwTfgwf7ycuLt9FWy0K9uSJhIhmzh84pHxrNeMiWcD0dwqEjwNKKkBHInF09jU5kRNIFt8lm9ZIX/MxWOd7WHyArJVtDasClqTITuUrLUCqGMA5DOSOtrLwow6J0TQRTla/ffDda6STiVkaNDypw5jEeZkmHAQjgekT3cKJAJ8odr5dPtNfalctfsWltpZvxoP3jtbALkvkv8FYZ4phP6sL4+vUD+CI+rhJTgo8zJxCyNKK/wva9iGrbdNeshH5y3ApaFEqSXejQHRZCrR6/bPVLFJ+UJg3KJ173Om8Z7GbrchagVH64UrDrI/FnaNVUj0zeSf2+tSYiWzXC8NvnhZ9K5Nzm6AIkb8h7mS7f6K4GXeWtOi6icWHF3QagWpo24t0mrQjNjmw494lj5ZnNTA28gNMQ2NgC+CYpoU7io1DLKrMHejjGfl2/iY1dYmcvdboSBOYz3m1Ac76GDTD+LLdjO6Ak2JG7EjtKFvMqmJn3OyvBGGVi2YS1TkW+sipRhXRY0hJVcAkiA4Lmlp1L9HCzrcTdylI/qMClLqRFXm8NpL/UTqMY3+2pw69BMRp1nQblTAayioS/x/s0mhCR+L6L82L4YIvvcx4EH44v8O754aKRxN4iOmH+0aD7bUVRNEVsTvO+4GO3SsURK4wO3EeWmv7U4rCt9OIzVou5bVwu2chaGeC81sDwgMOXjwmjGuOsaFqYP7/Qft9U+MibWgVvXxxnZHMQ+GTW7dpN25bMelNNvZzwMagipa1SQWzoNE1iD5wXpPRag+A7WqRLXioDfLAlqRP4MEQv3TLEs7hXtufbvln9N7AzJuZ8G0nkJATyJ5mrom I+Gq+VN3 zS4yTmHi+1k+IIROItvxilh5UipO0FyKgpIqmIstrH/UXq6GujjfPeHp7ud/bjudBIyDPA4VDxtsNI7h/5BEqHoiAnV7dXtl5SYVuENjh1SCEx2JT1k5l1bO+9j267mb968zdDJwMHDEeV6qJB78V8+aRUcKVBPPYp73rxCxllO6ygE2AWtcmTlQNuTYpUXfrUlTdPFjllmcT+p0RlIuY8r6R2dFsw94grfFL5G0CKb8fd7iwFg2V0jV4bpCC2bIw2Qx4LuJOYbN05PyftWIuIW4kJNVGARZ5X7jyXCxOQlMpp2W+iLkO96gLA1nse45HgPOrA6izUBI21uBhD07CBwQihRzypS3LFBtja50jbCYidTxk8EIlZYw+q/mGUUdRljZi6Tp9m+wBkNttVO/EjzrU6Zupo0G0wsNMhp7DsnUwINSiIeuRkU2+X959/9l5chb2W9UzaHGZ0Nn54vGNsFUPC+d0WhCuqYQHgX+BZI3dzVKXinJTwhnVWdko5ch1ik4IC2fAzuC1scft5yccIU2Oha2qlYQ5y6alIAFVzXmmoEM4JV23mELeRxNSeeY7EoF3BJfyjJdHjAf13YFvYn/AOb9et64VHPnGwj1FDAfVqT1EBu7TPed8JQ== 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 Tue, May 28, 2024 at 03:21:02PM +0200, Uladzislau Rezki wrote: > On Tue, May 28, 2024 at 02:08:20PM +0200, Julia Lawall wrote: > > > > > > On Tue, 28 May 2024, Uladzislau Rezki wrote: > > > > > Hello. > > > > > > > > > > > > > > > On Mon, 27 May 2024, Paul E. McKenney wrote: > > > > > > > > > On Mon, May 27, 2024 at 10:13:40AM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > > > > On Mon, 27 May 2024, Vlastimil Babka wrote: > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > one bit from LSF/MM discussions is that there might be call_rcu users with a > > > > > > > callback that only does a kmem_cache_free() to a specific cache. Since SLOB > > > > > > > was removed, it's always ok to use kfree() and thus also kfree_rcu() on > > > > > > > allocations from kmem_cache_alloc() in addition to kmalloc(). Thus, such > > > > > > > call_rcu() users might be simplified to kfree_rcu(). I found some cases > > > > > > > semi-manually, but I'd expect coccinelle could help here so if anyone wants > > > > > > > to take this task, feel free to. > > > > > > > > > > > > Thanks for the suggestion! I will try to look into it. > > > > > > > > > > Thank you both! > > > > > > > > I found the following functions. Do you want some other information, such > > > > as where they are called from? > > > > > > > > Ignore the -s at the beginning of some lines. Those are for emphasis. not > > > > to suggest to remove the code. > > > > > > > > I checked that the functions are only used in calls to call_rcu. > > > > > > > > Without more effort, Coccinelle only looks for functions defined in the > > > > same file. Here are the functions that are passed to call_rcu where the > > > > function is not defined in the same file: > > > > > > > > need definition for audit_free_rule_rcu > > > > need definition for __i915_gem_free_object_rcu > > > > need definition for io_eventfd_ops > > > > need definition for ip_vs_dest_dst_rcu_free > > > > need definition for __put_task_struct_rcu_cb > > > > need definition for radix_tree_node_rcu_free > > > > > > > > They all do something more, although radix_tree_node_rcu_free doesn't do > > > > much more (some memsets). > > > > > > > > julia > > > > > > > > diff -u -p /home/jll/linux/drivers/net/wireguard/allowedips.c /tmp/nothing/drivers/net/wireguard/allowedips.c > > > > --- /home/jll/linux/drivers/net/wireguard/allowedips.c > > > > +++ /tmp/nothing/drivers/net/wireguard/allowedips.c > > > > @@ -50,7 +50,6 @@ static void push_rcu(struct allowedips_n > > > > > > > > static void node_free_rcu(struct rcu_head *rcu) > > > > { > > > > - kmem_cache_free(node_cache, container_of(rcu, struct allowedips_node, rcu)); > > > > } > > > > > > > > static void root_free_rcu(struct rcu_head *rcu) > > > > diff -u -p /home/jll/linux/fs/ecryptfs/dentry.c /tmp/nothing/fs/ecryptfs/dentry.c > > > > --- /home/jll/linux/fs/ecryptfs/dentry.c > > > > +++ /tmp/nothing/fs/ecryptfs/dentry.c > > > > @@ -53,8 +53,6 @@ struct kmem_cache *ecryptfs_dentry_info_ > > > > > > > > static void ecryptfs_dentry_free_rcu(struct rcu_head *head) > > > > { > > > > - kmem_cache_free(ecryptfs_dentry_info_cache, > > > > - container_of(head, struct ecryptfs_dentry_info, rcu)); > > > > } > > > > > > > > /** > > > > diff -u -p /home/jll/linux/kernel/fork.c /tmp/nothing/kernel/fork.c > > > > --- /home/jll/linux/kernel/fork.c > > > > +++ /tmp/nothing/kernel/fork.c > > > > @@ -378,7 +378,6 @@ static struct kmem_cache *thread_stack_c > > > > > > > > static void thread_stack_free_rcu(struct rcu_head *rh) > > > > { > > > > - kmem_cache_free(thread_stack_cache, rh); > > > > } > > > > > > > > static void thread_stack_delayed_free(struct task_struct *tsk) > > > > diff -u -p /home/jll/linux/kernel/workqueue.c /tmp/nothing/kernel/workqueue.c > > > > --- /home/jll/linux/kernel/workqueue.c > > > > +++ /tmp/nothing/kernel/workqueue.c > > > > @@ -5024,8 +5024,6 @@ fail: > > > > > > > > static void rcu_free_pwq(struct rcu_head *rcu) > > > > { > > > > - kmem_cache_free(pwq_cache, > > > > - container_of(rcu, struct pool_workqueue, rcu)); > > > > } > > > > > > > > /* > > > > diff -u -p /home/jll/linux/net/ipv4/inetpeer.c /tmp/nothing/net/ipv4/inetpeer.c > > > > --- /home/jll/linux/net/ipv4/inetpeer.c > > > > +++ /tmp/nothing/net/ipv4/inetpeer.c > > > > @@ -130,7 +130,6 @@ static struct inet_peer *lookup(const st > > > > > > > > static void inetpeer_free_rcu(struct rcu_head *head) > > > > { > > > > - kmem_cache_free(peer_cachep, container_of(head, struct inet_peer, rcu)); > > > > } > > > > > > > > /* perform garbage collect on all items stacked during a lookup */ > > > > diff -u -p /home/jll/linux/net/ipv6/xfrm6_tunnel.c /tmp/nothing/net/ipv6/xfrm6_tunnel.c > > > > --- /home/jll/linux/net/ipv6/xfrm6_tunnel.c > > > > +++ /tmp/nothing/net/ipv6/xfrm6_tunnel.c > > > > @@ -180,8 +180,6 @@ EXPORT_SYMBOL(xfrm6_tunnel_alloc_spi); > > > > > > > > static void x6spi_destroy_rcu(struct rcu_head *head) > > > > { > > > > - kmem_cache_free(xfrm6_tunnel_spi_kmem, > > > > - container_of(head, struct xfrm6_tunnel_spi, rcu_head)); > > > > } > > > > > > > > static void xfrm6_tunnel_free_spi(struct net *net, xfrm_address_t *saddr) > > > > diff -u -p /home/jll/linux/security/security.c /tmp/nothing/security/security.c > > > > --- /home/jll/linux/security/security.c > > > > +++ /tmp/nothing/security/security.c > > > > @@ -1599,7 +1599,6 @@ static void inode_free_by_rcu(struct rcu > > > > /* > > > > * The rcu head is at the start of the inode blob > > > > */ > > > > - kmem_cache_free(lsm_inode_cache, head); > > > > } > > > > > > > > /** > > > > > > > See below some extra functions which can be eliminated. How i found them: > > > > > > find ./ -name "*.c" -o -name "*.h" | xargs grep -rn call_rcu -B 10 | grep kmem_cache_free > > > > > > > > > static void nfsd4_free_file_rcu(struct rcu_head *rcu); > > > > Thanks. I tried to find this kind of issue, but it turned up nothing. > > There must be some mistake, and I will try again. > > > You are welcome. Also there are several places like below: > > > static void blk_free_queue_rcu(struct rcu_head *rcu_head) > { > struct request_queue *q = container_of(rcu_head, > struct request_queue, rcu_head); > > percpu_ref_exit(&q->q_usage_counter); > kmem_cache_free(blk_requestq_cachep, q); > } > > static void blk_free_queue(struct request_queue *q) > { > blk_free_queue_stats(q->stats); > if (queue_is_mq(q)) > blk_mq_release(q); > > ida_free(&blk_queue_ida, q->id); > call_rcu(&q->rcu_head, blk_free_queue_rcu); > } > > > and potentially it can be also replaced: > > > diff --git a/block/blk-core.c b/block/blk-core.c > index 82c3ae22d76d..898d6990600d 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -257,15 +257,6 @@ void blk_clear_pm_only(struct request_queue *q) > } > EXPORT_SYMBOL_GPL(blk_clear_pm_only); > > -static void blk_free_queue_rcu(struct rcu_head *rcu_head) > -{ > - struct request_queue *q = container_of(rcu_head, > - struct request_queue, rcu_head); > - > - percpu_ref_exit(&q->q_usage_counter); > - kmem_cache_free(blk_requestq_cachep, q); > -} > - > static void blk_free_queue(struct request_queue *q) > { > blk_free_queue_stats(q->stats); > @@ -273,7 +264,8 @@ static void blk_free_queue(struct request_queue *q) > blk_mq_release(q); > > ida_free(&blk_queue_ida, q->id); > - call_rcu(&q->rcu_head, blk_free_queue_rcu); > + percpu_ref_exit(&q->q_usage_counter); > + kfree_rcu(q, rcu_head); > } > > /** > This should be functionally fine, but could inflict serious memory contention on the last few readers. Which, for all I know, might be just fine. As you say: > but a maintainer of "blk-core.c" should check it if a usage counter can > be updated before a completion of GP. > > Probably we can skip as of now such cases, so i do not have a strong > opinion here. Thanx, Paul