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 <email@example.com>, Jens Axboe <firstname.lastname@example.org>, Josef Bacik <email@example.com>, Minchan Kim <firstname.lastname@example.org>, Nitin Gupta <email@example.com>, Jan Kara <firstname.lastname@example.org>, "Darrick J . Wong" <email@example.com>, Ming Lei <firstname.lastname@example.org>, Matteo Croce <email@example.com>, firstname.lastname@example.org, email@example.com
- Subject: Re: [PATCH 13/14] loop: remove lo_refcount and avoid lo_mutex in ->open / ->release
- From: Christoph Hellwig <firstname.lastname@example.org>
- 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: <[🔎] email@example.com> <[🔎] firstname.lastname@example.org> <[🔎] 03628e13-ca56-4ed0-da5a-ee698c83f48d@I-love.SAKURA.ne.jp>
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.