[PATCH] wake up s_wait_unfrozen when ->freeze_fs fails

[PATCH] wake up s_wait_unfrozen when ->freeze_fs fails

[Kazuya Mio]

dd slept infinitely when fsfeeze failed because of EIO.
To fix this problem, if ->freeze_fs fails, freeze_super() wakes up
the tasks waiting for the filesystem to become unfrozen.

When s_frozen isn't SB_UNFROZEN in __generic_file_aio_write(),
the function sleeps until FITHAW ioctl wakes up s_wait_unfrozen.

However, if ->freeze_fs fails, s_frozen is set to SB_UNFROZEN and then
freeze_super() returns an error number. In this case, FITHAW ioctl returns
EINVAL because s_frozen is already SB_UNFROZEN. There is no way to wake up
s_wait_unfrozen, so __generic_file_aio_write() sleeps infinitely.

Signed-off-by: Kazuya Mio <k-mio@sx.jp.nec.com>
---
 fs/super.c |    2 ++
 1 file changed, 2 insertions(+)
diff --git a/fs/super.c b/fs/super.c
index 3f56a26..04c7db5 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1161,6 +1161,8 @@ int freeze_super(struct super_block *sb)
 			printk(KERN_ERR
 				"VFS:Filesystem freeze failed\n");
 			sb->s_frozen = SB_UNFROZEN;
+			smp_wmb();
+			wake_up(&sb->s_wait_unfrozen);
 			deactivate_locked_super(sb);
 			return ret;
 		}

Re: [PATCH] wake up s_wait_unfrozen when ->freeze_fs fails

[Christoph Hellwig]

On Thu, Dec 01, 2011 at 04:51:07PM +0900, Kazuya Mio wrote:
> dd slept infinitely when fsfeeze failed because of EIO.
> To fix this problem, if ->freeze_fs fails, freeze_super() wakes up
> the tasks waiting for the filesystem to become unfrozen.
> 
> When s_frozen isn't SB_UNFROZEN in __generic_file_aio_write(),
> the function sleeps until FITHAW ioctl wakes up s_wait_unfrozen.
> 
> However, if ->freeze_fs fails, s_frozen is set to SB_UNFROZEN and then
> freeze_super() returns an error number. In this case, FITHAW ioctl returns
> EINVAL because s_frozen is already SB_UNFROZEN. There is no way to wake up
> s_wait_unfrozen, so __generic_file_aio_write() sleeps infinitely.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

You don't happen to have testcase (e.g. by using dm-fauly or XFS error
injection) that we could add to xfstests for this?

Re: [PATCH] wake up s_wait_unfrozen when ->freeze_fs fails

[Kazuya Mio]

2011/12/07 0:20, Christoph Hellwig wrote:
> On Thu, Dec 01, 2011 at 04:51:07PM +0900, Kazuya Mio wrote:
>> dd slept infinitely when fsfeeze failed because of EIO.
>> To fix this problem, if ->freeze_fs fails, freeze_super() wakes up
>> the tasks waiting for the filesystem to become unfrozen.
>>
>> When s_frozen isn't SB_UNFROZEN in __generic_file_aio_write(),
>> the function sleeps until FITHAW ioctl wakes up s_wait_unfrozen.
>>
>> However, if ->freeze_fs fails, s_frozen is set to SB_UNFROZEN and then
>> freeze_super() returns an error number. In this case, FITHAW ioctl returns
>> EINVAL because s_frozen is already SB_UNFROZEN. There is no way to wake up
>> s_wait_unfrozen, so __generic_file_aio_write() sleeps infinitely.
>
> Looks good,
>
> Reviewed-by: Christoph Hellwig<hch@lst.de>
>
> You don't happen to have testcase (e.g. by using dm-fauly or XFS error
> injection) that we could add to xfstests for this?

The problem can be reproduced by the following steps:

1. make faulty md device with mdadm
    # mdadm -B /dev/md1000 -l faulty -n 1 -R <device>

2. mkfs and mount filesystem
    # mkfs.ext4 /dev/md1000
    # mount /dev/md1000 <mount point>

3. run 1000 dd commands in the background to create one 10MB file
    # for NO in {1..1000} ; do dd if=/dev/zero of=<mount point>/testfile \
      bs=1M count=10 ;done

4. sleep three seconds for the adjustment with the time of fault injection
    # sleep 3

5. inject the md device with fault in the background
    # mdadm -G /dev/md1000 -l faulty -p wp1 &

6. freeze filesystem with fsfreeze (and it fails because of EIO)
    # freeze -f <mount point>

As a result, dd executed in step 3 sleeps forever.

Regards,
Kazuya Mio