On Wed 26-01-22 16:50:37, Christoph Hellwig wrote:
> lo_refcnt is only incremented in lo_open and decremented in lo_release,
> and thus protected by open_mutex. Only take lo_mutex when lo_open is
> called the first time, as only for the first open there is any affect
> on the driver state (incremental opens on partitions don't end up in
> lo_open at all already).
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> drivers/block/loop.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 43980ec69dfdd..4b0058a67c48e 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1725,13 +1725,16 @@ static int lo_open(struct block_device *bdev, fmode_t mode)
> struct loop_device *lo = bdev->bd_disk->private_data;
> int err;
>
> + if (atomic_inc_return(&lo->lo_refcnt) > 1)
> + return 0;
> +
> err = mutex_lock_killable(&lo->lo_mutex);
> if (err)
> return err;
> - if (lo->lo_state == Lo_deleting)
> + if (lo->lo_state == Lo_deleting) {
> + atomic_dec(&lo->lo_refcnt);
> err = -ENXIO;
> - else
> - atomic_inc(&lo->lo_refcnt);
> + }
> mutex_unlock(&lo->lo_mutex);
> return err;
So this also relies on disk->open_mutex for correctness. Otherwise a race
like:
Thread1 Thread2
lo_open()
if (atomic_inc_return(&lo->lo_refcnt) > 1)
mutex_lock_killable(&lo->lo_mutex);
lo_open()
if (atomic_inc_return(&lo->lo_refcnt) > 1)
return 0;
..
can result in Thread2 using the loop device before Thread1 actually did the
"first open" checks. So perhaps one common comment for lo_open + lo_release
explaining the locking?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
