[dm-devel] Re: [PATCH] drop mutex in __set_size

[Jun'ichi Nomura]

Hi Mikulas,

Mikulas Patocka wrote:
> Drop the mutex.
> 
> It doesn't make sense to lock it for a single assignment, this code can't
> be executed concurrently. The size should be read with i_size_read which
> is automatically protected against concurrent i_size_write.

But it seems there are codes which depend on i_mutex for
protected access to i_size: e.g. block_write_begin().

> This locking can lead to a deadlock, if someone holds i_mutex while
> the device is being suspended, described by Jun'ichi Nomura at
> https://www.redhat.com/archives/dm-devel/2009-May/msg00097.html

And while my description only mentioned noflush suspend,
with 2.6.31-rc, the deadlock can occur under normal suspend
because the new barrier code may push bios to deferred queue.

> Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>
> 
> ---
>  drivers/md/dm.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> Index: linux-2.6.31-rc3-devel/drivers/md/dm.c
> ===================================================================
> --- linux-2.6.31-rc3-devel.orig/drivers/md/dm.c	2009-07-28 01:20:17.000000000 +0200
> +++ linux-2.6.31-rc3-devel/drivers/md/dm.c	2009-07-28 01:20:27.000000000 +0200
> @@ -1901,9 +1901,7 @@ static void __set_size(struct mapped_dev
>  {
>  	set_capacity(md->disk, size);
>  
> -	mutex_lock(&md->bdev->bd_inode->i_mutex);
>  	i_size_write(md->bdev->bd_inode, (loff_t)size << SECTOR_SHIFT);
> -	mutex_unlock(&md->bdev->bd_inode->i_mutex);
>  }
>  
>  static int __bind(struct mapped_device *md, struct dm_table *t,

-- 
Jun'ichi Nomura, NEC Corporation