From: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
To: Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>
Cc: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>,
Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
Alexander Viro
<viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
Dennis Zhou <dennis-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Dave Chinner <dchinner-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
Subject: Re: [PATCH v8 3/8] writeback, cgroup: increment isw_nr_in_flight before grabbing an inode
Date: Tue, 8 Jun 2021 10:45:44 +0200 [thread overview]
Message-ID: <20210608084544.GB5562@quack2.suse.cz> (raw)
In-Reply-To: <20210608013123.1088882-4-guro-b10kYP2dOMg@public.gmane.org>
On Mon 07-06-21 18:31:18, Roman Gushchin wrote:
> isw_nr_in_flight is used do determine whether the inode switch queue
> should be flushed from the umount path. Currently it's increased
> after grabbing an inode and even scheduling the switch work. It means
> the umount path can be walked past cleanup_offline_cgwb() with active
> inode references, which can result in a "Busy inodes after unmount."
> message and use-after-free issues (with inode->i_sb which gets freed).
>
> Fix it by incrementing isw_nr_in_flight before doing anything with
> the inode and decrementing in the case when switching wasn't scheduled.
>
> The problem hasn't yet been seen in the real life and was discovered
> by Jan Kara by looking into the code.
>
> Suggested-by: Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
> Signed-off-by: Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Honza
> ---
> fs/fs-writeback.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 3564efcc4b78..e2cc860a001b 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -505,6 +505,8 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
> if (!isw)
> return;
>
> + atomic_inc(&isw_nr_in_flight);
> +
> /* find and pin the new wb */
> rcu_read_lock();
> memcg_css = css_from_id(new_wb_id, &memory_cgrp_subsys);
> @@ -535,11 +537,10 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
> * Let's continue after I_WB_SWITCH is guaranteed to be visible.
> */
> call_rcu(&isw->rcu_head, inode_switch_wbs_rcu_fn);
> -
> - atomic_inc(&isw_nr_in_flight);
> return;
>
> out_free:
> + atomic_dec(&isw_nr_in_flight);
> if (isw->new_wb)
> wb_put(isw->new_wb);
> kfree(isw);
> --
> 2.31.1
>
--
Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
SUSE Labs, CR
next prev parent reply other threads:[~2021-06-08 8:45 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-08 1:31 [PATCH v8 0/8] cgroup, blkcg: prevent dirty inodes to pin dying memory cgroups Roman Gushchin
2021-06-08 1:31 ` [PATCH v8 1/8] writeback, cgroup: do not switch inodes with I_WILL_FREE flag Roman Gushchin
2021-06-08 1:31 ` [PATCH v8 4/8] writeback, cgroup: switch to rcu_work API in inode_switch_wbs() Roman Gushchin
[not found] ` <20210608013123.1088882-1-guro-b10kYP2dOMg@public.gmane.org>
2021-06-08 1:31 ` [PATCH v8 2/8] writeback, cgroup: add smp_mb() to cgroup_writeback_umount() Roman Gushchin
2021-06-08 8:43 ` Jan Kara
2021-06-08 1:31 ` [PATCH v8 3/8] writeback, cgroup: increment isw_nr_in_flight before grabbing an inode Roman Gushchin
[not found] ` <20210608013123.1088882-4-guro-b10kYP2dOMg@public.gmane.org>
2021-06-08 8:45 ` Jan Kara [this message]
2021-06-08 1:31 ` [PATCH v8 5/8] writeback, cgroup: keep list of inodes attached to bdi_writeback Roman Gushchin
2021-06-08 1:31 ` [PATCH v8 6/8] writeback, cgroup: split out the functional part of inode_switch_wbs_work_fn() Roman Gushchin
2021-06-08 1:31 ` [PATCH v8 8/8] writeback, cgroup: release dying cgwbs by switching attached inodes Roman Gushchin
2021-06-08 8:54 ` Jan Kara
[not found] ` <20210608013123.1088882-9-guro-b10kYP2dOMg@public.gmane.org>
2021-06-08 16:08 ` Dennis Zhou
[not found] ` <YL+WC5beBH/N0ddz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2021-06-08 22:37 ` Roman Gushchin
2021-06-08 1:31 ` [PATCH v8 7/8] writeback, cgroup: support switching multiple inodes at once Roman Gushchin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210608084544.GB5562@quack2.suse.cz \
--to=jack-alswssmvlrq@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=dchinner-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=dennis-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=guro-b10kYP2dOMg@public.gmane.org \
--cc=jack-IBi9RG/b67k@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox