public inbox for cgroups@vger.kernel.org
 help / color / mirror / Atom feed
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

  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