Re: [PATCH] btrfs: do not call synchronize_srcu() in inode_tree_del

From: David Sterba <dsterba@suse.cz>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH] btrfs: do not call synchronize_srcu() in inode_tree_del
Date: Fri, 22 Nov 2019 19:29:27 +0100	[thread overview]
Message-ID: <20191122182927.GE3001@twin.jikos.cz> (raw)
In-Reply-To: <20191119185935.3079-1-josef@toxicpanda.com>

On Tue, Nov 19, 2019 at 01:59:35PM -0500, Josef Bacik wrote:
> Testing with the new fsstress uncovered a pretty nasty deadlock with
> lookup and snapshot deletion.
> 
> Process A
> unlink
>  -> final iput
>    -> inode_tree_del
>      -> synchronize_srcu(subvol_srcu)
> 
> Process B
> btrfs_lookup  <- srcu_read_lock() acquired here
>   -> btrfs_iget
>     -> find inode that has I_FREEING set
>       -> __wait_on_freeing_inode()
> 
> We're holding the srcu_read_lock() while doing the iget in order to make
> sure our fs root doesn't go away, and then we are waiting for the inode
> to finish freeing.  However because the free'ing process is doing a
> synchronize_srcu() we deadlock.
> 
> Fix this by dropping the synchronize_srcu() in inode_tree_del().  We
> don't need people to stop accessing the fs root at this point, we're
> only adding our empty root to the dead roots list.
> 
> A larger much more invasive fix is forthcoming to address how we deal
> with fs roots, but this fixes the immediate problem.
> 
> Fixes: 76dda93c6ae2 ("Btrfs: add snapshot/subvolume destroy ioctl")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to misc-next, thanks.