FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 10-19-2010, 10:07 PM
Mike Snitzer
 
Default dm: use revalidate_disk to update device size after set_capacity

Avoid taking md->bdev->bd_inode->i_mutex to update the DM block device's
size. Doing so eliminates the potential for deadlock if an fsync is
racing with a device resize.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index f934e98..fd315a7 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1995,10 +1995,7 @@ static void event_callback(void *context)
static void __set_size(struct mapped_device *md, sector_t size)
{
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);
+ revalidate_disk(md->disk);
}

/*

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 10-20-2010, 05:42 AM
"Jun'ichi Nomura"
 
Default dm: use revalidate_disk to update device size after set_capacity

Hi Mike,

(10/20/10 07:07), Mike Snitzer wrote:
> Avoid taking md->bdev->bd_inode->i_mutex to update the DM block device's
> size. Doing so eliminates the potential for deadlock if an fsync is
> racing with a device resize.
>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
> drivers/md/dm.c | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index f934e98..fd315a7 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1995,10 +1995,7 @@ static void event_callback(void *context)
> static void __set_size(struct mapped_device *md, sector_t size)
> {
> 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);
> + revalidate_disk(md->disk);
> }

Some concerns/questions:

- revalidate_disk() calls bdget() inside it.
Does bdget() no longer block on I/O?
Sometime ago, bdget() has been blocked by I_LOCK,
where process holding I_LOCK blocked by resize.
Since I_LOCK has disappeared, I suspect this might not
be a valid concern anymore.
FYI, past discussion on this topic:
http://www.redhat.com/archives/dm-devel/2007-October/msg00134.html
(it's not my intention to push the patch in the above URL)

- revalidate_disk() ends up with get_super():
revalidate_disk
check_disk_size_change
flush_disk
__invalidate_device
get_super
and get_super() takes sb->s_umount.
OTOH, there are codes which wait on I/O holding s_umount
exclusively. E.g. freeze_super() calls sync_filesystem().
So it seems there is a possible deadlock if you use
revalidate_disk from dm_swap_table.

I've been away from that part of the code for a while.
So it's nice if the above is just a false alarm...

Thanks,
--
Jun'ichi Nomura, NEC Corporation

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 12:29 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org