On Wed, Nov 02, 2022 at 08:17:37PM +0800, Yu Kuai wrote:
> I think this is still not safe 🤔
Indeed - wrong open_mutex.
> + /*
> + * del_gendisk drops the initial reference to bd_holder_dir, so we
> need
> + * to keep our own here to allow for cleanup past that point.
> + */
> + mutex_lock(&bdev->bd_disk->open_mutex);
> + if (!disk_live(bdev->bd_disk)) {
> + ret = -ENODEV;
> + mutex_unlock(&bdev->bd_disk->open_mutex);
> + goto out_unlock;
> + }
I think this needs to be done before taking disk->open_mutex, otherwise
we create a lock order dependency. Also the comment seems to overflow
the usual 80 character limit, and as you noted in the next mail this
needs more unwinding. But yes, otherwise this is the right thing to
do. Do you want to send a replacement for this patch?
