Re: [PATCH 13/14] loop: remove lo_refcount and avoid lo_mutex in ->open / ->release
- To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
- Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>, Josef Bacik <josef@toxicpanda.com>, Minchan Kim <minchan@kernel.org>, Nitin Gupta <ngupta@vflare.org>, Jan Kara <jack@suse.cz>, "Darrick J . Wong" <djwong@kernel.org>, Ming Lei <ming.lei@redhat.com>, Matteo Croce <mcroce@microsoft.com>, linux-block@vger.kernel.org, nbd@other.debian.org
- Subject: Re: [PATCH 13/14] loop: remove lo_refcount and avoid lo_mutex in ->open / ->release
- From: Christoph Hellwig <hch@lst.de>
- Date: Tue, 29 Mar 2022 08:52:34 +0200
- Message-id: <[🔎] 20220329065234.GA20006@lst.de>
- In-reply-to: <[🔎] 03628e13-ca56-4ed0-da5a-ee698c83f48d@I-love.SAKURA.ne.jp>
- References: <[🔎] 20220325063929.1773899-1-hch@lst.de> <[🔎] 20220325063929.1773899-14-hch@lst.de> <[🔎] 03628e13-ca56-4ed0-da5a-ee698c83f48d@I-love.SAKURA.ne.jp>
Hi Tetsuo,
I'm a bit confused. Partially due to the two patches in one mail, but
also because I can't actually find a try that they cleanly apply to.
But let me add my thoughts here:
- I think the change
On Sat, Mar 26, 2022 at 11:52:36AM +0900, Tetsuo Handa wrote:
> Since __loop_clr_fd() is currently holding loop_validate_mutex and lo->lo_mutex,
> "avoid lo_mutex in ->release" part is incomplete.
>
> The intent of holding loop_validate_mutex (which causes disk->open_mutex =>
> loop_validate_mutex => lo->lo_mutex => blk_mq_freeze_queue()/blk_mq_unfreeze_queue()
> chain) is to make loop_validate_file() traversal safe.
>
> Since ->release is called with disk->open_mutex held, and __loop_clr_fd() from
> lo_release() is called via ->release when disk_openers() == 0, we are guaranteed
> that "struct file" which will be passed to loop_validate_file() via fget() cannot
> be the loop device __loop_clr_fd(lo, true) will clear. Thus, there is no need to
> hold loop_validate_mutex and lo->lo_mutex from __loop_clr_fd() if release == true.
This part looks reasonable. Can you give a signoff and proper commit
log and I'll slot it in before the lo_refcount removal.
Reply to: