Date: Tue, 15 Mar 2016 08:51:45 +0100
From: Christoph Hellwig <hch@lst.de>
To: Jan Kara <jack@suse.cz>
Cc: Rabin Vincent <rabin.vincent@axis.com>, viro@zeniv.linux.org.uk,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        Rabin Vincent <rabinv@axis.com>, jaxboe@kernel.dk, hch@lst.de
Subject: Re: [PATCH] block: protect iterate_bdevs() against concurrent close
Message-ID: <20160315075145.GA11004@lst.de>
References: <1457612763-23196-1-git-send-email-rabin.vincent@axis.com> <20160310173727.GA19795@quack.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160310173727.GA19795@quack.suse.cz>
User-Agent: Mutt/1.5.17 (2007-11-01)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1232
Lines: 21

On Thu, Mar 10, 2016 at 06:37:27PM +0100, Jan Kara wrote:
> Thanks for spotting the problem. The patch will fix the problem you found.
> But what prevents e.g. flusher thread from trying to writeback the block
> device inode while that gets invalidated at the same moment? I.e., the
> problem doesn't seem to be specific to iterate_bdevs() but rather hits
> anything which writes back block device inodes, right?
> 
> Frankly it seems like a catch that mapping_cap_foo() requires block device
> to be open (if mapping belongs to a block device) to be safe. The trouble
> is we'd need to make inode_to_bdi() result stable while we work with the
> block device inode. Christoph, do you have any idea how to cleanly achieve
> that? All I'm able to come up with are ugly hacks...

I think the whole idea of clearing a queue from a block device
just because it's not open is the root of the problem.  Or, to dive
deeper into the issue our whole object model and life time rules
between struct block_device, struct gendisk, struct request_queue and
struct backing_dev_info are rather broken.  I'd done a few attempt at
sorting this out, but it's a huge task and I haven't gotten far.

I think for now the patch from Rabin should be fine.