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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B9A4DC433EF for ; Fri, 15 Oct 2021 03:34:41 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 58CAF61168 for ; Fri, 15 Oct 2021 03:34:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 58CAF61168 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id AB1D66B006C; Thu, 14 Oct 2021 23:34:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A61F96B0071; Thu, 14 Oct 2021 23:34:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 901E5900002; Thu, 14 Oct 2021 23:34:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0235.hostedemail.com [216.40.44.235]) by kanga.kvack.org (Postfix) with ESMTP id 7D0D86B006C for ; Thu, 14 Oct 2021 23:34:40 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 2ACB339B89 for ; Fri, 15 Oct 2021 03:34:40 +0000 (UTC) X-FDA: 78697254720.13.08BBDEF Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) by imf06.hostedemail.com (Postfix) with ESMTP id 69921801A89B for ; Fri, 15 Oct 2021 03:34:39 +0000 (UTC) Received: by mail-pj1-f44.google.com with SMTP id e5-20020a17090a804500b001a116ad95caso764165pjw.2 for ; Thu, 14 Oct 2021 20:34:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=b1bdfAAZWj+ZhLRSlWkR4KBpma1mylAV7bjBGXhcWH0=; b=DggTWqaS+nMUxNrALxZPAzcyWSwOUf/nnubZpV7ujoDk441SNWN69rh+s0Otvhv+E7 2a3MN+ghSZJqgmRXyKkkoIuyZYL8c+emnAxGix2t2n6gFbPOnoO4b3PyFFPdBGfOjt3v 4u4nOZVV8OSiT5otd/W22EYNTrmDCiQWEfqohfNlz6pC9qZDkSfYEeEd/M3dg9kkjGCg AL+JSUN8feMlq9VI3jaNKc9OKHOqxd7QvJczGw/m60flXDMRhUGWHgoFmca30zHhYb6F /RPwMyouyYZ2lHac60kLCuvejh7WjGYBu/rc810c5bp/gehGqxzfpt4cqCsxsqTIYLuO ltbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=b1bdfAAZWj+ZhLRSlWkR4KBpma1mylAV7bjBGXhcWH0=; b=ZawwZShyMrCgitvZCI8bPpB3fakYvrLuAMyyloRevxED3hYVNTr3Ldi4NVOluRPRXZ Nmrx140U7uGpPrMeV8FXIv4kS5j2hHV3kO4QOheTTaYJeulnFUd2SlVH79GVQ16BRSi/ 7nygxdQh7kL23uZIMFI3JdJW/Amk1DUW7bw69yzxORNsvoVtAfzzJwsM2I0Exn0jYq9N 2Sv/dGcJKj9ksfu0bqiDXL5mt/R4eZYeiiQc3bHs1shbeKwriK5pweJl/8Ph5zWpuj0F 2eNcGVQkEghSHVKpyZ6hVQaDP8SeL2UmiLmG1umRPzesDT7tp2yL1cG8wJGw8lmLRC+v C6pg== X-Gm-Message-State: AOAM533niw3FZASFYi5D76bFYGHYzOLTt3j5W1vs8qWC10Svq+KEZ6Dp 5WuI2O80VNFyQ5hjc+uqk3MvQaAO4wCOO6R4nY8= X-Google-Smtp-Source: ABdhPJzr+D+vA1IqMNQxO2aJsu8MyCM/APPbavEekhgVERSRoEF30kWu07Xd8vbfWJ/rx5d8VTql22ZpKyKxmEAxGDY= X-Received: by 2002:a17:90b:1804:: with SMTP id lw4mr10635055pjb.174.1634268878709; Thu, 14 Oct 2021 20:34:38 -0700 (PDT) MIME-Version: 1.0 References: <20211014082433.30733-1-qiang.zhang1211@gmail.com> In-Reply-To: From: Qiang Zhang Date: Fri, 15 Oct 2021 11:34:27 +0800 Message-ID: Subject: Re: [PATCH] mm: backing-dev: use kfree_rcu() instead of synchronize_rcu_expedited() To: Matthew Wilcox , hch@infradead.org Cc: akpm@linux-foundation.org, sunhao.th@gmail.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: multipart/alternative; boundary="000000000000fba72105ce5bdf41" Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=DggTWqaS; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of qiang.zhang1211@gmail.com designates 209.85.216.44 as permitted sender) smtp.mailfrom=qiang.zhang1211@gmail.com X-Stat-Signature: 13pauqyrcgqfess1eqz7hbw9bdputadt X-Rspamd-Queue-Id: 69921801A89B X-Rspamd-Server: rspam01 X-HE-Tag: 1634268879-597600 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: --000000000000fba72105ce5bdf41 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Qiang Zhang =E4=BA=8E2021=E5=B9=B410=E6=9C=8815= =E6=97=A5=E5=91=A8=E4=BA=94 =E4=B8=8A=E5=8D=8810:57=E5=86=99=E9=81=93=EF=BC= =9A > > > Matthew Wilcox =E4=BA=8E2021=E5=B9=B410=E6=9C=8814= =E6=97=A5=E5=91=A8=E5=9B=9B =E4=B8=8B=E5=8D=887:26=E5=86=99=E9=81=93=EF=BC= =9A > >> On Thu, Oct 14, 2021 at 04:24:33PM +0800, Zqiang wrote: >> > The bdi_remove_from_list() is called in RCU softirq, however the >> > synchronize_rcu_expedited() will produce sleep action, use kfree_rcu() >> > instead of it. >> > >> > Reported-by: Hao Sun >> > Signed-off-by: Zqiang >> > --- >> > include/linux/backing-dev-defs.h | 1 + >> > mm/backing-dev.c | 4 +--- >> > 2 files changed, 2 insertions(+), 3 deletions(-) >> > >> > diff --git a/include/linux/backing-dev-defs.h >> b/include/linux/backing-dev-defs.h >> > index 33207004cfde..35a093384518 100644 >> > --- a/include/linux/backing-dev-defs.h >> > +++ b/include/linux/backing-dev-defs.h >> > @@ -202,6 +202,7 @@ struct backing_dev_info { >> > #ifdef CONFIG_DEBUG_FS >> > struct dentry *debug_dir; >> > #endif >> > + struct rcu_head rcu; >> > }; >> >> >Instead of growing struct backing_dev_info, it seems to me this rcu_hea= d >> >could be placed in a union with rb_node, since it will have been remove= d >> >from the bdi_tree by this point and the tree is never walked under >> >RCU protection? >> >> > Thanks for your advice, I find this bdi_tree is traversed under the > protection of a spin lock, not under the protection of RCU. > I find this modification does not avoid the problem described in patch, > the flush_delayed_work() may be called in release_bdi() > The same will cause problems. > > may be we can replace queue_rcu_work() of call_rcu(&inode->i_rcu, > i_callback) or do you have any better suggestions? > > Thanks > Zqiang > > --000000000000fba72105ce5bdf41 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
Qiang Zhang <qiang.zhang1211@gmail.com> =E4=BA=8E2021=E5= =B9=B410=E6=9C=8815=E6=97=A5=E5=91=A8=E4=BA=94 =E4=B8=8A=E5=8D=8810:57=E5= =86=99=E9=81=93=EF=BC=9A


Matthew Wilcox <willy@infradead.org&g= t; =E4=BA=8E2021=E5=B9=B410=E6=9C=8814=E6=97=A5=E5=91=A8=E5=9B=9B =E4=B8=8B= =E5=8D=887:26=E5=86=99=E9=81=93=EF=BC=9A
On Thu, Oct 14, 2021 at 04:24:33PM +0800, Zqiang w= rote:
> The bdi_remove_from_list() is called in RCU softirq, however the
> synchronize_rcu_expedited() will produce sleep action, use kfree_rcu()=
> instead of it.
>
> Reported-by: Hao Sun <sunhao.th@gmail.com>
> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>
> ---
>=C2=A0 include/linux/backing-dev-defs.h | 1 +
>=C2=A0 mm/backing-dev.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0| 4 +---
>=C2=A0 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-= dev-defs.h
> index 33207004cfde..35a093384518 100644
> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -202,6 +202,7 @@ struct backing_dev_info {
>=C2=A0 #ifdef CONFIG_DEBUG_FS
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct dentry *debug_dir;
>=C2=A0 #endif
> +=C2=A0 =C2=A0 =C2=A0struct rcu_head rcu;
>=C2=A0 };

>Instead of growing struct backing_dev_info, it seems to me this rcu= _head
>could be placed in a union with rb_node, since it will have be= en removed
>from the bdi_tree by this point and the tree is never wal= ked under
>RCU protection?


Thanks for your advice, I find this bd= i_tree=C2=A0is traversed under the protection of a spin lock, not under the= protection of RCU.
I find this modification does not avoid the p= roblem described in patch, the=C2=A0flush_delayed_work() may be called in= =C2=A0release_bdi()
The same will cause problems.
= =C2=A0
may be=C2=A0 we can replace queue_rcu_work() of call_rcu(&= amp;inode->i_rcu, i_callback) or do you have any better suggestions?

Thanks
Zqiang
=C2=A0
<= /div>
--000000000000fba72105ce5bdf41--