LinuxLists.cc - [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Mon 07-02-11 20:53:25, Masayoshi MIZUMA wrote:
> Hi,
>
> When I checked the freeze feature for ext4 filesystem using fsfreeze command
> at 2.6.38-rc3, I got the following messeges:
>
> ---------------------------------------------------------------------
> Feb 7 15:05:09 RX300S6 kernel: INFO: task fsfreeze:2104 blocked for more than 120 seconds.
> Feb 7 15:05:09 RX300S6 kernel: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> Feb 7 15:05:09 RX300S6 kernel: fsfreeze D ffff880076d5f040 0 2104 2018 0x00000000
> Feb 7 15:05:09 RX300S6 kernel: ffff88005a9f3d98 0000000000000086 ffff88005a9f3d38 ffffffff00000000
> Feb 7 15:05:09 RX300S6 kernel: 0000000000014d40 ffff880076d5eab0 ffff880076d5f040 ffff88005a9f3fd8
> Feb 7 15:05:09 RX300S6 kernel: ffff880076d5f048 0000000000014d40 ffff88005a9f2010 0000000000014d40
> Feb 7 15:05:09 RX300S6 kernel: Call Trace:
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff814aa5f5>] rwsem_down_failed_common+0xb5/0x140
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff814aa693>] rwsem_down_write_failed+0x13/0x20
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff8122f1a3>] call_rwsem_down_write_failed+0x13/0x20
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff814a9c12>] ? down_write+0x32/0x40
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff81155b48>] thaw_super+0x28/0xd0
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff81164338>] do_vfs_ioctl+0x368/0x560
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff81157c73>] ? sys_newfstat+0x33/0x40
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff811645d1>] sys_ioctl+0xa1/0xb0
> Feb 7 15:05:09 RX300S6 kernel: [<ffffffff8100bf82>] system_call_fastpath+0x16/0x1b
> ...
> Feb 7 15:07:09 RX300S6 kernel: INFO: task flush-8:0:1409 blocked for more than 120 seconds.
> Feb 7 15:07:09 RX300S6 kernel: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> Feb 7 15:07:09 RX300S6 kernel: flush-8:0 D ffff880037777a30 0 1409 2 0x00000000
> Feb 7 15:07:09 RX300S6 kernel: ffff880037c95a80 0000000000000046 ffff88007c8037a0 0000000000000000
> Feb 7 15:07:09 RX300S6 kernel: 0000000000014d40 ffff8800377774a0 ffff880037777a30 ffff880037c95fd8
> Feb 7 15:07:09 RX300S6 kernel: ffff880037777a38 0000000000014d40 ffff880037c94010 0000000000014d40
> Feb 7 15:07:09 RX300S6 kernel: Call Trace:
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffffa00abb85>] ext4_journal_start_sb+0x75/0x130 [ext4]
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81082fc0>] ? autoremove_wake_function+0x0/0x40
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffffa0097f0a>] ext4_da_writepages+0x27a/0x640 [ext4]
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81102c91>] do_writepages+0x21/0x40
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff811776b8>] writeback_single_inode+0x98/0x240
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81177cfe>] writeback_sb_inodes+0xce/0x170
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81178709>] writeback_inodes_wb+0x99/0x160
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81178a8b>] wb_writeback+0x2bb/0x430
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81178e2c>] wb_do_writeback+0x22c/0x280
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81178f32>] bdi_writeback_thread+0xb2/0x260
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81178e80>] ? bdi_writeback_thread+0x0/0x260
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81178e80>] ? bdi_writeback_thread+0x0/0x260
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff81082936>] kthread+0x96/0xa0
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff8100cdc4>] kernel_thread_helper+0x4/0x10
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff810828a0>] ? kthread+0x0/0xa0
> Feb 7 15:07:09 RX300S6 kernel: [<ffffffff8100cdc0>] ? kernel_thread_helper+0x0/0x10
> ---------------------------------------------------------------------
>
> I think the following deadlock problem happened:
>
> [flush-8:0:1409] | [fsfreeze:2104]
> --------------------------------------------+--------------------------------
> writeback_inodes_wb |
> pin_sb_for_writeback |
> down_read_trylock(&sb->s_umount) |
> writeback_sb_inodes |thaw_super
> writeback_single_inode | down_write(&sb->s_umount)
> do_writepages | # stop until flush-8:0 releases
> ext4_da_writepages | # read lock of sb->s_umount...
> ext4_journal_start_sb |
> vfs_check_frozen |
> wait_event((sb)->s_wait_unfrozen, |
> ((sb)->s_frozen < (level))) |
> # stop until being waked up by |
> # fsfreeze... |
> --------------------------------------------+--------------------------------
>
> Could anyone check this problem?
Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
under s_umount semaphore, we are prone to deadlock like the one you
describe above.

Looking at the code, s_frozen acts as lock but it's lock ranking is
unclear. Logically, the only sane ranking seems to be to rank above all
other VFS locks because we return with s_frozen held to userspace. But then
the need to wait for s_frozen from inside the filesystem violates this
ranking and causes the above deadlock.

Gosh, this is so broken. The whole thing is made even worse because
filesystems can take different locks in their unfreeze_fs callbacks and we
can possibly deadlock on these locks the same way as we do on s_umount
semaphore.

I have to think how this can be possibly fixed...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-02-15 17:04:00

by Theodore Ts'o

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> under s_umount semaphore, we are prone to deadlock like the one you
> describe above.

One of the fundamental problems here is that the freeze and thaw
routines are using down_write(&sb->s_umount) for two purposes. The
first is to prevent the resume/thaw from racing with a umount (which
it could do just as well by taking a read lock), but the second is to
prevent the resume/thaw code from racing with itself. That's the core
fundamental problem here.

So I think we can solve this by introduce a new mutex, s_freeze, and
having the the resume/thaw first take the s_freeze mutex and then
second take a read lock on the s_umount.

- Ted

2011-02-15 17:29:54

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > under s_umount semaphore, we are prone to deadlock like the one you
> > describe above.
>
> One of the fundamental problems here is that the freeze and thaw
> routines are using down_write(&sb->s_umount) for two purposes. The
> first is to prevent the resume/thaw from racing with a umount (which
> it could do just as well by taking a read lock), but the second is to
> prevent the resume/thaw code from racing with itself. That's the core
> fundamental problem here.
>
> So I think we can solve this by introduce a new mutex, s_freeze, and
> having the the resume/thaw first take the s_freeze mutex and then
> second take a read lock on the s_umount.
Sadly this does not quite work because even down_read(&sb->s_umount)
in thaw_super() can block if there is another process that tries to acquire
s_umount for writing - a situation like:
TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
down_read(&sb->s_umount)
block on s_frozen
down_write(&sb->s_umount)
-blocked
down_read(&sb->s_umount)
-blocked
behind the write access...

The only working solution I see is to check for frozen filesystem before
taking s_umount semaphore which seems rather ugly (but might be bearable if
we did so in some well described wrapper).

And in particular ext4 has another deadlock of this kind because it does
IO from ext4_remount() e.g. when doing online resize (I know it's a bit
artifical but still ;).

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-02-15 18:04:42

by Theodore Ts'o

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Tue, Feb 15, 2011 at 06:29:54PM +0100, Jan Kara wrote:
> Sadly this does not quite work because even down_read(&sb->s_umount)
> in thaw_super() can block if there is another process that tries to acquire
> s_umount for writing - a situation like:
> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> down_read(&sb->s_umount)
> block on s_frozen
> down_write(&sb->s_umount)
> -blocked
> down_read(&sb->s_umount)
> -blocked
> behind the write access...

OK, sorry for being dense, but why does this cause a deadlock? What
are you imaging TASK 3 doing that would impede the flusher from
eventually resuming? Or how would TASK 3 prevent userspace from
completing whatever it needs to do (say, a device mapper ioctl)?

freeze_fs has always been inherently dangerous if the userspace does
not know what it's doing. If it freezes the root file system, and
then while the file system is frozen, userspace attempts to modify
/etc/mtab, it's going to lose. I've in the past argued for some kind
of safety timeout that prevents the system from wedging, but the
argument I've gotten back is (a) it's too complex, and (b) userspace
programmers aren't that stupid, and (c) it could cause the filesystem
to unfreeze when userspace wasn't expecting it. Oh, and (d) if the
system wedges up due to userspace being stupid, it's acceptable.

Obviously, if the kernel does something to itself that causes a
deadlock, we need to fix it, but userspace doing something stupid has
been explicitly ruled out of scope, at least in previous
discussions...

> And in particular ext4 has another deadlock of this kind because it does
> IO from ext4_remount() e.g. when doing online resize (I know it's a bit
> artifical but still ;).

OK, I'm being dense again. How does remount and online resize relate
with each other? and it's not I/O in general which is a problem, it's
writeback activity which causes a problem because it takes a read lock
on s_umount, right?

- Ted

2011-02-15 19:11:25

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Tue 15-02-11 13:04:35, Ted Ts'o wrote:
> On Tue, Feb 15, 2011 at 06:29:54PM +0100, Jan Kara wrote:
> > Sadly this does not quite work because even down_read(&sb->s_umount)
> > in thaw_super() can block if there is another process that tries to acquire
> > s_umount for writing - a situation like:
> > TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > down_read(&sb->s_umount)
> > block on s_frozen
> > down_write(&sb->s_umount)
> > -blocked
> > down_read(&sb->s_umount)
> > -blocked
> > behind the write access...
>
> OK, sorry for being dense, but why does this cause a deadlock? What
> are you imaging TASK 3 doing that would impede the flusher from
> eventually resuming? Or how would TASK 3 prevent userspace from
> completing whatever it needs to do (say, a device mapper ioctl)?
I was arguing that using down_read(sb->s_umount) in thaw_super() instead
of down_write() does not solve anything. The deadlock as originally
reported can still happen, you just need another task (TASK 2 in the above
scheme) to block in down_write() before thaw_super() happens.

> freeze_fs has always been inherently dangerous if the userspace does
> not know what it's doing. If it freezes the root file system, and
> then while the file system is frozen, userspace attempts to modify
> /etc/mtab, it's going to lose. I've in the past argued for some kind
> of safety timeout that prevents the system from wedging, but the
> argument I've gotten back is (a) it's too complex, and (b) userspace
> programmers aren't that stupid, and (c) it could cause the filesystem
> to unfreeze when userspace wasn't expecting it. Oh, and (d) if the
> system wedges up due to userspace being stupid, it's acceptable.
>
> Obviously, if the kernel does something to itself that causes a
> deadlock, we need to fix it, but userspace doing something stupid has
> been explicitly ruled out of scope, at least in previous
> discussions...
>
> > And in particular ext4 has another deadlock of this kind because it does
> > IO from ext4_remount() e.g. when doing online resize (I know it's a bit
> > artifical but still ;).
>
> OK, I'm being dense again. How does remount and online resize relate
> with each other? and it's not I/O in general which is a problem, it's
> writeback activity which causes a problem because it takes a read lock
> on s_umount, right?
The problem is to start a transaction while holding s_umount semaphore,
or actually any lock that thaw_super() (including per-filesystem
->unfreeze_fs() callback) needs. For ext4 this seems to be sb->s_lock.
I was actually wrong with the ext4 online resizing using resize option
causing possible deadlocks because do_remount_sb() refuses to do anything
with the superblock while it is frozen... But still if we ever happen to
start a transaction in ext4 while sb->s_lock is held, the deadlock with
freezing code can happen and that's just subtle and ugly IMHO.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-02-15 23:17:46

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi.

On Tue, 15 Feb 2011 18:29:54 +0100
Jan Kara <[email protected]> wrote:
> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > under s_umount semaphore, we are prone to deadlock like the one you
> > > describe above.
> >
> > One of the fundamental problems here is that the freeze and thaw
> > routines are using down_write(&sb->s_umount) for two purposes. The
> > first is to prevent the resume/thaw from racing with a umount (which
> > it could do just as well by taking a read lock), but the second is to
> > prevent the resume/thaw code from racing with itself. That's the core
> > fundamental problem here.
> >
> > So I think we can solve this by introduce a new mutex, s_freeze, and
> > having the the resume/thaw first take the s_freeze mutex and then
> > second take a read lock on the s_umount.
> Sadly this does not quite work because even down_read(&sb->s_umount)
> in thaw_super() can block if there is another process that tries to acquire
> s_umount for writing - a situation like:
> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> down_read(&sb->s_umount)
> block on s_frozen
> down_write(&sb->s_umount)
> -blocked
> down_read(&sb->s_umount)
> -blocked
> behind the write access...
>
> The only working solution I see is to check for frozen filesystem before
> taking s_umount semaphore which seems rather ugly (but might be bearable if
> we did so in some well described wrapper).
I created the patch that you imagine yesterday.

I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
without a fixed patch. After an hour, I confirmed that this deadlock happened.

However, on the kernel with a fixed patch, this deadlock doesn't still happen
after 12 hours passed.

The patch for linux-2.6.38-rc4 is as follows:
---
fs/fs-writeback.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 59c6e49..1c9a05e 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
spin_unlock(&sb_lock);

if (down_read_trylock(&sb->s_umount)) {
- if (sb->s_root)
+ if (sb->s_frozen == SB_UNFROZEN && sb->s_root)
return true;
up_read(&sb->s_umount);
}
--

Best Regards,
Toshiyuki Okajima

2011-02-16 14:56:30

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> On Tue, 15 Feb 2011 18:29:54 +0100
> Jan Kara <[email protected]> wrote:
> > On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > > Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > > under s_umount semaphore, we are prone to deadlock like the one you
> > > > describe above.
> > >
> > > One of the fundamental problems here is that the freeze and thaw
> > > routines are using down_write(&sb->s_umount) for two purposes. The
> > > first is to prevent the resume/thaw from racing with a umount (which
> > > it could do just as well by taking a read lock), but the second is to
> > > prevent the resume/thaw code from racing with itself. That's the core
> > > fundamental problem here.
> > >
> > > So I think we can solve this by introduce a new mutex, s_freeze, and
> > > having the the resume/thaw first take the s_freeze mutex and then
> > > second take a read lock on the s_umount.
> > Sadly this does not quite work because even down_read(&sb->s_umount)
> > in thaw_super() can block if there is another process that tries to acquire
> > s_umount for writing - a situation like:
> > TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > down_read(&sb->s_umount)
> > block on s_frozen
> > down_write(&sb->s_umount)
> > -blocked
> > down_read(&sb->s_umount)
> > -blocked
> > behind the write access...
> >
> > The only working solution I see is to check for frozen filesystem before
> > taking s_umount semaphore which seems rather ugly (but might be bearable if
> > we did so in some well described wrapper).
> I created the patch that you imagine yesterday.
>
> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> without a fixed patch. After an hour, I confirmed that this deadlock happened.
>
> However, on the kernel with a fixed patch, this deadlock doesn't still happen
> after 12 hours passed.
>
> The patch for linux-2.6.38-rc4 is as follows:
> ---
> fs/fs-writeback.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 59c6e49..1c9a05e 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> spin_unlock(&sb_lock);
>
> if (down_read_trylock(&sb->s_umount)) {
> - if (sb->s_root)
> + if (sb->s_frozen == SB_UNFROZEN && sb->s_root)
> return true;
> up_read(&sb->s_umount);
So this is something along the lines I thought but it actually won't work
for example if sync(1) is run while the filesystem is frozen (that takes
s_umount semaphore in a different place). And generally, I'm not convinced
there are not other places that try to do IO while holding s_umount
semaphore...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-02-17 03:50:13

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

(2011/02/16 23:56), Jan Kara wrote:
> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>> On Tue, 15 Feb 2011 18:29:54 +0100
>> Jan Kara<[email protected]> wrote:
>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
>>>>> Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
>>>>> under s_umount semaphore, we are prone to deadlock like the one you
>>>>> describe above.
>>>>
>>>> One of the fundamental problems here is that the freeze and thaw
>>>> routines are using down_write(&sb->s_umount) for two purposes. The
>>>> first is to prevent the resume/thaw from racing with a umount (which
>>>> it could do just as well by taking a read lock), but the second is to
>>>> prevent the resume/thaw code from racing with itself. That's the core
>>>> fundamental problem here.
>>>>
>>>> So I think we can solve this by introduce a new mutex, s_freeze, and
>>>> having the the resume/thaw first take the s_freeze mutex and then
>>>> second take a read lock on the s_umount.
>>> Sadly this does not quite work because even down_read(&sb->s_umount)
>>> in thaw_super() can block if there is another process that tries to acquire
>>> s_umount for writing - a situation like:
>>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
>>> down_read(&sb->s_umount)
>>> block on s_frozen
>>> down_write(&sb->s_umount)
>>> -blocked
>>> down_read(&sb->s_umount)
>>> -blocked
>>> behind the write access...
>>>
>>> The only working solution I see is to check for frozen filesystem before
>>> taking s_umount semaphore which seems rather ugly (but might be bearable if
>>> we did so in some well described wrapper).
>> I created the patch that you imagine yesterday.
>>
>> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
>> without a fixed patch. After an hour, I confirmed that this deadlock happened.
>>
>> However, on the kernel with a fixed patch, this deadlock doesn't still happen
>> after 12 hours passed.
>>
>> The patch for linux-2.6.38-rc4 is as follows:
>> ---
>> fs/fs-writeback.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
>> index 59c6e49..1c9a05e 100644
>> --- a/fs/fs-writeback.c
>> +++ b/fs/fs-writeback.c
>> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
>> spin_unlock(&sb_lock);
>>
>> if (down_read_trylock(&sb->s_umount)) {
>> - if (sb->s_root)
>> + if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
>> return true;
>> up_read(&sb->s_umount);

> So this is something along the lines I thought but it actually won't work
> for example if sync(1) is run while the filesystem is frozen (that takes
> s_umount semaphore in a different place). And generally, I'm not convinced
> there are not other places that try to do IO while holding s_umount
> semaphore...
OK. I understand.

This code only fixes the case for the following path:
writeback_inodes_wb
-> ext4_da_writepages
-> ext4_journal_start_sb
-> vfs_check_frozen
But, the code doesn't fix the other cases.

We must modify the local filesystem part in order to fix all cases...?

Regards,
Toshiyuki Okajima

2011-02-17 05:13:53

by Andreas Dilger

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 2011-02-16, at 20:50, Toshiyuki Okajima wrote:
> (2011/02/16 23:56), Jan Kara wrote:
>>
>>> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel without a fixed patch. After an hour, I confirmed that this deadlock happened.
>>>
>>> However, on the kernel with a fixed patch, this deadlock doesn't still happen
>>> after 12 hours passed.
>>>
>>> The patch for linux-2.6.38-rc4 is as follows:
>>> ---
>>> fs/fs-writeback.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
>>> index 59c6e49..1c9a05e 100644
>>> --- a/fs/fs-writeback.c
>>> +++ b/fs/fs-writeback.c
>>> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
>>> spin_unlock(&sb_lock);
>>>
>>> if (down_read_trylock(&sb->s_umount)) {
>>> - if (sb->s_root)
>>> + if (sb->s_frozen == SB_UNFROZEN && sb->s_root)
>>> return true;
>>> up_read(&sb->s_umount);

This seems like a very low-risk fix.

>> So this is something along the lines I thought but it actually won't work
>> for example if sync(1) is run while the filesystem is frozen (that takes
>> s_umount semaphore in a different place). And generally, I'm not convinced
>> there are not other places that try to do IO while holding s_umount
>> semaphore...
>
> OK. I understand.
>
> This code only fixes the case for the following path:
> writeback_inodes_wb
> -> ext4_da_writepages
> -> ext4_journal_start_sb
> -> vfs_check_frozen
> But, the code doesn't fix the other cases.
>
> We must modify the local filesystem part in order to fix all cases...?

It seems worthwhile to implement the low-risk fix that covers the common case, and if/when someone hits the rare 3-process case and/or submits a patch for it then that one will be fixed also.

Cheers, Andreas

2011-02-17 10:41:26

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed 16-02-11 22:13:53, Andreas Dilger wrote:
> On 2011-02-16, at 20:50, Toshiyuki Okajima wrote:
> > (2011/02/16 23:56), Jan Kara wrote:
> >>
> >>> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel without a fixed patch. After an hour, I confirmed that this deadlock happened.
> >>>
> >>> However, on the kernel with a fixed patch, this deadlock doesn't still happen
> >>> after 12 hours passed.
> >>>
> >>> The patch for linux-2.6.38-rc4 is as follows:
> >>> ---
> >>> fs/fs-writeback.c | 2 +-
> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> >>> index 59c6e49..1c9a05e 100644
> >>> --- a/fs/fs-writeback.c
> >>> +++ b/fs/fs-writeback.c
> >>> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> >>> spin_unlock(&sb_lock);
> >>>
> >>> if (down_read_trylock(&sb->s_umount)) {
> >>> - if (sb->s_root)
> >>> + if (sb->s_frozen == SB_UNFROZEN && sb->s_root)
> >>> return true;
> >>> up_read(&sb->s_umount);
>
> This seems like a very low-risk fix.
>
> >> So this is something along the lines I thought but it actually won't work
> >> for example if sync(1) is run while the filesystem is frozen (that takes
> >> s_umount semaphore in a different place). And generally, I'm not convinced
> >> there are not other places that try to do IO while holding s_umount
> >> semaphore...
> >
> > OK. I understand.
> >
> > This code only fixes the case for the following path:
> > writeback_inodes_wb
> > -> ext4_da_writepages
> > -> ext4_journal_start_sb
> > -> vfs_check_frozen
> > But, the code doesn't fix the other cases.
> >
> > We must modify the local filesystem part in order to fix all cases...?
>
> It seems worthwhile to implement the low-risk fix that covers the common
> case, and if/when someone hits the rare 3-process case and/or submits a
> patch for it then that one will be fixed also.
Yes, the fix is simple enough that I won't oppose it getting in as a
band aid and if we add this band aid to fs/sync.c:sync_one_sb(), it would
even be a reasonably reliable band aid. But that doesn't change the fact
that the locking is simply broken ;).

Honza

--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-02-17 10:45:54

[permalink] [raw]

Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> (2011/02/16 23:56), Jan Kara wrote:
> >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> >>On Tue, 15 Feb 2011 18:29:54 +0100
> >>Jan Kara<[email protected]> wrote:
> >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> >>>>>describe above.
> >>>>
> >>>>One of the fundamental problems here is that the freeze and thaw
> >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> >>>>first is to prevent the resume/thaw from racing with a umount (which
> >>>>it could do just as well by taking a read lock), but the second is to
> >>>>prevent the resume/thaw code from racing with itself. That's the core
> >>>>fundamental problem here.
> >>>>
> >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> >>>>having the the resume/thaw first take the s_freeze mutex and then
> >>>>second take a read lock on the s_umount.
> >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> >>>in thaw_super() can block if there is another process that tries to acquire
> >>>s_umount for writing - a situation like:
> >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> >>>down_read(&sb->s_umount)
> >>> block on s_frozen
> >>> down_write(&sb->s_umount)
> >>> -blocked
> >>> down_read(&sb->s_umount)
> >>> -blocked
> >>>behind the write access...
> >>>
> >>>The only working solution I see is to check for frozen filesystem before
> >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> >>>we did so in some well described wrapper).
> >>I created the patch that you imagine yesterday.
> >>
> >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> >>
> >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> >>after 12 hours passed.
> >>
> >>The patch for linux-2.6.38-rc4 is as follows:
> >>---
> >> fs/fs-writeback.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> >>index 59c6e49..1c9a05e 100644
> >>--- a/fs/fs-writeback.c
> >>+++ b/fs/fs-writeback.c
> >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> >> spin_unlock(&sb_lock);
> >>
> >> if (down_read_trylock(&sb->s_umount)) {
> >>- if (sb->s_root)
> >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> >> return true;
> >> up_read(&sb->s_umount);
>
> > So this is something along the lines I thought but it actually won't work
> >for example if sync(1) is run while the filesystem is frozen (that takes
> >s_umount semaphore in a different place). And generally, I'm not convinced
> >there are not other places that try to do IO while holding s_umount
> >semaphore...
> OK. I understand.
>
> This code only fixes the case for the following path:
> writeback_inodes_wb
> -> ext4_da_writepages
> -> ext4_journal_start_sb
> -> vfs_check_frozen
> But, the code doesn't fix the other cases.
>
> We must modify the local filesystem part in order to fix all cases...?
Yes, possibly. But most importantly we should first find clear locking
rules for frozen filesystem that avoid deadlocks like the one above. And
the freezing / unfreezing code might become subtle for that reason, that's
fine, but it would be really good to avoid any complicated things for the
code in the rest of the VFS / filesystems.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-03-28 09:28:26

[permalink] [raw]

Subject: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi.

On Thu, 17 Feb 2011 11:45:52 +0100
Jan Kara <[email protected]> wrote:
> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> > (2011/02/16 23:56), Jan Kara wrote:
> > >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> > >>On Tue, 15 Feb 2011 18:29:54 +0100
> > >>Jan Kara<[email protected]> wrote:
> > >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> > >>>>>describe above.
> > >>>>
> > >>>>One of the fundamental problems here is that the freeze and thaw
> > >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> > >>>>first is to prevent the resume/thaw from racing with a umount (which
> > >>>>it could do just as well by taking a read lock), but the second is to
> > >>>>prevent the resume/thaw code from racing with itself. That's the core
> > >>>>fundamental problem here.
> > >>>>
> > >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> > >>>>having the the resume/thaw first take the s_freeze mutex and then
> > >>>>second take a read lock on the s_umount.
> > >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> > >>>in thaw_super() can block if there is another process that tries to acquire
> > >>>s_umount for writing - a situation like:
> > >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > >>>down_read(&sb->s_umount)
> > >>> block on s_frozen
> > >>> down_write(&sb->s_umount)
> > >>> -blocked
> > >>> down_read(&sb->s_umount)
> > >>> -blocked
> > >>>behind the write access...
> > >>>
> > >>>The only working solution I see is to check for frozen filesystem before
> > >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> > >>>we did so in some well described wrapper).
> > >>I created the patch that you imagine yesterday.
> > >>
> > >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> > >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> > >>
> > >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> > >>after 12 hours passed.
> > >>
> > >>The patch for linux-2.6.38-rc4 is as follows:
> > >>---
> > >> fs/fs-writeback.c | 2 +-
> > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > >>
> > >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > >>index 59c6e49..1c9a05e 100644
> > >>--- a/fs/fs-writeback.c
> > >>+++ b/fs/fs-writeback.c
> > >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> > >> spin_unlock(&sb_lock);
> > >>
> > >> if (down_read_trylock(&sb->s_umount)) {
> > >>- if (sb->s_root)
> > >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> > >> return true;
> > >> up_read(&sb->s_umount);
> >
> > > So this is something along the lines I thought but it actually won't work
> > >for example if sync(1) is run while the filesystem is frozen (that takes
> > >s_umount semaphore in a different place). And generally, I'm not convinced
> > >there are not other places that try to do IO while holding s_umount
> > >semaphore...
> > OK. I understand.
> >
> > This code only fixes the case for the following path:
> > writeback_inodes_wb
> > -> ext4_da_writepages
> > -> ext4_journal_start_sb
> > -> vfs_check_frozen
> > But, the code doesn't fix the other cases.
> >
> > We must modify the local filesystem part in order to fix all cases...?
> Yes, possibly. But most importantly we should first find clear locking
> rules for frozen filesystem that avoid deadlocks like the one above. And
> the freezing / unfreezing code might become subtle for that reason, that's
> fine, but it would be really good to avoid any complicated things for the
> code in the rest of the VFS / filesystems.
I have deeply continued to examined the root cause of this problem, then
I found it.

It is that we can write a memory which is mmaped to a file. Then the memory
becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
"writeback" the memory.

Therefore, the root cause of this hangup is not only ext4 component (with
delayed allocation feature) but also writeback mechanism for mmap. If you
use the other filesystem, you can write something to the filesystem though
you have freezed the filesystem.

A sample problem is attached on this mail. Try to execute it then you can
confirm that we can write some data to your filesystem while freezing the
filesystem.
(If you change FS variable in go.sh from ext3 to ext4 and you execute
"fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)

I think the best approach to fix this problem is to let users not to write
memory which is mapped to a certain file while the filesystem is freezing.
However, it is very difficult to control users not to write memory which has
been already mapped to the file.

Therefore, I think there is only actual method that we stop writeback thread
to resolve the mmap problem. Also, by this fix, the original problem
(ext4 delayed write vs unfreeze) can be solved.

I created a patch for this problem. Please confirm it.

------------------------------------------------------------------------------
----------
reproducer
----------
[run script] go.sh
#!/bin/sh

FS=ext3
gcc -o ./write ./write.c
dd if=/dev/zero of=/tmp/loop.$$ bs=1k seek=64k count=1 > /dev/null 2>&1
/sbin/mkfs.$FS -Fq /tmp/loop.$$
/sbin/losetup /dev/loop7 /tmp/loop.$$
mkdir -p mnt
mount -t $FS /dev/loop7 mnt
dd if=/dev/zero of=mnt/file bs=4k count=100 > /dev/null 2>&1
./write mnt/file &
pid=$!
# write 0 then 1
/bin/kill -SIGUSR1 $pid
/bin/kill -SIGUSR1 $pid
/sbin/fsfreeze -f mnt
cp /tmp/loop.$$ /tmp/loop.$$.pre
/bin/kill -SIGUSR1 $pid
sync
sleep 30
cp /tmp/loop.$$ /tmp/loop.$$.post
/sbin/fsfreeze -u mnt
/bin/kill -SIGTERM $pid
umount mnt
/sbin/losetup -d /dev/loop7
/usr/bin/cmp /tmp/loop.$$.pre /tmp/loop.$$.post > /dev/null 2>&1
if [ $? -ne 0 ]; then
echo "freeze doesn't work correctly!"
else
echo "freeze works correctly!"
fi
rm -f /tmp/loop.$$*
exit 0

[program] write.c
#define LARGEFILE64_SOURCE

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <signal.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>

int counter = 0;
char *mmap_addr;
int fd;

#define LOOP 100
#define UNIT 4096
#define MMAPSZ (UNIT*LOOP)
#define FILENAME "./mnt/file"

void
write_inc(int sig)
{
int i;

for (i = 0; i < LOOP; i++)
*((int*)(mmap_addr + UNIT*i)) = counter;
counter ++;
}

void
main_exit(int sig)
{
munmap(mmap_addr, MMAPSZ);
close(fd);
exit(0);
}

int main(int argc, char *argv[])
{
char *file = FILENAME;

if ((fd = open(file, O_RDWR)) < 0) {
perror("open error");
exit(1);
}
if ((mmap_addr = mmap(0, MMAPSZ, PROT_WRITE, MAP_SHARED, fd, 0)) ==
MAP_FAILED) {
perror("mmap error");
close(fd);
exit(2);
}
sigset(SIGTERM, (void *)main_exit);
sigset(SIGUSR1, (void *)write_inc);
while (1)
pause();
}

[step to rerproduce]
# sh ./go.sh
------------------------------------------------------------------------------

[patch]
Now, we can write the memory which is mapped to a file while
the filesystem to which it belongs is being freezed.
Therefore, the filesystem can modify even if it is being freezed.
This fix prevents the flusher thread from updating the filesystem.

Signed-off-by: Toshiyuki Okajima <[email protected]>
---
fs/fs-writeback.c | 2 +-
fs/super.c | 7 ++++++-
mm/page-writeback.c | 2 ++
3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index b5ed541..2a60148 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -477,7 +477,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
spin_unlock(&sb_lock);

if (down_read_trylock(&sb->s_umount)) {
- if (sb->s_root)
+ if (sb->s_frozen == 0 && sb->s_root)
return true;
up_read(&sb->s_umount);
}
diff --git a/fs/super.c b/fs/super.c
index 8a06881..bac28c4 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -432,8 +432,13 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
continue;
sb->s_count++;
spin_unlock(&sb_lock);
-
+retry:
down_read(&sb->s_umount);
+ if (sb->s_frozen > 0) {
+ up_read(&sb->s_umount);
+ cond_resched();
+ goto retry;
+ }
if (sb->s_root)
f(sb, arg);
up_read(&sb->s_umount);
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 31f6988..eb19642 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1058,7 +1058,9 @@ EXPORT_SYMBOL(generic_writepages);
int do_writepages(struct address_space *mapping, struct writeback_control *wbc)
{
int ret;
+ struct super_block *sb = mapping->host->i_sb;

+ vfs_check_frozen(sb, SB_FREEZE_TRANS);
if (wbc->nr_to_write <= 0)
return 0;
if (mapping->a_ops->writepages)
--
1.5.5.6

2011-03-30 14:12:09

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hello,

On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
> On Thu, 17 Feb 2011 11:45:52 +0100
> Jan Kara <[email protected]> wrote:
> > On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> > > (2011/02/16 23:56), Jan Kara wrote:
> > > >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> > > >>On Tue, 15 Feb 2011 18:29:54 +0100
> > > >>Jan Kara<[email protected]> wrote:
> > > >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> > > >>>>>describe above.
> > > >>>>
> > > >>>>One of the fundamental problems here is that the freeze and thaw
> > > >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> > > >>>>first is to prevent the resume/thaw from racing with a umount (which
> > > >>>>it could do just as well by taking a read lock), but the second is to
> > > >>>>prevent the resume/thaw code from racing with itself. That's the core
> > > >>>>fundamental problem here.
> > > >>>>
> > > >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> > > >>>>having the the resume/thaw first take the s_freeze mutex and then
> > > >>>>second take a read lock on the s_umount.
> > > >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> > > >>>in thaw_super() can block if there is another process that tries to acquire
> > > >>>s_umount for writing - a situation like:
> > > >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > > >>>down_read(&sb->s_umount)
> > > >>> block on s_frozen
> > > >>> down_write(&sb->s_umount)
> > > >>> -blocked
> > > >>> down_read(&sb->s_umount)
> > > >>> -blocked
> > > >>>behind the write access...
> > > >>>
> > > >>>The only working solution I see is to check for frozen filesystem before
> > > >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> > > >>>we did so in some well described wrapper).
> > > >>I created the patch that you imagine yesterday.
> > > >>
> > > >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> > > >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> > > >>
> > > >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> > > >>after 12 hours passed.
> > > >>
> > > >>The patch for linux-2.6.38-rc4 is as follows:
> > > >>---
> > > >> fs/fs-writeback.c | 2 +-
> > > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > > >>
> > > >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > > >>index 59c6e49..1c9a05e 100644
> > > >>--- a/fs/fs-writeback.c
> > > >>+++ b/fs/fs-writeback.c
> > > >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> > > >> spin_unlock(&sb_lock);
> > > >>
> > > >> if (down_read_trylock(&sb->s_umount)) {
> > > >>- if (sb->s_root)
> > > >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> > > >> return true;
> > > >> up_read(&sb->s_umount);
> > >
> > > > So this is something along the lines I thought but it actually won't work
> > > >for example if sync(1) is run while the filesystem is frozen (that takes
> > > >s_umount semaphore in a different place). And generally, I'm not convinced
> > > >there are not other places that try to do IO while holding s_umount
> > > >semaphore...
> > > OK. I understand.
> > >
> > > This code only fixes the case for the following path:
> > > writeback_inodes_wb
> > > -> ext4_da_writepages
> > > -> ext4_journal_start_sb
> > > -> vfs_check_frozen
> > > But, the code doesn't fix the other cases.
> > >
> > > We must modify the local filesystem part in order to fix all cases...?
> > Yes, possibly. But most importantly we should first find clear locking
> > rules for frozen filesystem that avoid deadlocks like the one above. And
> > the freezing / unfreezing code might become subtle for that reason, that's
> > fine, but it would be really good to avoid any complicated things for the
> > code in the rest of the VFS / filesystems.
> I have deeply continued to examined the root cause of this problem, then
> I found it.
>
> It is that we can write a memory which is mmaped to a file. Then the memory
> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> "writeback" the memory.
>
> Therefore, the root cause of this hangup is not only ext4 component (with
> delayed allocation feature) but also writeback mechanism for mmap. If you
> use the other filesystem, you can write something to the filesystem though
> you have freezed the filesystem.
Well, you can write something only in the caches, not to the on disk
image. So it's not a problem as such.

> A sample problem is attached on this mail. Try to execute it then you can
> confirm that we can write some data to your filesystem while freezing the
> filesystem.
> (If you change FS variable in go.sh from ext3 to ext4 and you execute
> "fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
>
> I think the best approach to fix this problem is to let users not to write
> memory which is mapped to a certain file while the filesystem is freezing.
> However, it is very difficult to control users not to write memory which has
> been already mapped to the file.
It is actually possible. In case of ext4, you could add a check (+ wait)
in ext4_page_mkwrite() whether the filesystem is frozen or in the process
of being frozen and if so, wait for it to get unfrozen. The only tough
problem here might be the locking as ext4_page_mkwrite() is called with
mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
But you'd have to fix all filesystems (and all paths possibly creating
dirty data) in this way.

> Therefore, I think there is only actual method that we stop writeback thread
> to resolve the mmap problem. Also, by this fix, the original problem
> (ext4 delayed write vs unfreeze) can be solved.
Hmm, I had a look at the code again and think we could fix the issue
cleanly (i.e. all possible users of s_umount) as follows: The lock
ordering will be
s_umount -> "fs frozen"
and there will be a new mutex s_freeze_mutex protecting changes of
s_frozen.

freeze_bdev() already observes this lock ordering, it will only take
s_freeze_mutex for the changes of s_frozen values. The only other code
that is relevant for the lock ordering is thaw_super() (the freezing
process is not expected to reenter kernel for the frozen filesystem).
In thaw_super() we could take s_freeze_mutex, do all the thawing work,
set s_frozen, release s_freeze_mutex and put superblock reference.

So something like the patch below - it seems to work for me, can you test
it please?

>From 0939f4c2fd5d69d7d1bf7ece9a641bb561e9d0dd Mon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Wed, 30 Mar 2011 15:21:44 +0200
Subject: [PATCH] vfs: Fix deadlocks on frozen filesystem

When a filesystem is frozen and the flusher thread decides to do writeback
for the frozen filesystem (e.g. because pages were marked dirty by mmaped
write) we deadlock because we take s_umount semaphore and then try to write
dirty pages which blocks. In this situation there is no way to unfreeze
the filesystem because thawing code requires s_umount semaphore.

Fix the problem removing the need to take s_umount from thawing code. Instead
we introduce new s_freeze_mutex to provide necessary exclusion.

Reported-by: Toshiyuki Okajima <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
---
fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
include/linux/fs.h | 1 +
2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index e848649..4f74718 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -77,6 +77,7 @@ static struct super_block *alloc_super(struct file_system_type *type)
INIT_LIST_HEAD(&s->s_dentry_lru);
init_rwsem(&s->s_umount);
mutex_init(&s->s_lock);
+ mutex_init(&s->s_freeze_mutex);
lockdep_set_class(&s->s_umount, &type->s_umount_key);
/*
* The locking rules for s_lock are up to the
@@ -971,6 +972,24 @@ out:
* Syncs the super to make sure the filesystem is consistent and calls the fs's
* freeze_fs. Subsequent calls to this without first thawing the fs will return
* -EBUSY.
+ *
+ * Locking of freeze / thaw is tricky (if not messy). Freezing is protected by
+ * exclusively taking s_umount to avoid races with mount / remount / umount and
+ * also provide exclusion of concurrent freeze calls. Then we have
+ * s_freeze_mutex which protects changes to s_frozen and the call ->freeze_fs()
+ * against races with thawing code.
+ *
+ * Thawing code must not take s_umount before the filesystem is unfrozen
+ * because that would cause deadlocks (e.g. background flushing takes s_umount
+ * and then does writeback which blocks on a frozen filesystem). So we take
+ * only s_freeze_mutex, which provides us exclusion against concurrent
+ * freezing, and hold it until the thawing is finished. We are protected
+ * against superblock going away by holding an active sb reference and against
+ * remounting by the fact that the sb is frozen.
+ *
+ * Notes: s_freeze_mutex cannot be merged with bd_fsfreeze_mutex because we
+ * can freeze block devices without filesystems and also freeze filesystems
+ * not backed by block devices.
*/
int freeze_super(struct super_block *sb)
{
@@ -978,7 +997,9 @@ int freeze_super(struct super_block *sb)

atomic_inc(&sb->s_active);
down_write(&sb->s_umount);
- if (sb->s_frozen) {
+ mutex_lock(&sb->s_freeze_mutex);
+ if (sb->s_frozen != SB_UNFROZEN) {
+ mutex_unlock(&sb->s_freeze_mutex);
deactivate_locked_super(sb);
return -EBUSY;
}
@@ -986,15 +1007,18 @@ int freeze_super(struct super_block *sb)
if (sb->s_flags & MS_RDONLY) {
sb->s_frozen = SB_FREEZE_TRANS;
smp_wmb();
+ mutex_unlock(&sb->s_freeze_mutex);
up_write(&sb->s_umount);
return 0;
}

sb->s_frozen = SB_FREEZE_WRITE;
+ mutex_unlock(&sb->s_freeze_mutex);
smp_wmb();

sync_filesystem(sb);

+ mutex_lock(&sb->s_freeze_mutex);
sb->s_frozen = SB_FREEZE_TRANS;
smp_wmb();

@@ -1005,10 +1029,12 @@ int freeze_super(struct super_block *sb)
printk(KERN_ERR
"VFS:Filesystem freeze failed\n");
sb->s_frozen = SB_UNFROZEN;
+ mutex_unlock(&sb->s_freeze_mutex);
deactivate_locked_super(sb);
return ret;
}
}
+ mutex_unlock(&sb->s_freeze_mutex);
up_write(&sb->s_umount);
return 0;
}
@@ -1019,14 +1045,15 @@ EXPORT_SYMBOL(freeze_super);
* @sb: the super to thaw
*
* Unlocks the filesystem and marks it writeable again after freeze_super().
+ * See freeze_super() for locking comments.
*/
int thaw_super(struct super_block *sb)
{
int error;

- down_write(&sb->s_umount);
- if (sb->s_frozen == SB_UNFROZEN) {
- up_write(&sb->s_umount);
+ mutex_lock(&sb->s_freeze_mutex);
+ if (sb->s_frozen != SB_FREEZE_TRANS) {
+ mutex_unlock(&sb->s_freeze_mutex);
return -EINVAL;
}

@@ -1039,7 +1066,7 @@ int thaw_super(struct super_block *sb)
printk(KERN_ERR
"VFS:Filesystem thaw failed\n");
sb->s_frozen = SB_FREEZE_TRANS;
- up_write(&sb->s_umount);
+ mutex_unlock(&sb->s_freeze_mutex);
return error;
}
}
@@ -1048,7 +1075,8 @@ out:
sb->s_frozen = SB_UNFROZEN;
smp_wmb();
wake_up(&sb->s_wait_unfrozen);
- deactivate_locked_super(sb);
+ mutex_unlock(&sb->s_freeze_mutex);
+ deactivate_super(sb);

return 0;
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7061a85..230892d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1382,6 +1382,7 @@ struct super_block {
struct dentry *s_root;
struct rw_semaphore s_umount;
struct mutex s_lock;
+ struct mutex s_freeze_mutex;
int s_count;
atomic_t s_active;
#ifdef CONFIG_SECURITY
--
1.7.1

2011-03-31 08:37:27

by Yongqiang Yang

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi everyone,

Amir met a deadlock when he tested ext4 with snapshot. The deadlock
was reported on
https://github.com/amir73il/ext4-snapshots/commit/56396185d922a73524a091b545e665543abf741a.
It is difficult to reproduce the deadlock. There is a deadlock
reported on http://www.spinics.net/lists/linux-ext4/msg23018.html.
Actually, these two deadlocks come from a same source.

Below are my analysis on the 1st one. Mail is not a good place to
describe parallel processes. I have submitted the analysis to
https://github.com/YANGYongqiang/ext4-snapshots/blob/9e0ae9ae9907125e6bf45aa91db296d4cc041b17/fs/ext4/BUGS#L143.
It is much more readable.

-- deadlock in ext4 with snapshot
ext4 with snapshot calls freeze_super() to bring
a fs be in a clean state when a user takes a snapshot.

freeze truncate kjournald

| ext4_ext_truncate |
freeze_super() | starts a handle |
sets s_frozen | |
| ext4_ext_truncate |
| holds i_data_sem |
ext4_freeze() | | commit_transaction()
wait for updates | | waits for i_data_sem
| ext4_free_blocks |
| calls dquot_free_block|
| |
| dquot_free_block call |
| ext4_dirty_inode |
| |
| ext4_dirty_inode |
| trys to start a handle|
| |
| block due to s_frozen |

in ext3, ext3_freeze() prevents journal from being updated by
lock_journal_updates(), ext3_unfreeze() allow journal to be updated by
unlock_journal_updates().

in ext4, however, before ext4_freeze() returns, it unlock journal, and
ext4 prevents journal from being updated by s_frozen. s_frozen is in
an upper layer, so it is out control of ext4 and deadlock is easy to
happen.

Could someone explain why ext4 does like above but not follow ext3?

Yongqiang.
--
Best Wishes
Yongqiang Yang

2011-03-31 08:48:40

by Yongqiang Yang

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Thu, Mar 31, 2011 at 4:37 PM, Yongqiang Yang <[email protected]> wrote:
> Hi everyone,
>
> Amir met a deadlock when he tested ext4 with snapshot. ?The deadlock
> was reported on
> https://github.com/amir73il/ext4-snapshots/commit/56396185d922a73524a091b545e665543abf741a.
> ?It is difficult to reproduce the deadlock. ?There is a deadlock
> reported on http://www.spinics.net/lists/linux-ext4/msg23018.html.
> Actually, these two deadlocks come from a same source.
>
> Below are my analysis on the 1st one. ?Mail is not a good place to
> describe parallel processes. ?I have submitted the analysis to
> https://github.com/YANGYongqiang/ext4-snapshots/blob/9e0ae9ae9907125e6bf45aa91db296d4cc041b17/fs/ext4/BUGS#L143.
> ?It is much more readable.
>
> -- deadlock in ext4 with snapshot
> ? ? ? ext4 with snapshot calls freeze_super() to bring
> ? ? ? a fs be in a clean state when a user takes a snapshot.
>
> ? ? freeze ? ? ? ? ? ? ? ? ?truncate ? ? ? ? ? ? ?kjournald
>
> ? ? ? ? ? ? ? ? ? ?| ?ext4_ext_truncate ? ? |
> ? ?freeze_super() ?| ? starts a handle ? ? ?|
> ? ?sets s_frozen ? | ? ? ? ? ? ? ? ? ? ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ?ext4_ext_truncate ? ? |
> ? ? ? ? ? ? ? ? ? ?| ?holds i_data_sem ? ? ?|
> ?ext4_freeze() ? ? | ? ? ? ? ? ? ? ? ? ? ? ?| ? commit_transaction()
> ? wait for updates | ? ? ? ? ? ? ? ? ? ? ? ?| ? waits for i_data_sem
> ? ? ? ? ? ? ? ? ? ?| ?ext4_free_blocks ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ?calls dquot_free_block|
> ? ? ? ? ? ? ? ? ? ?| ? ? ? ? ? ? ? ? ? ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ?dquot_free_block call |
> ? ? ? ? ? ? ? ? ? ?| ?ext4_dirty_inode ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ? ? ? ? ? ? ? ? ? ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ?ext4_dirty_inode ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ?trys to start a handle|
> ? ? ? ? ? ? ? ? ? ?| ? ? ? ? ? ? ? ? ? ? ? ?|
> ? ? ? ? ? ? ? ? ? ?| ?block due to s_frozen |
>
> in ext3, ext3_freeze() prevents journal from being updated by
> lock_journal_updates(), ext3_unfreeze() allow journal to be updated by
> unlock_journal_updates().
>
> in ext4, however, before ext4_freeze() returns, it unlock journal, and
> ext4 prevents journal from being updated by s_frozen. s_frozen is in
> an upper layer, so it is out control of ext4 and deadlock is easy to
> happen.

Virtually, it is not right to block ext4_journal_start_sb() before we
confirm that current thread has no a active handle. But ext4 does like
that. Deadlock is thus easy to happen. Right?

>
> Could someone explain why ext4 does like above but not follow ext3?
>
> Yongqiang.
> --
> Best Wishes
> Yongqiang Yang
>

--
Best Wishes
Yongqiang Yang

2011-03-31 12:02:28

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi, thanks for your reviewing.

(2011/03/30 23:12), Jan Kara wrote:
> Hello,
>
> On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
>> On Thu, 17 Feb 2011 11:45:52 +0100
>> Jan Kara<[email protected]> wrote:
>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
<SNIP>
>> I have deeply continued to examined the root cause of this problem, then
>> I found it.
>>
>> It is that we can write a memory which is mmaped to a file. Then the memory
>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>> "writeback" the memory.
>>
>> Therefore, the root cause of this hangup is not only ext4 component (with
>> delayed allocation feature) but also writeback mechanism for mmap. If you
>> use the other filesystem, you can write something to the filesystem though
>> you have freezed the filesystem.

> Well, you can write something only in the caches, not to the on disk
> image. So it's not a problem as such.
My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
we can write in not only the caches but also the loopback device. However,
I don't still confirm that we can write to the real device(/dev/sdaX).

>
>> A sample problem is attached on this mail. Try to execute it then you can
>> confirm that we can write some data to your filesystem while freezing the
>> filesystem.
>> (If you change FS variable in go.sh from ext3 to ext4 and you execute
>> "fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
>>
>> I think the best approach to fix this problem is to let users not to write
>> memory which is mapped to a certain file while the filesystem is freezing.
>> However, it is very difficult to control users not to write memory which has
>> been already mapped to the file.
> It is actually possible. In case of ext4, you could add a check (+ wait)
> in ext4_page_mkwrite() whether the filesystem is frozen or in the process
> of being frozen and if so, wait for it to get unfrozen. The only tough
> problem here might be the locking as ext4_page_mkwrite() is called with
> mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
> But you'd have to fix all filesystems (and all paths possibly creating
> dirty data) in this way.
>

>> Therefore, I think there is only actual method that we stop writeback thread
>> to resolve the mmap problem. Also, by this fix, the original problem
>> (ext4 delayed write vs unfreeze) can be solved.
> Hmm, I had a look at the code again and think we could fix the issue
> cleanly (i.e. all possible users of s_umount) as follows: The lock
> ordering will be
> s_umount -> "fs frozen"
> and there will be a new mutex s_freeze_mutex protecting changes of
> s_frozen.
>
> freeze_bdev() already observes this lock ordering, it will only take
> s_freeze_mutex for the changes of s_frozen values. The only other code
> that is relevant for the lock ordering is thaw_super() (the freezing
> process is not expected to reenter kernel for the frozen filesystem).
> In thaw_super() we could take s_freeze_mutex, do all the thawing work,
> set s_frozen, release s_freeze_mutex and put superblock reference.
>

> So something like the patch below - it seems to work for me, can you test
> it please?
I think your patch looks good, so, the original problem seems to be solved.
OK, I will test your patch.
This weekend I cannot test it. So, I will reply next week.

Thanks,
Toshiyuki Okajima

2011-03-31 14:04:13

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 3/31/11 3:37 AM, Yongqiang Yang wrote:

> in ext3, ext3_freeze() prevents journal from being updated by
> lock_journal_updates(), ext3_unfreeze() allow journal to be updated by
> unlock_journal_updates().
>
> in ext4, however, before ext4_freeze() returns, it unlock journal, and
> ext4 prevents journal from being updated by s_frozen. s_frozen is in
> an upper layer, so it is out control of ext4 and deadlock is easy to
> happen.
>
> Could someone explain why ext4 does like above but not follow ext3?
>
> Yongqiang.

That was me, I think ...

commit 6b0310fbf087ad6e9e3b8392adca97cd77184084
Author: Eric Sandeen <[email protected]>
Date: Sun May 16 02:00:00 2010 -0400

ext4: don't return to userspace after freezing the fs with a mutex held

ext4_freeze() used jbd2_journal_lock_updates() which takes
the j_barrier mutex, and then returns to userspace. The
kernel does not like this:

================================================
[ BUG: lock held when returning to user space! ]
------------------------------------------------
lvcreate/1075 is leaving the kernel with locks still held!
1 lock held by lvcreate/1075:
#0: (&journal->j_barrier){+.+...}, at: [<ffffffff811c6214>]
jbd2_journal_lock_updates+0xe1/0xf0

Use vfs_check_frozen() added to ext4_journal_start_sb() and
ext4_force_commit() instead.

Addresses-Red-Hat-Bugzilla: #568503

Signed-off-by: Eric Sandeen <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>

2011-03-31 14:36:47

by Yongqiang Yang

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Thu, Mar 31, 2011 at 10:04 PM, Eric Sandeen <[email protected]> wrote:
> On 3/31/11 3:37 AM, Yongqiang Yang wrote:
>
>> in ext3, ext3_freeze() prevents journal from being updated by
>> lock_journal_updates(), ext3_unfreeze() allow journal to be updated by
>> unlock_journal_updates().
>>
>> in ext4, however, before ext4_freeze() returns, it unlock journal, and
>> ext4 prevents journal from being updated by s_frozen. s_frozen is in
>> an upper layer, so it is out control of ext4 and deadlock is easy to
>> happen.
>>
>> Could someone explain why ext4 does like above but not follow ext3?
>>
>> Yongqiang.
>
> That was me, I think ...

Thank you, Eric.

I think ext4_journal_start() should check if current thread has an
active handle before vfs_check_frozen(), if so, current handle will
be returned. Thus, we can avoid deadlocks.

Do you agree with me? If I am right, I will send a patch.
>
> commit 6b0310fbf087ad6e9e3b8392adca97cd77184084
> Author: Eric Sandeen <[email protected]>
> Date: ? Sun May 16 02:00:00 2010 -0400
>
> ? ?ext4: don't return to userspace after freezing the fs with a mutex held
>
> ? ?ext4_freeze() used jbd2_journal_lock_updates() which takes
> ? ?the j_barrier mutex, and then returns to userspace. ?The
> ? ?kernel does not like this:
>
> ? ?================================================
> ? ?[ BUG: lock held when returning to user space! ]
> ? ?------------------------------------------------
> ? ?lvcreate/1075 is leaving the kernel with locks still held!
> ? ?1 lock held by lvcreate/1075:
> ? ? #0: ?(&journal->j_barrier){+.+...}, at: [<ffffffff811c6214>]
> ? ?jbd2_journal_lock_updates+0xe1/0xf0
>
> ? ?Use vfs_check_frozen() added to ext4_journal_start_sb() and
> ? ?ext4_force_commit() instead.
>
> ? ?Addresses-Red-Hat-Bugzilla: #568503
>
>
> ? ?Signed-off-by: Eric Sandeen <[email protected]>
> ? ?Signed-off-by: "Theodore Ts'o" <[email protected]>
>

--
Best Wishes
Yongqiang Yang

2011-03-31 15:25:39

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 3/31/11 9:36 AM, Yongqiang Yang wrote:
> On Thu, Mar 31, 2011 at 10:04 PM, Eric Sandeen <[email protected]> wrote:
>> On 3/31/11 3:37 AM, Yongqiang Yang wrote:
>>
>>> in ext3, ext3_freeze() prevents journal from being updated by
>>> lock_journal_updates(), ext3_unfreeze() allow journal to be updated by
>>> unlock_journal_updates().
>>>
>>> in ext4, however, before ext4_freeze() returns, it unlock journal, and
>>> ext4 prevents journal from being updated by s_frozen. s_frozen is in
>>> an upper layer, so it is out control of ext4 and deadlock is easy to
>>> happen.
>>>
>>> Could someone explain why ext4 does like above but not follow ext3?
>>>
>>> Yongqiang.
>>
>> That was me, I think ...
>
> Thank you, Eric.
>
> I think ext4_journal_start() should check if current thread has an
> active handle before vfs_check_frozen(), if so, current handle will
> be returned. Thus, we can avoid deadlocks.
>
> Do you agree with me? If I am right, I will send a patch.

If you have a testcase to test it with, sure. plus a patch would help me know for sure what you propose :)

Sorry for breaking it (if I did!) But holding a mutex and returning to userspace was pretty bad, too :(

Thanks,
-Eric

>>
>> commit 6b0310fbf087ad6e9e3b8392adca97cd77184084
>> Author: Eric Sandeen <[email protected]>
>> Date: Sun May 16 02:00:00 2010 -0400
>>
>> ext4: don't return to userspace after freezing the fs with a mutex held
>>
>> ext4_freeze() used jbd2_journal_lock_updates() which takes
>> the j_barrier mutex, and then returns to userspace. The
>> kernel does not like this:
>>
>> ================================================
>> [ BUG: lock held when returning to user space! ]
>> ------------------------------------------------
>> lvcreate/1075 is leaving the kernel with locks still held!
>> 1 lock held by lvcreate/1075:
>> #0: (&journal->j_barrier){+.+...}, at: [<ffffffff811c6214>]
>> jbd2_journal_lock_updates+0xe1/0xf0
>>
>> Use vfs_check_frozen() added to ext4_journal_start_sb() and
>> ext4_force_commit() instead.
>>
>> Addresses-Red-Hat-Bugzilla: #568503
>>
>>
>> Signed-off-by: Eric Sandeen <[email protected]>
>> Signed-off-by: "Theodore Ts'o" <[email protected]>
>>
>
>
>

2011-03-31 16:28:43

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Thu 31-03-11 22:36:46, Yongqiang Yang wrote:
> On Thu, Mar 31, 2011 at 10:04 PM, Eric Sandeen <[email protected]> wrote:
> > On 3/31/11 3:37 AM, Yongqiang Yang wrote:
> >
> >> in ext3, ext3_freeze() prevents journal from being updated by
> >> lock_journal_updates(), ext3_unfreeze() allow journal to be updated by
> >> unlock_journal_updates().
> >>
> >> in ext4, however, before ext4_freeze() returns, it unlock journal, and
> >> ext4 prevents journal from being updated by s_frozen. s_frozen is in
> >> an upper layer, so it is out control of ext4 and deadlock is easy to
> >> happen.
> >>
> >> Could someone explain why ext4 does like above but not follow ext3?
> >>
> >> Yongqiang.
> >
> > That was me, I think ...
>
> Thank you, Eric.
>
> I think ext4_journal_start() should check if current thread has an
> active handle before vfs_check_frozen(), if so, current handle will
> be returned. Thus, we can avoid deadlocks.
>
> Do you agree with me? If I am right, I will send a patch.
Yes, definitely. This was exactly what I wanted to propose as well.
Thanks for looking into this.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-03-31 23:40:56

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Mon, Mar 28, 2011 at 05:06:28PM +0900, Toshiyuki Okajima wrote:
> Hi.
>
> On Thu, 17 Feb 2011 11:45:52 +0100
> Jan Kara <[email protected]> wrote:
> > On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> > > (2011/02/16 23:56), Jan Kara wrote:
> > > >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> > > >>On Tue, 15 Feb 2011 18:29:54 +0100
> > > >>Jan Kara<[email protected]> wrote:
> > > >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> > > >>>>>describe above.
> > > >>>>
> > > >>>>One of the fundamental problems here is that the freeze and thaw
> > > >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> > > >>>>first is to prevent the resume/thaw from racing with a umount (which
> > > >>>>it could do just as well by taking a read lock), but the second is to
> > > >>>>prevent the resume/thaw code from racing with itself. That's the core
> > > >>>>fundamental problem here.
> > > >>>>
> > > >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> > > >>>>having the the resume/thaw first take the s_freeze mutex and then
> > > >>>>second take a read lock on the s_umount.
> > > >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> > > >>>in thaw_super() can block if there is another process that tries to acquire
> > > >>>s_umount for writing - a situation like:
> > > >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > > >>>down_read(&sb->s_umount)
> > > >>> block on s_frozen
> > > >>> down_write(&sb->s_umount)
> > > >>> -blocked
> > > >>> down_read(&sb->s_umount)
> > > >>> -blocked
> > > >>>behind the write access...
> > > >>>
> > > >>>The only working solution I see is to check for frozen filesystem before
> > > >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> > > >>>we did so in some well described wrapper).
> > > >>I created the patch that you imagine yesterday.
> > > >>
> > > >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> > > >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> > > >>
> > > >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> > > >>after 12 hours passed.
> > > >>
> > > >>The patch for linux-2.6.38-rc4 is as follows:
> > > >>---
> > > >> fs/fs-writeback.c | 2 +-
> > > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > > >>
> > > >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > > >>index 59c6e49..1c9a05e 100644
> > > >>--- a/fs/fs-writeback.c
> > > >>+++ b/fs/fs-writeback.c
> > > >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> > > >> spin_unlock(&sb_lock);
> > > >>
> > > >> if (down_read_trylock(&sb->s_umount)) {
> > > >>- if (sb->s_root)
> > > >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> > > >> return true;
> > > >> up_read(&sb->s_umount);
> > >
> > > > So this is something along the lines I thought but it actually won't work
> > > >for example if sync(1) is run while the filesystem is frozen (that takes
> > > >s_umount semaphore in a different place). And generally, I'm not convinced
> > > >there are not other places that try to do IO while holding s_umount
> > > >semaphore...
> > > OK. I understand.
> > >
> > > This code only fixes the case for the following path:
> > > writeback_inodes_wb
> > > -> ext4_da_writepages
> > > -> ext4_journal_start_sb
> > > -> vfs_check_frozen
> > > But, the code doesn't fix the other cases.
> > >
> > > We must modify the local filesystem part in order to fix all cases...?
> > Yes, possibly. But most importantly we should first find clear locking
> > rules for frozen filesystem that avoid deadlocks like the one above. And
> > the freezing / unfreezing code might become subtle for that reason, that's
> > fine, but it would be really good to avoid any complicated things for the
> > code in the rest of the VFS / filesystems.
> I have deeply continued to examined the root cause of this problem, then
> I found it.
>
> It is that we can write a memory which is mmaped to a file. Then the memory
> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> "writeback" the memory.

Then surely the issue is that .page_mkwrite is not checking that the
filesystem is frozen before allowing the page fault to continue and
dirty the page?

> I think the best approach to fix this problem is to let users not to write
> memory which is mapped to a certain file while the filesystem is freezing.
> However, it is very difficult to control users not to write memory which has
> been already mapped to the file.

If you don't allow the page to be dirtied in the fist place, then
nothing needs to be done to the writeback path because there is
nothing dirty for it to write back.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2011-03-31 23:53:14

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 3/31/11 6:40 PM, Dave Chinner wrote:
> On Mon, Mar 28, 2011 at 05:06:28PM +0900, Toshiyuki Okajima wrote:
>> Hi.
>>
>> On Thu, 17 Feb 2011 11:45:52 +0100
>> Jan Kara <[email protected]> wrote:
>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
>>>>>>>>> Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
>>>>>>>>> under s_umount semaphore, we are prone to deadlock like the one you
>>>>>>>>> describe above.
>>>>>>>>
>>>>>>>> One of the fundamental problems here is that the freeze and thaw
>>>>>>>> routines are using down_write(&sb->s_umount) for two purposes. The
>>>>>>>> first is to prevent the resume/thaw from racing with a umount (which
>>>>>>>> it could do just as well by taking a read lock), but the second is to
>>>>>>>> prevent the resume/thaw code from racing with itself. That's the core
>>>>>>>> fundamental problem here.
>>>>>>>>
>>>>>>>> So I think we can solve this by introduce a new mutex, s_freeze, and
>>>>>>>> having the the resume/thaw first take the s_freeze mutex and then
>>>>>>>> second take a read lock on the s_umount.
>>>>>>> Sadly this does not quite work because even down_read(&sb->s_umount)
>>>>>>> in thaw_super() can block if there is another process that tries to acquire
>>>>>>> s_umount for writing - a situation like:
>>>>>>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
>>>>>>> down_read(&sb->s_umount)
>>>>>>> block on s_frozen
>>>>>>> down_write(&sb->s_umount)
>>>>>>> -blocked
>>>>>>> down_read(&sb->s_umount)
>>>>>>> -blocked
>>>>>>> behind the write access...
>>>>>>>
>>>>>>> The only working solution I see is to check for frozen filesystem before
>>>>>>> taking s_umount semaphore which seems rather ugly (but might be bearable if
>>>>>>> we did so in some well described wrapper).
>>>>>> I created the patch that you imagine yesterday.
>>>>>>
>>>>>> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
>>>>>> without a fixed patch. After an hour, I confirmed that this deadlock happened.
>>>>>>
>>>>>> However, on the kernel with a fixed patch, this deadlock doesn't still happen
>>>>>> after 12 hours passed.
>>>>>>
>>>>>> The patch for linux-2.6.38-rc4 is as follows:
>>>>>> ---
>>>>>> fs/fs-writeback.c | 2 +-
>>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
>>>>>> index 59c6e49..1c9a05e 100644
>>>>>> --- a/fs/fs-writeback.c
>>>>>> +++ b/fs/fs-writeback.c
>>>>>> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
>>>>>> spin_unlock(&sb_lock);
>>>>>>
>>>>>> if (down_read_trylock(&sb->s_umount)) {
>>>>>> - if (sb->s_root)
>>>>>> + if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
>>>>>> return true;
>>>>>> up_read(&sb->s_umount);
>>>>
>>>>> So this is something along the lines I thought but it actually won't work
>>>>> for example if sync(1) is run while the filesystem is frozen (that takes
>>>>> s_umount semaphore in a different place). And generally, I'm not convinced
>>>>> there are not other places that try to do IO while holding s_umount
>>>>> semaphore...
>>>> OK. I understand.
>>>>
>>>> This code only fixes the case for the following path:
>>>> writeback_inodes_wb
>>>> -> ext4_da_writepages
>>>> -> ext4_journal_start_sb
>>>> -> vfs_check_frozen
>>>> But, the code doesn't fix the other cases.
>>>>
>>>> We must modify the local filesystem part in order to fix all cases...?
>>> Yes, possibly. But most importantly we should first find clear locking
>>> rules for frozen filesystem that avoid deadlocks like the one above. And
>>> the freezing / unfreezing code might become subtle for that reason, that's
>>> fine, but it would be really good to avoid any complicated things for the
>>> code in the rest of the VFS / filesystems.
>> I have deeply continued to examined the root cause of this problem, then
>> I found it.
>>
>> It is that we can write a memory which is mmaped to a file. Then the memory
>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>> "writeback" the memory.
>
> Then surely the issue is that .page_mkwrite is not checking that the
> filesystem is frozen before allowing the page fault to continue and
> dirty the page?
>
>> I think the best approach to fix this problem is to let users not to write
>> memory which is mapped to a certain file while the filesystem is freezing.
>> However, it is very difficult to control users not to write memory which has
>> been already mapped to the file.
>
> If you don't allow the page to be dirtied in the fist place, then
> nothing needs to be done to the writeback path because there is
> nothing dirty for it to write back.

I floated

[PATCH, RFC] check for frozen filesystems in the mmap path

a long time ago, but it went nowhere; maybe time to revive that approach.

-Eric

> Cheers,
>
> Dave.

2011-04-01 14:08:59

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> On Mon, Mar 28, 2011 at 05:06:28PM +0900, Toshiyuki Okajima wrote:
> > On Thu, 17 Feb 2011 11:45:52 +0100
> > Jan Kara <[email protected]> wrote:
> > > On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> > > > (2011/02/16 23:56), Jan Kara wrote:
> > > > >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> > > > >>On Tue, 15 Feb 2011 18:29:54 +0100
> > > > >>Jan Kara<[email protected]> wrote:
> > > > >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > > >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > > >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > > >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> > > > >>>>>describe above.
> > > > >>>>
> > > > >>>>One of the fundamental problems here is that the freeze and thaw
> > > > >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> > > > >>>>first is to prevent the resume/thaw from racing with a umount (which
> > > > >>>>it could do just as well by taking a read lock), but the second is to
> > > > >>>>prevent the resume/thaw code from racing with itself. That's the core
> > > > >>>>fundamental problem here.
> > > > >>>>
> > > > >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> > > > >>>>having the the resume/thaw first take the s_freeze mutex and then
> > > > >>>>second take a read lock on the s_umount.
> > > > >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> > > > >>>in thaw_super() can block if there is another process that tries to acquire
> > > > >>>s_umount for writing - a situation like:
> > > > >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > > > >>>down_read(&sb->s_umount)
> > > > >>> block on s_frozen
> > > > >>> down_write(&sb->s_umount)
> > > > >>> -blocked
> > > > >>> down_read(&sb->s_umount)
> > > > >>> -blocked
> > > > >>>behind the write access...
> > > > >>>
> > > > >>>The only working solution I see is to check for frozen filesystem before
> > > > >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> > > > >>>we did so in some well described wrapper).
> > > > >>I created the patch that you imagine yesterday.
> > > > >>
> > > > >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> > > > >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> > > > >>
> > > > >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> > > > >>after 12 hours passed.
> > > > >>
> > > > >>The patch for linux-2.6.38-rc4 is as follows:
> > > > >>---
> > > > >> fs/fs-writeback.c | 2 +-
> > > > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > > > >>
> > > > >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > > > >>index 59c6e49..1c9a05e 100644
> > > > >>--- a/fs/fs-writeback.c
> > > > >>+++ b/fs/fs-writeback.c
> > > > >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> > > > >> spin_unlock(&sb_lock);
> > > > >>
> > > > >> if (down_read_trylock(&sb->s_umount)) {
> > > > >>- if (sb->s_root)
> > > > >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> > > > >> return true;
> > > > >> up_read(&sb->s_umount);
> > > >
> > > > > So this is something along the lines I thought but it actually won't work
> > > > >for example if sync(1) is run while the filesystem is frozen (that takes
> > > > >s_umount semaphore in a different place). And generally, I'm not convinced
> > > > >there are not other places that try to do IO while holding s_umount
> > > > >semaphore...
> > > > OK. I understand.
> > > >
> > > > This code only fixes the case for the following path:
> > > > writeback_inodes_wb
> > > > -> ext4_da_writepages
> > > > -> ext4_journal_start_sb
> > > > -> vfs_check_frozen
> > > > But, the code doesn't fix the other cases.
> > > >
> > > > We must modify the local filesystem part in order to fix all cases...?
> > > Yes, possibly. But most importantly we should first find clear locking
> > > rules for frozen filesystem that avoid deadlocks like the one above. And
> > > the freezing / unfreezing code might become subtle for that reason, that's
> > > fine, but it would be really good to avoid any complicated things for the
> > > code in the rest of the VFS / filesystems.
> > I have deeply continued to examined the root cause of this problem, then
> > I found it.
> >
> > It is that we can write a memory which is mmaped to a file. Then the memory
> > becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> > "writeback" the memory.
>
> Then surely the issue is that .page_mkwrite is not checking that the
> filesystem is frozen before allowing the page fault to continue and
> dirty the page?
And is this a bug? That isn't clear to me...

> > I think the best approach to fix this problem is to let users not to write
> > memory which is mapped to a certain file while the filesystem is freezing.
> > However, it is very difficult to control users not to write memory which has
> > been already mapped to the file.
>
> If you don't allow the page to be dirtied in the fist place, then
> nothing needs to be done to the writeback path because there is
> nothing dirty for it to write back.
Sure but that's only the problem he was able to hit. But generally,
there's a problem with needing s_umount for unfreezing because it isn't
clear there aren't other code paths which can block with s_umount held
waiting for fs to get unfrozen. And these code paths would cause the same
deadlock. That's why I chose to get rid of s_umount during thawing.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-05 10:24:13

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi.

(2011/03/31 21:03), Toshiyuki Okajima wrote:
> Hi, thanks for your reviewing.
>
> (2011/03/30 23:12), Jan Kara wrote:
>> Hello,
>>
>> On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
>>> On Thu, 17 Feb 2011 11:45:52 +0100
>>> Jan Kara<[email protected]> wrote:
>>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> <SNIP>
>>> I have deeply continued to examined the root cause of this problem, then
>>> I found it.
>>>
>>> It is that we can write a memory which is mmaped to a file. Then the memory
>>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>>> "writeback" the memory.
>>>
>>> Therefore, the root cause of this hangup is not only ext4 component (with
>>> delayed allocation feature) but also writeback mechanism for mmap. If you
>>> use the other filesystem, you can write something to the filesystem though
>>> you have freezed the filesystem.
>
>> Well, you can write something only in the caches, not to the on disk
>> image. So it's not a problem as such.
> My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
> we can write in not only the caches but also the loopback device. However,
> I don't still confirm that we can write to the real device(/dev/sdaX).
>
>>
>>> A sample problem is attached on this mail. Try to execute it then you can
>>> confirm that we can write some data to your filesystem while freezing the
>>> filesystem.
>>> (If you change FS variable in go.sh from ext3 to ext4 and you execute
>>> "fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
>>>
>>> I think the best approach to fix this problem is to let users not to write
>>> memory which is mapped to a certain file while the filesystem is freezing.
>>> However, it is very difficult to control users not to write memory which has
>>> been already mapped to the file.
>> It is actually possible. In case of ext4, you could add a check (+ wait)
>> in ext4_page_mkwrite() whether the filesystem is frozen or in the process
>> of being frozen and if so, wait for it to get unfrozen. The only tough
>> problem here might be the locking as ext4_page_mkwrite() is called with
>> mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
>> But you'd have to fix all filesystems (and all paths possibly creating
>> dirty data) in this way.
>>
>
>>> Therefore, I think there is only actual method that we stop writeback thread
>>> to resolve the mmap problem. Also, by this fix, the original problem
>>> (ext4 delayed write vs unfreeze) can be solved.
>> Hmm, I had a look at the code again and think we could fix the issue
>> cleanly (i.e. all possible users of s_umount) as follows: The lock
>> ordering will be
>> s_umount -> "fs frozen"
>> and there will be a new mutex s_freeze_mutex protecting changes of
>> s_frozen.
>>
>> freeze_bdev() already observes this lock ordering, it will only take
>> s_freeze_mutex for the changes of s_frozen values. The only other code
>> that is relevant for the lock ordering is thaw_super() (the freezing
>> process is not expected to reenter kernel for the frozen filesystem).
>> In thaw_super() we could take s_freeze_mutex, do all the thawing work,
>> set s_frozen, release s_freeze_mutex and put superblock reference.
>>
>
>> So something like the patch below - it seems to work for me, can you test
>> it please?
> I think your patch looks good, so, the original problem seems to be solved.
> OK, I will test your patch.
> This weekend I cannot test it. So, I will reply next week.
I have tested whether Mizuma-san's reproducer can cause to deadlock with your
patch. And then any problems didn't hit while the reproducer was running.

I think your patch solves the original deadlock problem which is reported by
Mizuma-san.

> Reported-by: Toshiyuki Okajima <[email protected]>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
> include/linux/fs.h | 1 +
> 2 files changed, 35 insertions(+), 6 deletions(-)

However, I think a write which causes the deadlock is from mmapped dirty
pages. So, I guess we also need to fix in the mmap path while fsfreezing.

> I floated
>
> [PATCH, RFC] check for frozen filesystems in the mmap path
>
> a long time ago, but it went nowhere; maybe time to revive that approach.

Thanks,
Toshiyuki Okajima

2011-04-05 10:43:19

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi.

(2011/04/01 8:40), Dave Chinner wrote:
> On Mon, Mar 28, 2011 at 05:06:28PM +0900, Toshiyuki Okajima wrote:
>> Hi.
>>
>> On Thu, 17 Feb 2011 11:45:52 +0100
>> Jan Kara<[email protected]> wrote:
>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
>>>>>>>>> Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
>>>>>>>>> under s_umount semaphore, we are prone to deadlock like the one you
>>>>>>>>> describe above.
>>>>>>>>
>>>>>>>> One of the fundamental problems here is that the freeze and thaw
>>>>>>>> routines are using down_write(&sb->s_umount) for two purposes. The
>>>>>>>> first is to prevent the resume/thaw from racing with a umount (which
>>>>>>>> it could do just as well by taking a read lock), but the second is to
>>>>>>>> prevent the resume/thaw code from racing with itself. That's the core
>>>>>>>> fundamental problem here.
>>>>>>>>
>>>>>>>> So I think we can solve this by introduce a new mutex, s_freeze, and
>>>>>>>> having the the resume/thaw first take the s_freeze mutex and then
>>>>>>>> second take a read lock on the s_umount.
>>>>>>> Sadly this does not quite work because even down_read(&sb->s_umount)
>>>>>>> in thaw_super() can block if there is another process that tries to acquire
>>>>>>> s_umount for writing - a situation like:
>>>>>>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
>>>>>>> down_read(&sb->s_umount)
>>>>>>> block on s_frozen
>>>>>>> down_write(&sb->s_umount)
>>>>>>> -blocked
>>>>>>> down_read(&sb->s_umount)
>>>>>>> -blocked
>>>>>>> behind the write access...
>>>>>>>
>>>>>>> The only working solution I see is to check for frozen filesystem before
>>>>>>> taking s_umount semaphore which seems rather ugly (but might be bearable if
>>>>>>> we did so in some well described wrapper).
>>>>>> I created the patch that you imagine yesterday.
>>>>>>
>>>>>> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
>>>>>> without a fixed patch. After an hour, I confirmed that this deadlock happened.
>>>>>>
>>>>>> However, on the kernel with a fixed patch, this deadlock doesn't still happen
>>>>>> after 12 hours passed.
>>>>>>
>>>>>> The patch for linux-2.6.38-rc4 is as follows:
>>>>>> ---
>>>>>> fs/fs-writeback.c | 2 +-
>>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
>>>>>> index 59c6e49..1c9a05e 100644
>>>>>> --- a/fs/fs-writeback.c
>>>>>> +++ b/fs/fs-writeback.c
>>>>>> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
>>>>>> spin_unlock(&sb_lock);
>>>>>>
>>>>>> if (down_read_trylock(&sb->s_umount)) {
>>>>>> - if (sb->s_root)
>>>>>> + if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
>>>>>> return true;
>>>>>> up_read(&sb->s_umount);
>>>>
>>>>> So this is something along the lines I thought but it actually won't work
>>>>> for example if sync(1) is run while the filesystem is frozen (that takes
>>>>> s_umount semaphore in a different place). And generally, I'm not convinced
>>>>> there are not other places that try to do IO while holding s_umount
>>>>> semaphore...
>>>> OK. I understand.
>>>>
>>>> This code only fixes the case for the following path:
>>>> writeback_inodes_wb
>>>> -> ext4_da_writepages
>>>> -> ext4_journal_start_sb
>>>> -> vfs_check_frozen
>>>> But, the code doesn't fix the other cases.
>>>>
>>>> We must modify the local filesystem part in order to fix all cases...?
>>> Yes, possibly. But most importantly we should first find clear locking
>>> rules for frozen filesystem that avoid deadlocks like the one above. And
>>> the freezing / unfreezing code might become subtle for that reason, that's
>>> fine, but it would be really good to avoid any complicated things for the
>>> code in the rest of the VFS / filesystems.
>> I have deeply continued to examined the root cause of this problem, then
>> I found it.
>>
>> It is that we can write a memory which is mmaped to a file. Then the memory
>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>> "writeback" the memory.
>
> Then surely the issue is that .page_mkwrite is not checking that the
> filesystem is frozen before allowing the page fault to continue and
> dirty the page?
>
>> I think the best approach to fix this problem is to let users not to write
>> memory which is mapped to a certain file while the filesystem is freezing.
>> However, it is very difficult to control users not to write memory which has
>> been already mapped to the file.
>

> If you don't allow the page to be dirtied in the fist place, then
> nothing needs to be done to the writeback path because there is
> nothing dirty for it to write back.
OK. We can block the write operation by not allowing the page to be
dirtied in the first place.

But we can not easily stop writing the page which is *already mapped*
in the next place. Therefore I think writing back such pages can
be blocked only in the flusher thread.

Thanks,
Toshiyuki Okajima

2011-04-05 22:54:33

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Tue 05-04-11 19:25:44, Toshiyuki Okajima wrote:
> (2011/03/31 21:03), Toshiyuki Okajima wrote:
> >Hi, thanks for your reviewing.
> >
> >(2011/03/30 23:12), Jan Kara wrote:
> >>Hello,
> >>
> >>On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
> >>>On Thu, 17 Feb 2011 11:45:52 +0100
> >>>Jan Kara<[email protected]> wrote:
> >>>>On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> >>>>>(2011/02/16 23:56), Jan Kara wrote:
> >>>>>>On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> >>>>>>>On Tue, 15 Feb 2011 18:29:54 +0100
> >>>>>>>Jan Kara<[email protected]> wrote:
> >>>>>>>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> >>>>>>>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> ><SNIP>
> >>>I have deeply continued to examined the root cause of this problem, then
> >>>I found it.
> >>>
> >>>It is that we can write a memory which is mmaped to a file. Then the memory
> >>>becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> >>>"writeback" the memory.
> >>>
> >>>Therefore, the root cause of this hangup is not only ext4 component (with
> >>>delayed allocation feature) but also writeback mechanism for mmap. If you
> >>>use the other filesystem, you can write something to the filesystem though
> >>>you have freezed the filesystem.
> >
> >>Well, you can write something only in the caches, not to the on disk
> >>image. So it's not a problem as such.
> >My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
> >we can write in not only the caches but also the loopback device. However,
> >I don't still confirm that we can write to the real device(/dev/sdaX).
> >
> >>
> >>>A sample problem is attached on this mail. Try to execute it then you can
> >>>confirm that we can write some data to your filesystem while freezing the
> >>>filesystem.
> >>>(If you change FS variable in go.sh from ext3 to ext4 and you execute
> >>>"fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
> >>>
> >>>I think the best approach to fix this problem is to let users not to write
> >>>memory which is mapped to a certain file while the filesystem is freezing.
> >>>However, it is very difficult to control users not to write memory which has
> >>>been already mapped to the file.
> >>It is actually possible. In case of ext4, you could add a check (+ wait)
> >>in ext4_page_mkwrite() whether the filesystem is frozen or in the process
> >>of being frozen and if so, wait for it to get unfrozen. The only tough
> >>problem here might be the locking as ext4_page_mkwrite() is called with
> >>mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
> >>But you'd have to fix all filesystems (and all paths possibly creating
> >>dirty data) in this way.
> >>
> >
> >>>Therefore, I think there is only actual method that we stop writeback thread
> >>>to resolve the mmap problem. Also, by this fix, the original problem
> >>>(ext4 delayed write vs unfreeze) can be solved.
> >>Hmm, I had a look at the code again and think we could fix the issue
> >>cleanly (i.e. all possible users of s_umount) as follows: The lock
> >>ordering will be
> >>s_umount -> "fs frozen"
> >>and there will be a new mutex s_freeze_mutex protecting changes of
> >>s_frozen.
> >>
> >>freeze_bdev() already observes this lock ordering, it will only take
> >>s_freeze_mutex for the changes of s_frozen values. The only other code
> >>that is relevant for the lock ordering is thaw_super() (the freezing
> >>process is not expected to reenter kernel for the frozen filesystem).
> >>In thaw_super() we could take s_freeze_mutex, do all the thawing work,
> >>set s_frozen, release s_freeze_mutex and put superblock reference.
> >>
> >
> >>So something like the patch below - it seems to work for me, can you test
> >>it please?
> >I think your patch looks good, so, the original problem seems to be solved.
> >OK, I will test your patch.
> >This weekend I cannot test it. So, I will reply next week.
> I have tested whether Mizuma-san's reproducer can cause to deadlock with your
> patch. And then any problems didn't hit while the reproducer was running.
>
> I think your patch solves the original deadlock problem which is reported by
> Mizuma-san.
Good. Thanks.

> >Reported-by: Toshiyuki Okajima <[email protected]>
> >Signed-off-by: Jan Kara <[email protected]>
> >---
> > fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
> > include/linux/fs.h | 1 +
> > 2 files changed, 35 insertions(+), 6 deletions(-)
>
> However, I think a write which causes the deadlock is from mmapped dirty
> pages. So, I guess we also need to fix in the mmap path while fsfreezing.
Why? If you dirty a page, writeback thread can come and try to write it -
which blocks - but now that does not matter...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-06 05:09:14

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi.

(2011/04/06 7:54), Jan Kara wrote:
> On Tue 05-04-11 19:25:44, Toshiyuki Okajima wrote:
>> (2011/03/31 21:03), Toshiyuki Okajima wrote:
>>> Hi, thanks for your reviewing.
>>>
>>> (2011/03/30 23:12), Jan Kara wrote:
>>>> Hello,
>>>>
>>>> On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
>>>>> On Thu, 17 Feb 2011 11:45:52 +0100
>>>>> Jan Kara<[email protected]> wrote:
>>>>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
>>> <SNIP>
>>>>> I have deeply continued to examined the root cause of this problem, then
>>>>> I found it.
>>>>>
>>>>> It is that we can write a memory which is mmaped to a file. Then the memory
>>>>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>>>>> "writeback" the memory.
>>>>>
>>>>> Therefore, the root cause of this hangup is not only ext4 component (with
>>>>> delayed allocation feature) but also writeback mechanism for mmap. If you
>>>>> use the other filesystem, you can write something to the filesystem though
>>>>> you have freezed the filesystem.
>>>
>>>> Well, you can write something only in the caches, not to the on disk
>>>> image. So it's not a problem as such.
>>> My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
>>> we can write in not only the caches but also the loopback device. However,
>>> I don't still confirm that we can write to the real device(/dev/sdaX).
>>>
>>>>
>>>>> A sample problem is attached on this mail. Try to execute it then you can
>>>>> confirm that we can write some data to your filesystem while freezing the
>>>>> filesystem.
>>>>> (If you change FS variable in go.sh from ext3 to ext4 and you execute
>>>>> "fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
>>>>>
>>>>> I think the best approach to fix this problem is to let users not to write
>>>>> memory which is mapped to a certain file while the filesystem is freezing.
>>>>> However, it is very difficult to control users not to write memory which has
>>>>> been already mapped to the file.
>>>> It is actually possible. In case of ext4, you could add a check (+ wait)
>>>> in ext4_page_mkwrite() whether the filesystem is frozen or in the process
>>>> of being frozen and if so, wait for it to get unfrozen. The only tough
>>>> problem here might be the locking as ext4_page_mkwrite() is called with
>>>> mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
>>>> But you'd have to fix all filesystems (and all paths possibly creating
>>>> dirty data) in this way.
>>>>
>>>
>>>>> Therefore, I think there is only actual method that we stop writeback thread
>>>>> to resolve the mmap problem. Also, by this fix, the original problem
>>>>> (ext4 delayed write vs unfreeze) can be solved.
>>>> Hmm, I had a look at the code again and think we could fix the issue
>>>> cleanly (i.e. all possible users of s_umount) as follows: The lock
>>>> ordering will be
>>>> s_umount -> "fs frozen"
>>>> and there will be a new mutex s_freeze_mutex protecting changes of
>>>> s_frozen.
>>>>
>>>> freeze_bdev() already observes this lock ordering, it will only take
>>>> s_freeze_mutex for the changes of s_frozen values. The only other code
>>>> that is relevant for the lock ordering is thaw_super() (the freezing
>>>> process is not expected to reenter kernel for the frozen filesystem).
>>>> In thaw_super() we could take s_freeze_mutex, do all the thawing work,
>>>> set s_frozen, release s_freeze_mutex and put superblock reference.
>>>>
>>>
>>>> So something like the patch below - it seems to work for me, can you test
>>>> it please?
>>> I think your patch looks good, so, the original problem seems to be solved.
>>> OK, I will test your patch.
>>> This weekend I cannot test it. So, I will reply next week.
>> I have tested whether Mizuma-san's reproducer can cause to deadlock with your
>> patch. And then any problems didn't hit while the reproducer was running.
>>
>> I think your patch solves the original deadlock problem which is reported by
>> Mizuma-san.
> Good. Thanks.
>
>>> Reported-by: Toshiyuki Okajima<[email protected]>
>>> Signed-off-by: Jan Kara<[email protected]>
>>> ---
>>> fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
>>> include/linux/fs.h | 1 +
>>> 2 files changed, 35 insertions(+), 6 deletions(-)
>>

>> However, I think a write which causes the deadlock is from mmapped dirty
>> pages. So, I guess we also need to fix in the mmap path while fsfreezing.
> Why? If you dirty a page, writeback thread can come and try to write it -
> which blocks - but now that does not matter...
I have not understood the code around writeback thread very much...
Please explain me the concrete function name which blocks some writes?

Mizuma-san's reproducer also writes the data which maps to the file (mmap).
The original problem happens after the fsfreeze operation is done.
I understand the normal write operation (not mmap) can be blocked while
fsfreezing. So, I guess we don't always block all the write operation
while fsfreezing.

Thanks
Toshiyuki Okajima

2011-04-06 05:40:05

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Fri, Apr 01, 2011 at 04:08:56PM +0200, Jan Kara wrote:
> On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> > On Mon, Mar 28, 2011 at 05:06:28PM +0900, Toshiyuki Okajima wrote:
> > > On Thu, 17 Feb 2011 11:45:52 +0100
> > > Jan Kara <[email protected]> wrote:
> > > > On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> > > > > (2011/02/16 23:56), Jan Kara wrote:
> > > > > >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> > > > > >>On Tue, 15 Feb 2011 18:29:54 +0100
> > > > > >>Jan Kara<[email protected]> wrote:
> > > > > >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > > > >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > > > >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > > > >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> > > > > >>>>>describe above.
> > > > > >>>>
> > > > > >>>>One of the fundamental problems here is that the freeze and thaw
> > > > > >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> > > > > >>>>first is to prevent the resume/thaw from racing with a umount (which
> > > > > >>>>it could do just as well by taking a read lock), but the second is to
> > > > > >>>>prevent the resume/thaw code from racing with itself. That's the core
> > > > > >>>>fundamental problem here.
> > > > > >>>>
> > > > > >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> > > > > >>>>having the the resume/thaw first take the s_freeze mutex and then
> > > > > >>>>second take a read lock on the s_umount.
> > > > > >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> > > > > >>>in thaw_super() can block if there is another process that tries to acquire
> > > > > >>>s_umount for writing - a situation like:
> > > > > >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > > > > >>>down_read(&sb->s_umount)
> > > > > >>> block on s_frozen
> > > > > >>> down_write(&sb->s_umount)
> > > > > >>> -blocked
> > > > > >>> down_read(&sb->s_umount)
> > > > > >>> -blocked
> > > > > >>>behind the write access...
> > > > > >>>
> > > > > >>>The only working solution I see is to check for frozen filesystem before
> > > > > >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> > > > > >>>we did so in some well described wrapper).
> > > > > >>I created the patch that you imagine yesterday.
> > > > > >>
> > > > > >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> > > > > >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> > > > > >>
> > > > > >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> > > > > >>after 12 hours passed.
> > > > > >>
> > > > > >>The patch for linux-2.6.38-rc4 is as follows:
> > > > > >>---
> > > > > >> fs/fs-writeback.c | 2 +-
> > > > > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > > > > >>
> > > > > >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > > > > >>index 59c6e49..1c9a05e 100644
> > > > > >>--- a/fs/fs-writeback.c
> > > > > >>+++ b/fs/fs-writeback.c
> > > > > >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> > > > > >> spin_unlock(&sb_lock);
> > > > > >>
> > > > > >> if (down_read_trylock(&sb->s_umount)) {
> > > > > >>- if (sb->s_root)
> > > > > >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> > > > > >> return true;
> > > > > >> up_read(&sb->s_umount);
> > > > >
> > > > > > So this is something along the lines I thought but it actually won't work
> > > > > >for example if sync(1) is run while the filesystem is frozen (that takes
> > > > > >s_umount semaphore in a different place). And generally, I'm not convinced
> > > > > >there are not other places that try to do IO while holding s_umount
> > > > > >semaphore...
> > > > > OK. I understand.
> > > > >
> > > > > This code only fixes the case for the following path:
> > > > > writeback_inodes_wb
> > > > > -> ext4_da_writepages
> > > > > -> ext4_journal_start_sb
> > > > > -> vfs_check_frozen
> > > > > But, the code doesn't fix the other cases.
> > > > >
> > > > > We must modify the local filesystem part in order to fix all cases...?
> > > > Yes, possibly. But most importantly we should first find clear locking
> > > > rules for frozen filesystem that avoid deadlocks like the one above. And
> > > > the freezing / unfreezing code might become subtle for that reason, that's
> > > > fine, but it would be really good to avoid any complicated things for the
> > > > code in the rest of the VFS / filesystems.
> > > I have deeply continued to examined the root cause of this problem, then
> > > I found it.
> > >
> > > It is that we can write a memory which is mmaped to a file. Then the memory
> > > becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> > > "writeback" the memory.
> >
> > Then surely the issue is that .page_mkwrite is not checking that the
> > filesystem is frozen before allowing the page fault to continue and
> > dirty the page?
> And is this a bug? That isn't clear to me...

Given the semantics of a frozen filesystem, letting any object be
dirtied while frozen (be it an inode, a page, a metadata block, etc)
is definitely a bug.

The way the freeze code is architected is that incoming dirtying
events are prevented so that the writeback side does not need to
care about the frozen state of the filesystem at all. The freeze
operation is supposed to block new dirtiers, then flush all dirty
objects resulting in everything being clean in the filesystem.

Hence if no objects are being dirtied, then there should never be
any need to block writeback threads due to the filesytem being
frozen because, by definition, there should be no work for them to
do. Hence if objects are being dirtied while the filesystem is
frozen, then that is a bug.

> > > I think the best approach to fix this problem is to let users not to write
> > > memory which is mapped to a certain file while the filesystem is freezing.
> > > However, it is very difficult to control users not to write memory which has
> > > been already mapped to the file.
> >
> > If you don't allow the page to be dirtied in the fist place, then
> > nothing needs to be done to the writeback path because there is
> > nothing dirty for it to write back.
> Sure but that's only the problem he was able to hit. But generally,
> there's a problem with needing s_umount for unfreezing because it isn't
> clear there aren't other code paths which can block with s_umount held
> waiting for fs to get unfrozen. And these code paths would cause the same
> deadlock. That's why I chose to get rid of s_umount during thawing.

Holding the s_umount lock while checking if frozen and sleeping
is essentially an ABBA lock inversion bug that can bite in many more
places that just thawing the filesystem. . Any where this is done
should be fixed, so I don't think just removing the s_umount lock
from the thaw path is sufficient to avoid problems.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2011-04-06 05:57:13

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed 06-04-11 14:09:14, Toshiyuki Okajima wrote:
> (2011/04/06 7:54), Jan Kara wrote:
> >On Tue 05-04-11 19:25:44, Toshiyuki Okajima wrote:
> >>(2011/03/31 21:03), Toshiyuki Okajima wrote:
> >>>Hi, thanks for your reviewing.
> >>>
> >>>(2011/03/30 23:12), Jan Kara wrote:
> >>>>Hello,
> >>>>
> >>>>On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
> >>>>>On Thu, 17 Feb 2011 11:45:52 +0100
> >>>>>Jan Kara<[email protected]> wrote:
> >>>>>>On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> >>>>>>>(2011/02/16 23:56), Jan Kara wrote:
> >>>>>>>>On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> >>>>>>>>>On Tue, 15 Feb 2011 18:29:54 +0100
> >>>>>>>>>Jan Kara<[email protected]> wrote:
> >>>>>>>>>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> >>>>>>>>>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> >>><SNIP>
> >>>>>I have deeply continued to examined the root cause of this problem, then
> >>>>>I found it.
> >>>>>
> >>>>>It is that we can write a memory which is mmaped to a file. Then the memory
> >>>>>becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> >>>>>"writeback" the memory.
> >>>>>
> >>>>>Therefore, the root cause of this hangup is not only ext4 component (with
> >>>>>delayed allocation feature) but also writeback mechanism for mmap. If you
> >>>>>use the other filesystem, you can write something to the filesystem though
> >>>>>you have freezed the filesystem.
> >>>
> >>>>Well, you can write something only in the caches, not to the on disk
> >>>>image. So it's not a problem as such.
> >>>My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
> >>>we can write in not only the caches but also the loopback device. However,
> >>>I don't still confirm that we can write to the real device(/dev/sdaX).
> >>>
> >>>>
> >>>>>A sample problem is attached on this mail. Try to execute it then you can
> >>>>>confirm that we can write some data to your filesystem while freezing the
> >>>>>filesystem.
> >>>>>(If you change FS variable in go.sh from ext3 to ext4 and you execute
> >>>>>"fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
> >>>>>
> >>>>>I think the best approach to fix this problem is to let users not to write
> >>>>>memory which is mapped to a certain file while the filesystem is freezing.
> >>>>>However, it is very difficult to control users not to write memory which has
> >>>>>been already mapped to the file.
> >>>>It is actually possible. In case of ext4, you could add a check (+ wait)
> >>>>in ext4_page_mkwrite() whether the filesystem is frozen or in the process
> >>>>of being frozen and if so, wait for it to get unfrozen. The only tough
> >>>>problem here might be the locking as ext4_page_mkwrite() is called with
> >>>>mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
> >>>>But you'd have to fix all filesystems (and all paths possibly creating
> >>>>dirty data) in this way.
> >>>>
> >>>
> >>>>>Therefore, I think there is only actual method that we stop writeback thread
> >>>>>to resolve the mmap problem. Also, by this fix, the original problem
> >>>>>(ext4 delayed write vs unfreeze) can be solved.
> >>>>Hmm, I had a look at the code again and think we could fix the issue
> >>>>cleanly (i.e. all possible users of s_umount) as follows: The lock
> >>>>ordering will be
> >>>>s_umount -> "fs frozen"
> >>>>and there will be a new mutex s_freeze_mutex protecting changes of
> >>>>s_frozen.
> >>>>
> >>>>freeze_bdev() already observes this lock ordering, it will only take
> >>>>s_freeze_mutex for the changes of s_frozen values. The only other code
> >>>>that is relevant for the lock ordering is thaw_super() (the freezing
> >>>>process is not expected to reenter kernel for the frozen filesystem).
> >>>>In thaw_super() we could take s_freeze_mutex, do all the thawing work,
> >>>>set s_frozen, release s_freeze_mutex and put superblock reference.
> >>>>
> >>>
> >>>>So something like the patch below - it seems to work for me, can you test
> >>>>it please?
> >>>I think your patch looks good, so, the original problem seems to be solved.
> >>>OK, I will test your patch.
> >>>This weekend I cannot test it. So, I will reply next week.
> >>I have tested whether Mizuma-san's reproducer can cause to deadlock with your
> >>patch. And then any problems didn't hit while the reproducer was running.
> >>
> >>I think your patch solves the original deadlock problem which is reported by
> >>Mizuma-san.
> > Good. Thanks.
> >
> >>>Reported-by: Toshiyuki Okajima<[email protected]>
> >>>Signed-off-by: Jan Kara<[email protected]>
> >>>---
> >>>fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
> >>>include/linux/fs.h | 1 +
> >>>2 files changed, 35 insertions(+), 6 deletions(-)
> >>
>
> >>However, I think a write which causes the deadlock is from mmapped dirty
> >>pages. So, I guess we also need to fix in the mmap path while fsfreezing.
> > Why? If you dirty a page, writeback thread can come and try to write it -
> >which blocks - but now that does not matter...
> I have not understood the code around writeback thread very much...
> Please explain me the concrete function name which blocks some writes?
It would block in ext4_da_writepages() function.

> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
> The original problem happens after the fsfreeze operation is done.
> I understand the normal write operation (not mmap) can be blocked while
> fsfreezing. So, I guess we don't always block all the write operation
> while fsfreezing.
Technically speaking, we block all the transaction starts which means we
end up blocking all the writes from going to disk. But that does not mean
we block all the writes from going to in-memory cache - as you properly
note the mmap case is one of such exceptions.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-06 06:18:56

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed 06-04-11 15:40:05, Dave Chinner wrote:
> On Fri, Apr 01, 2011 at 04:08:56PM +0200, Jan Kara wrote:
> > On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> > > On Mon, Mar 28, 2011 at 05:06:28PM +0900, Toshiyuki Okajima wrote:
> > > > On Thu, 17 Feb 2011 11:45:52 +0100
> > > > Jan Kara <[email protected]> wrote:
> > > > > On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> > > > > > (2011/02/16 23:56), Jan Kara wrote:
> > > > > > >On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> > > > > > >>On Tue, 15 Feb 2011 18:29:54 +0100
> > > > > > >>Jan Kara<[email protected]> wrote:
> > > > > > >>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > > > > >>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > > > > >>>>>Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > > > > >>>>>under s_umount semaphore, we are prone to deadlock like the one you
> > > > > > >>>>>describe above.
> > > > > > >>>>
> > > > > > >>>>One of the fundamental problems here is that the freeze and thaw
> > > > > > >>>>routines are using down_write(&sb->s_umount) for two purposes. The
> > > > > > >>>>first is to prevent the resume/thaw from racing with a umount (which
> > > > > > >>>>it could do just as well by taking a read lock), but the second is to
> > > > > > >>>>prevent the resume/thaw code from racing with itself. That's the core
> > > > > > >>>>fundamental problem here.
> > > > > > >>>>
> > > > > > >>>>So I think we can solve this by introduce a new mutex, s_freeze, and
> > > > > > >>>>having the the resume/thaw first take the s_freeze mutex and then
> > > > > > >>>>second take a read lock on the s_umount.
> > > > > > >>> Sadly this does not quite work because even down_read(&sb->s_umount)
> > > > > > >>>in thaw_super() can block if there is another process that tries to acquire
> > > > > > >>>s_umount for writing - a situation like:
> > > > > > >>> TASK 1 (e.g. flusher) TASK 2 (e.g. remount) TASK 3 (unfreeze)
> > > > > > >>>down_read(&sb->s_umount)
> > > > > > >>> block on s_frozen
> > > > > > >>> down_write(&sb->s_umount)
> > > > > > >>> -blocked
> > > > > > >>> down_read(&sb->s_umount)
> > > > > > >>> -blocked
> > > > > > >>>behind the write access...
> > > > > > >>>
> > > > > > >>>The only working solution I see is to check for frozen filesystem before
> > > > > > >>>taking s_umount semaphore which seems rather ugly (but might be bearable if
> > > > > > >>>we did so in some well described wrapper).
> > > > > > >>I created the patch that you imagine yesterday.
> > > > > > >>
> > > > > > >>I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> > > > > > >>without a fixed patch. After an hour, I confirmed that this deadlock happened.
> > > > > > >>
> > > > > > >>However, on the kernel with a fixed patch, this deadlock doesn't still happen
> > > > > > >>after 12 hours passed.
> > > > > > >>
> > > > > > >>The patch for linux-2.6.38-rc4 is as follows:
> > > > > > >>---
> > > > > > >> fs/fs-writeback.c | 2 +-
> > > > > > >> 1 files changed, 1 insertions(+), 1 deletions(-)
> > > > > > >>
> > > > > > >>diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> > > > > > >>index 59c6e49..1c9a05e 100644
> > > > > > >>--- a/fs/fs-writeback.c
> > > > > > >>+++ b/fs/fs-writeback.c
> > > > > > >>@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
> > > > > > >> spin_unlock(&sb_lock);
> > > > > > >>
> > > > > > >> if (down_read_trylock(&sb->s_umount)) {
> > > > > > >>- if (sb->s_root)
> > > > > > >>+ if (sb->s_frozen == SB_UNFROZEN&& sb->s_root)
> > > > > > >> return true;
> > > > > > >> up_read(&sb->s_umount);
> > > > > >
> > > > > > > So this is something along the lines I thought but it actually won't work
> > > > > > >for example if sync(1) is run while the filesystem is frozen (that takes
> > > > > > >s_umount semaphore in a different place). And generally, I'm not convinced
> > > > > > >there are not other places that try to do IO while holding s_umount
> > > > > > >semaphore...
> > > > > > OK. I understand.
> > > > > >
> > > > > > This code only fixes the case for the following path:
> > > > > > writeback_inodes_wb
> > > > > > -> ext4_da_writepages
> > > > > > -> ext4_journal_start_sb
> > > > > > -> vfs_check_frozen
> > > > > > But, the code doesn't fix the other cases.
> > > > > >
> > > > > > We must modify the local filesystem part in order to fix all cases...?
> > > > > Yes, possibly. But most importantly we should first find clear locking
> > > > > rules for frozen filesystem that avoid deadlocks like the one above. And
> > > > > the freezing / unfreezing code might become subtle for that reason, that's
> > > > > fine, but it would be really good to avoid any complicated things for the
> > > > > code in the rest of the VFS / filesystems.
> > > > I have deeply continued to examined the root cause of this problem, then
> > > > I found it.
> > > >
> > > > It is that we can write a memory which is mmaped to a file. Then the memory
> > > > becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> > > > "writeback" the memory.
> > >
> > > Then surely the issue is that .page_mkwrite is not checking that the
> > > filesystem is frozen before allowing the page fault to continue and
> > > dirty the page?
> > And is this a bug? That isn't clear to me...
>
> Given the semantics of a frozen filesystem, letting any object be
> dirtied while frozen (be it an inode, a page, a metadata block, etc)
> is definitely a bug.
>
> The way the freeze code is architected is that incoming dirtying
> events are prevented so that the writeback side does not need to
> care about the frozen state of the filesystem at all. The freeze
> operation is supposed to block new dirtiers, then flush all dirty
> objects resulting in everything being clean in the filesystem.
>
> Hence if no objects are being dirtied, then there should never be
> any need to block writeback threads due to the filesytem being
> frozen because, by definition, there should be no work for them to
> do. Hence if objects are being dirtied while the filesystem is
> frozen, then that is a bug.
OK, after some thought I start to agree with you that it would be nice
if we didn't allow the pages to be dirtied at the first place. Otherwise
things get a bit fragile as writing a data block does *not* need a
transaction start as such (we just happen to do it in all code paths)...

> > > > I think the best approach to fix this problem is to let users not to write
> > > > memory which is mapped to a certain file while the filesystem is freezing.
> > > > However, it is very difficult to control users not to write memory which has
> > > > been already mapped to the file.
> > >
> > > If you don't allow the page to be dirtied in the fist place, then
> > > nothing needs to be done to the writeback path because there is
> > > nothing dirty for it to write back.
> > Sure but that's only the problem he was able to hit. But generally,
> > there's a problem with needing s_umount for unfreezing because it isn't
> > clear there aren't other code paths which can block with s_umount held
> > waiting for fs to get unfrozen. And these code paths would cause the same
> > deadlock. That's why I chose to get rid of s_umount during thawing.
>
> Holding the s_umount lock while checking if frozen and sleeping
> is essentially an ABBA lock inversion bug that can bite in many more
> places that just thawing the filesystem. Any where this is done should
> be fixed, so I don't think just removing the s_umount lock from the thaw
> path is sufficient to avoid problems.
That's easily said but hard to do - any transaction start in ext3/4 may
block on filesystem being frozen (this seems to be similar for XFS as I'm
looking into the code) and transaction start traditionally nests inside
s_umount (and basically there's no way around that since sync() calls your
fs code with s_umount held). So I'm afraid we are not going to get rid of
this ABBA dependency unless we declare that s_umount ranks above filesystem
being frozen - but surely I'm open to suggestions.

Another possibility is just to hide the problem e.g. by checking for frozen
filesystem whenever we try to get s_umount. But that looks a bit ugly to
me.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-06 07:38:45

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi.

(2011/04/06 14:57), Jan Kara wrote:
> On Wed 06-04-11 14:09:14, Toshiyuki Okajima wrote:
>> (2011/04/06 7:54), Jan Kara wrote:
>>> On Tue 05-04-11 19:25:44, Toshiyuki Okajima wrote:
>>>> (2011/03/31 21:03), Toshiyuki Okajima wrote:
>>>>> Hi, thanks for your reviewing.
>>>>>
>>>>> (2011/03/30 23:12), Jan Kara wrote:
>>>>>> Hello,
>>>>>>
>>>>>> On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
>>>>>>> On Thu, 17 Feb 2011 11:45:52 +0100
>>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>>>>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>>>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
>>>>> <SNIP>
>>>>>>> I have deeply continued to examined the root cause of this problem, then
>>>>>>> I found it.
>>>>>>>
>>>>>>> It is that we can write a memory which is mmaped to a file. Then the memory
>>>>>>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>>>>>>> "writeback" the memory.
>>>>>>>
>>>>>>> Therefore, the root cause of this hangup is not only ext4 component (with
>>>>>>> delayed allocation feature) but also writeback mechanism for mmap. If you
>>>>>>> use the other filesystem, you can write something to the filesystem though
>>>>>>> you have freezed the filesystem.
>>>>>
>>>>>> Well, you can write something only in the caches, not to the on disk
>>>>>> image. So it's not a problem as such.
>>>>> My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
>>>>> we can write in not only the caches but also the loopback device. However,
>>>>> I don't still confirm that we can write to the real device(/dev/sdaX).
>>>>>
>>>>>>
>>>>>>> A sample problem is attached on this mail. Try to execute it then you can
>>>>>>> confirm that we can write some data to your filesystem while freezing the
>>>>>>> filesystem.
>>>>>>> (If you change FS variable in go.sh from ext3 to ext4 and you execute
>>>>>>> "fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
>>>>>>>
>>>>>>> I think the best approach to fix this problem is to let users not to write
>>>>>>> memory which is mapped to a certain file while the filesystem is freezing.
>>>>>>> However, it is very difficult to control users not to write memory which has
>>>>>>> been already mapped to the file.
>>>>>> It is actually possible. In case of ext4, you could add a check (+ wait)
>>>>>> in ext4_page_mkwrite() whether the filesystem is frozen or in the process
>>>>>> of being frozen and if so, wait for it to get unfrozen. The only tough
>>>>>> problem here might be the locking as ext4_page_mkwrite() is called with
>>>>>> mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
>>>>>> But you'd have to fix all filesystems (and all paths possibly creating
>>>>>> dirty data) in this way.
>>>>>>
>>>>>
>>>>>>> Therefore, I think there is only actual method that we stop writeback thread
>>>>>>> to resolve the mmap problem. Also, by this fix, the original problem
>>>>>>> (ext4 delayed write vs unfreeze) can be solved.
>>>>>> Hmm, I had a look at the code again and think we could fix the issue
>>>>>> cleanly (i.e. all possible users of s_umount) as follows: The lock
>>>>>> ordering will be
>>>>>> s_umount -> "fs frozen"
>>>>>> and there will be a new mutex s_freeze_mutex protecting changes of
>>>>>> s_frozen.
>>>>>>
>>>>>> freeze_bdev() already observes this lock ordering, it will only take
>>>>>> s_freeze_mutex for the changes of s_frozen values. The only other code
>>>>>> that is relevant for the lock ordering is thaw_super() (the freezing
>>>>>> process is not expected to reenter kernel for the frozen filesystem).
>>>>>> In thaw_super() we could take s_freeze_mutex, do all the thawing work,
>>>>>> set s_frozen, release s_freeze_mutex and put superblock reference.
>>>>>>
>>>>>
>>>>>> So something like the patch below - it seems to work for me, can you test
>>>>>> it please?
>>>>> I think your patch looks good, so, the original problem seems to be solved.
>>>>> OK, I will test your patch.
>>>>> This weekend I cannot test it. So, I will reply next week.
>>>> I have tested whether Mizuma-san's reproducer can cause to deadlock with your
>>>> patch. And then any problems didn't hit while the reproducer was running.
>>>>
>>>> I think your patch solves the original deadlock problem which is reported by
>>>> Mizuma-san.
>>> Good. Thanks.
>>>
>>>>> Reported-by: Toshiyuki Okajima<[email protected]>
>>>>> Signed-off-by: Jan Kara<[email protected]>
>>>>> ---
>>>>> fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
>>>>> include/linux/fs.h | 1 +
>>>>> 2 files changed, 35 insertions(+), 6 deletions(-)
>>>>
>>
>>>> However, I think a write which causes the deadlock is from mmapped dirty
>>>> pages. So, I guess we also need to fix in the mmap path while fsfreezing.
>>> Why? If you dirty a page, writeback thread can come and try to write it -
>>> which blocks - but now that does not matter...

>> I have not understood the code around writeback thread very much...
>> Please explain me the concrete function name which blocks some writes?
> It would block in ext4_da_writepages() function.
In ext4 with delayed allocation case, I understand it blocks.
(Original deadlock problem is just this case.)
But in ext4 without delayed allocation or other filesystems case, which function
can block writing?

>
>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
>> The original problem happens after the fsfreeze operation is done.
>> I understand the normal write operation (not mmap) can be blocked while
>> fsfreezing. So, I guess we don't always block all the write operation
>> while fsfreezing.
> Technically speaking, we block all the transaction starts which means we
> end up blocking all the writes from going to disk. But that does not mean
> we block all the writes from going to in-memory cache - as you properly
> note the mmap case is one of such exceptions.
Hm, I also think we can allow the writes to in-memory cache but we can't allow
the writes to disk while fsfreezing. I am considering that mmap path can
write to disk while fsfreezing because this deadlock problem happens after
fsfreeze operation is done...

Thanks,
Toshiyuki Okajima

2011-04-06 11:21:41

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed, Apr 06, 2011 at 08:18:56AM +0200, Jan Kara wrote:
> On Wed 06-04-11 15:40:05, Dave Chinner wrote:
> > On Fri, Apr 01, 2011 at 04:08:56PM +0200, Jan Kara wrote:
> > > On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> > > > If you don't allow the page to be dirtied in the fist place, then
> > > > nothing needs to be done to the writeback path because there is
> > > > nothing dirty for it to write back.
> > > Sure but that's only the problem he was able to hit. But generally,
> > > there's a problem with needing s_umount for unfreezing because it isn't
> > > clear there aren't other code paths which can block with s_umount held
> > > waiting for fs to get unfrozen. And these code paths would cause the same
> > > deadlock. That's why I chose to get rid of s_umount during thawing.
> >
> > Holding the s_umount lock while checking if frozen and sleeping
> > is essentially an ABBA lock inversion bug that can bite in many more
> > places that just thawing the filesystem. Any where this is done should
> > be fixed, so I don't think just removing the s_umount lock from the thaw
> > path is sufficient to avoid problems.
> That's easily said but hard to do - any transaction start in ext3/4 may
> block on filesystem being frozen (this seems to be similar for XFS as I'm
> looking into the code) and transaction start traditionally nests inside
> s_umount (and basically there's no way around that since sync() calls your
> fs code with s_umount held).

Sure, but the question must be asked - why is ext3/4 even starting a
transaction on a clean filesystem during sync? A frozen filesystem,
by definition, is a clean filesytem, and therefore sync calls of any
kind should not be trying to write to the FS or start transactions.
XFS does this just fine, so I'd consider such behaviour on a frozen
filesystem a bug in ext3/4...

> So I'm afraid we are not going to get rid of
> this ABBA dependency unless we declare that s_umount ranks above filesystem
> being frozen - but surely I'm open to suggestions.

Not sure I understand what you are saying there - this is already
the case, isn't it? i.e. it has to be held exclusive to freeze a
filesystem...

> Another possibility is just to hide the problem e.g. by checking for frozen
> filesystem whenever we try to get s_umount. But that looks a bit ugly to
> me.

And not necessary, AFAICT.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2011-04-06 13:44:28

by Christoph Hellwig

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed, Apr 06, 2011 at 09:21:35PM +1000, Dave Chinner wrote:
> Sure, but the question must be asked - why is ext3/4 even starting a
> transaction on a clean filesystem during sync? A frozen filesystem,
> by definition, is a clean filesytem, and therefore sync calls of any
> kind should not be trying to write to the FS or start transactions.
> XFS does this just fine, so I'd consider such behaviour on a frozen
> filesystem a bug in ext3/4...

XFS does have one special case for this. When writing the dummy log
record at the end of the freeze process we use _xfs_alloc_trans to
bypass the frozen filesystem check as we have to write out this record
when the filesystem already is frozen. But that's after the main
sync with its normal transactions.

2011-04-06 17:40:01

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed 06-04-11 21:21:35, Dave Chinner wrote:
> On Wed, Apr 06, 2011 at 08:18:56AM +0200, Jan Kara wrote:
> > On Wed 06-04-11 15:40:05, Dave Chinner wrote:
> > > On Fri, Apr 01, 2011 at 04:08:56PM +0200, Jan Kara wrote:
> > > > On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> > > > > If you don't allow the page to be dirtied in the fist place, then
> > > > > nothing needs to be done to the writeback path because there is
> > > > > nothing dirty for it to write back.
> > > > Sure but that's only the problem he was able to hit. But generally,
> > > > there's a problem with needing s_umount for unfreezing because it isn't
> > > > clear there aren't other code paths which can block with s_umount held
> > > > waiting for fs to get unfrozen. And these code paths would cause the same
> > > > deadlock. That's why I chose to get rid of s_umount during thawing.
> > >
> > > Holding the s_umount lock while checking if frozen and sleeping
> > > is essentially an ABBA lock inversion bug that can bite in many more
> > > places that just thawing the filesystem. Any where this is done should
> > > be fixed, so I don't think just removing the s_umount lock from the thaw
> > > path is sufficient to avoid problems.
> > That's easily said but hard to do - any transaction start in ext3/4 may
> > block on filesystem being frozen (this seems to be similar for XFS as I'm
> > looking into the code) and transaction start traditionally nests inside
> > s_umount (and basically there's no way around that since sync() calls your
> > fs code with s_umount held).
>
> Sure, but the question must be asked - why is ext3/4 even starting a
> transaction on a clean filesystem during sync? A frozen filesystem,
> by definition, is a clean filesytem, and therefore sync calls of any
> kind should not be trying to write to the FS or start transactions.
> XFS does this just fine, so I'd consider such behaviour on a frozen
> filesystem a bug in ext3/4...
But by this you are essentially agreeing that the lock inversion is there
in principle. We just hide it by relying on the fact that no code path
trying to change anything with s_umount held (which is the right lock
ordering) gets called while the fs is frozen. And that is fragile.
Actually, I've looked for a while and if you call quotactl(), it will get
s_umount and then tell filesystem to update quota information which blocks
inside the fs waiting for filesystem being unfrozen => deadlock. We can
change this code path to wait for frozen filesystem before taking s_umount
that essentially it just reinstates my point - it't fragile and IMHO we
need some more consistent way to handle this...

> > So I'm afraid we are not going to get rid of
> > this ABBA dependency unless we declare that s_umount ranks above filesystem
> > being frozen - but surely I'm open to suggestions.
>
> Not sure I understand what you are saying there - this is already
> the case, isn't it? i.e. it has to be held exclusive to freeze a
> filesystem...
Not really. We freeze the fs under s_umount but freezing essentially
implements trylock semantics while setting s_frozen so that does not really
establish any lock dependency. What establishes lock dependency is the
thawing path which blocks on s_umount while the filesystem is still frozen.
And this dependency is the other way around - i.e., freezing above
s_umount. This is why I was messing with thawing code to fix this...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-06 17:46:21

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hello,

On Wed 06-04-11 16:40:15, Toshiyuki Okajima wrote:
> (2011/04/06 14:57), Jan Kara wrote:
> >On Wed 06-04-11 14:09:14, Toshiyuki Okajima wrote:
> >>(2011/04/06 7:54), Jan Kara wrote:
> >>>On Tue 05-04-11 19:25:44, Toshiyuki Okajima wrote:
> >>>>(2011/03/31 21:03), Toshiyuki Okajima wrote:
> >>>>>Hi, thanks for your reviewing.
> >>>>>
> >>>>>(2011/03/30 23:12), Jan Kara wrote:
> >>>>>>Hello,
> >>>>>>
> >>>>>>On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
> >>>>>>>On Thu, 17 Feb 2011 11:45:52 +0100
> >>>>>>>Jan Kara<[email protected]> wrote:
> >>>>>>>>On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
> >>>>>>>>>(2011/02/16 23:56), Jan Kara wrote:
> >>>>>>>>>>On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> >>>>>>>>>>>On Tue, 15 Feb 2011 18:29:54 +0100
> >>>>>>>>>>>Jan Kara<[email protected]> wrote:
> >>>>>>>>>>>>On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> >>>>>>>>>>>>>On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> >>>>><SNIP>
> >>>>>>>I have deeply continued to examined the root cause of this problem, then
> >>>>>>>I found it.
> >>>>>>>
> >>>>>>>It is that we can write a memory which is mmaped to a file. Then the memory
> >>>>>>>becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
> >>>>>>>"writeback" the memory.
> >>>>>>>
> >>>>>>>Therefore, the root cause of this hangup is not only ext4 component (with
> >>>>>>>delayed allocation feature) but also writeback mechanism for mmap. If you
> >>>>>>>use the other filesystem, you can write something to the filesystem though
> >>>>>>>you have freezed the filesystem.
> >>>>>
> >>>>>>Well, you can write something only in the caches, not to the on disk
> >>>>>>image. So it's not a problem as such.
> >>>>>My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
> >>>>>we can write in not only the caches but also the loopback device. However,
> >>>>>I don't still confirm that we can write to the real device(/dev/sdaX).
> >>>>>
> >>>>>>
> >>>>>>>A sample problem is attached on this mail. Try to execute it then you can
> >>>>>>>confirm that we can write some data to your filesystem while freezing the
> >>>>>>>filesystem.
> >>>>>>>(If you change FS variable in go.sh from ext3 to ext4 and you execute
> >>>>>>>"fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
> >>>>>>>
> >>>>>>>I think the best approach to fix this problem is to let users not to write
> >>>>>>>memory which is mapped to a certain file while the filesystem is freezing.
> >>>>>>>However, it is very difficult to control users not to write memory which has
> >>>>>>>been already mapped to the file.
> >>>>>>It is actually possible. In case of ext4, you could add a check (+ wait)
> >>>>>>in ext4_page_mkwrite() whether the filesystem is frozen or in the process
> >>>>>>of being frozen and if so, wait for it to get unfrozen. The only tough
> >>>>>>problem here might be the locking as ext4_page_mkwrite() is called with
> >>>>>>mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
> >>>>>>But you'd have to fix all filesystems (and all paths possibly creating
> >>>>>>dirty data) in this way.
> >>>>>>
> >>>>>
> >>>>>>>Therefore, I think there is only actual method that we stop writeback thread
> >>>>>>>to resolve the mmap problem. Also, by this fix, the original problem
> >>>>>>>(ext4 delayed write vs unfreeze) can be solved.
> >>>>>>Hmm, I had a look at the code again and think we could fix the issue
> >>>>>>cleanly (i.e. all possible users of s_umount) as follows: The lock
> >>>>>>ordering will be
> >>>>>>s_umount -> "fs frozen"
> >>>>>>and there will be a new mutex s_freeze_mutex protecting changes of
> >>>>>>s_frozen.
> >>>>>>
> >>>>>>freeze_bdev() already observes this lock ordering, it will only take
> >>>>>>s_freeze_mutex for the changes of s_frozen values. The only other code
> >>>>>>that is relevant for the lock ordering is thaw_super() (the freezing
> >>>>>>process is not expected to reenter kernel for the frozen filesystem).
> >>>>>>In thaw_super() we could take s_freeze_mutex, do all the thawing work,
> >>>>>>set s_frozen, release s_freeze_mutex and put superblock reference.
> >>>>>>
> >>>>>
> >>>>>>So something like the patch below - it seems to work for me, can you test
> >>>>>>it please?
> >>>>>I think your patch looks good, so, the original problem seems to be solved.
> >>>>>OK, I will test your patch.
> >>>>>This weekend I cannot test it. So, I will reply next week.
> >>>>I have tested whether Mizuma-san's reproducer can cause to deadlock with your
> >>>>patch. And then any problems didn't hit while the reproducer was running.
> >>>>
> >>>>I think your patch solves the original deadlock problem which is reported by
> >>>>Mizuma-san.
> >>> Good. Thanks.
> >>>
> >>>>>Reported-by: Toshiyuki Okajima<[email protected]>
> >>>>>Signed-off-by: Jan Kara<[email protected]>
> >>>>>---
> >>>>>fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
> >>>>>include/linux/fs.h | 1 +
> >>>>>2 files changed, 35 insertions(+), 6 deletions(-)
> >>>>
> >>
> >>>>However, I think a write which causes the deadlock is from mmapped dirty
> >>>>pages. So, I guess we also need to fix in the mmap path while fsfreezing.
> >>> Why? If you dirty a page, writeback thread can come and try to write it -
> >>>which blocks - but now that does not matter...
>
> >>I have not understood the code around writeback thread very much...
> >>Please explain me the concrete function name which blocks some writes?
> > It would block in ext4_da_writepages() function.
> In ext4 with delayed allocation case, I understand it blocks.
> (Original deadlock problem is just this case.)
> But in ext4 without delayed allocation or other filesystems case, which function
> can block writing?
For ext3 or ext4 without delayed allocation we block inside writepage()
function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
get modified to block while minor-faulting the page on frozen fs because
when blocks are already allocated we may skip starting a transaction and so
we could possibly modify the filesystem.

> >>Mizuma-san's reproducer also writes the data which maps to the file (mmap).
> >>The original problem happens after the fsfreeze operation is done.
> >>I understand the normal write operation (not mmap) can be blocked while
> >>fsfreezing. So, I guess we don't always block all the write operation
> >>while fsfreezing.
> > Technically speaking, we block all the transaction starts which means we
> >end up blocking all the writes from going to disk. But that does not mean
> >we block all the writes from going to in-memory cache - as you properly
> >note the mmap case is one of such exceptions.
> Hm, I also think we can allow the writes to in-memory cache but we can't allow
> the writes to disk while fsfreezing. I am considering that mmap path can
> write to disk while fsfreezing because this deadlock problem happens after
> fsfreeze operation is done...
I'm sorry I don't understand now - are you speaking about the case above
when writepage() does not wait for filesystem being frozen or something
else?

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-06 22:54:01

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed, Apr 06, 2011 at 07:40:01PM +0200, Jan Kara wrote:
> On Wed 06-04-11 21:21:35, Dave Chinner wrote:
> > On Wed, Apr 06, 2011 at 08:18:56AM +0200, Jan Kara wrote:
> > > On Wed 06-04-11 15:40:05, Dave Chinner wrote:
> > > > On Fri, Apr 01, 2011 at 04:08:56PM +0200, Jan Kara wrote:
> > > > > On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> > > > > > If you don't allow the page to be dirtied in the fist place, then
> > > > > > nothing needs to be done to the writeback path because there is
> > > > > > nothing dirty for it to write back.
> > > > > Sure but that's only the problem he was able to hit. But generally,
> > > > > there's a problem with needing s_umount for unfreezing because it isn't
> > > > > clear there aren't other code paths which can block with s_umount held
> > > > > waiting for fs to get unfrozen. And these code paths would cause the same
> > > > > deadlock. That's why I chose to get rid of s_umount during thawing.
> > > >
> > > > Holding the s_umount lock while checking if frozen and sleeping
> > > > is essentially an ABBA lock inversion bug that can bite in many more
> > > > places that just thawing the filesystem. Any where this is done should
> > > > be fixed, so I don't think just removing the s_umount lock from the thaw
> > > > path is sufficient to avoid problems.
> > > That's easily said but hard to do - any transaction start in ext3/4 may
> > > block on filesystem being frozen (this seems to be similar for XFS as I'm
> > > looking into the code) and transaction start traditionally nests inside
> > > s_umount (and basically there's no way around that since sync() calls your
> > > fs code with s_umount held).
> >
> > Sure, but the question must be asked - why is ext3/4 even starting a
> > transaction on a clean filesystem during sync? A frozen filesystem,
> > by definition, is a clean filesytem, and therefore sync calls of any
> > kind should not be trying to write to the FS or start transactions.
> > XFS does this just fine, so I'd consider such behaviour on a frozen
> > filesystem a bug in ext3/4...
> But by this you are essentially agreeing that the lock inversion is there
> in principle. We just hide it by relying on the fact that no code path
> trying to change anything with s_umount held (which is the right lock
> ordering) gets called while the fs is frozen. And that is fragile.

It's just another lock ordering rule. i.e. don't sleep on a frozen
filesystem with s_umount held. It's no more fragile than the many
other lock ordering rules we have.

> Actually, I've looked for a while and if you call quotactl(), it will get
> s_umount and then tell filesystem to update quota information which blocks
> inside the fs waiting for filesystem being unfrozen => deadlock.

Which is a bug according to the above locking rule.

> We can
> change this code path to wait for frozen filesystem before taking s_umount
> that essentially it just reinstates my point - it't fragile and IMHO we
> need some more consistent way to handle this...
>
> > > So I'm afraid we are not going to get rid of
> > > this ABBA dependency unless we declare that s_umount ranks above filesystem
> > > being frozen - but surely I'm open to suggestions.
> >
> > Not sure I understand what you are saying there - this is already
> > the case, isn't it? i.e. it has to be held exclusive to freeze a
> > filesystem...
> Not really. We freeze the fs under s_umount but freezing essentially
> implements trylock semantics while setting s_frozen so that does not really
> establish any lock dependency. What establishes lock dependency is the
> thawing path which blocks on s_umount while the filesystem is still frozen.
> And this dependency is the other way around - i.e., freezing above
> s_umount. This is why I was messing with thawing code to fix this...

It's just the tip of the iceberg. If we allow s_umount to be held
while waiting on a frozen filesystem, we open ouselves up to all
manner of problems. Such as umount hanging on a frozen fs,
(which means a shutdown with a frozen filesystem will hang), it can
hang sync, it can hang memory reclaim, it can hang in any path that
takes s_umount and hence do all sorts of bad things.

Yes, unthawing the filesystem will get things moving again with your
patch, but my point is that it simply does not address the problems
caused by the bad behaviour that has already occurred while the FS
is frozen. Fixing the thaw code in this way is like shooting the
messenger - it doesn't fix the problems being reported.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2011-04-06 22:59:37

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed, Apr 06, 2011 at 09:44:28AM -0400, Christoph Hellwig wrote:
> On Wed, Apr 06, 2011 at 09:21:35PM +1000, Dave Chinner wrote:
> > Sure, but the question must be asked - why is ext3/4 even starting a
> > transaction on a clean filesystem during sync? A frozen filesystem,
> > by definition, is a clean filesytem, and therefore sync calls of any
> > kind should not be trying to write to the FS or start transactions.
> > XFS does this just fine, so I'd consider such behaviour on a frozen
> > filesystem a bug in ext3/4...
>
> XFS does have one special case for this. When writing the dummy log
> record at the end of the freeze process we use _xfs_alloc_trans to
> bypass the frozen filesystem check as we have to write out this record
> when the filesystem already is frozen. But that's after the main
> sync with its normal transactions.

Right, that is a special case in the _freeze process_ (i.e. before
we've declared the FS frozen), not a normal operation on a frozen
filesystem.

If you want to list exceptions (i.e. where we explicitly avoid
writes to frozen fs), look for xfs_fs_writeable(), which stops
various write operations from proceeding when the fs is either
frozen, read-only or shut down.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2011-04-08 21:33:15

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Thu 07-04-11 08:54:01, Dave Chinner wrote:
> On Wed, Apr 06, 2011 at 07:40:01PM +0200, Jan Kara wrote:
> > On Wed 06-04-11 21:21:35, Dave Chinner wrote:
> > > On Wed, Apr 06, 2011 at 08:18:56AM +0200, Jan Kara wrote:
> > > > On Wed 06-04-11 15:40:05, Dave Chinner wrote:
> > > > > On Fri, Apr 01, 2011 at 04:08:56PM +0200, Jan Kara wrote:
> > > > > > On Fri 01-04-11 10:40:50, Dave Chinner wrote:
> > > > > > > If you don't allow the page to be dirtied in the fist place, then
> > > > > > > nothing needs to be done to the writeback path because there is
> > > > > > > nothing dirty for it to write back.
> > > > > > Sure but that's only the problem he was able to hit. But generally,
> > > > > > there's a problem with needing s_umount for unfreezing because it isn't
> > > > > > clear there aren't other code paths which can block with s_umount held
> > > > > > waiting for fs to get unfrozen. And these code paths would cause the same
> > > > > > deadlock. That's why I chose to get rid of s_umount during thawing.
> > > > >
> > > > > Holding the s_umount lock while checking if frozen and sleeping
> > > > > is essentially an ABBA lock inversion bug that can bite in many more
> > > > > places that just thawing the filesystem. Any where this is done should
> > > > > be fixed, so I don't think just removing the s_umount lock from the thaw
> > > > > path is sufficient to avoid problems.
> > > > That's easily said but hard to do - any transaction start in ext3/4 may
> > > > block on filesystem being frozen (this seems to be similar for XFS as I'm
> > > > looking into the code) and transaction start traditionally nests inside
> > > > s_umount (and basically there's no way around that since sync() calls your
> > > > fs code with s_umount held).
> > >
> > > Sure, but the question must be asked - why is ext3/4 even starting a
> > > transaction on a clean filesystem during sync? A frozen filesystem,
> > > by definition, is a clean filesytem, and therefore sync calls of any
> > > kind should not be trying to write to the FS or start transactions.
> > > XFS does this just fine, so I'd consider such behaviour on a frozen
> > > filesystem a bug in ext3/4...
> > But by this you are essentially agreeing that the lock inversion is there
> > in principle. We just hide it by relying on the fact that no code path
> > trying to change anything with s_umount held (which is the right lock
> > ordering) gets called while the fs is frozen. And that is fragile.
>
> It's just another lock ordering rule. i.e. don't sleep on a frozen
> filesystem with s_umount held. It's no more fragile than the many
> other lock ordering rules we have.
Except that for all the filesystems transaction start => sleep on a
frozen filesystem and in some code paths we have s_umount held while doing
a transaction start. So I don't buy the argument that it's just another
normal locking rule because normally we require that all the code paths
follow correct lock ordering. Now we have some paths (like sync) which do
not follow the correct lock ordering and we just make sure they are not
called if they could cause deadlocks by other means...

> > Actually, I've looked for a while and if you call quotactl(), it will get
> > s_umount and then tell filesystem to update quota information which blocks
> > inside the fs waiting for filesystem being unfrozen => deadlock.
>
> Which is a bug according to the above locking rule.
Yes, I was just trying to demonstrate that the locking rule changes
"block until the fs is unfrozen" into "kernel is deadlocked" in an
unexpected places... fsync_bdev() is another case which deadlocks
currently.

> > We can
> > change this code path to wait for frozen filesystem before taking s_umount
> > that essentially it just reinstates my point - it't fragile and IMHO we
> > need some more consistent way to handle this...
> >
> > > > So I'm afraid we are not going to get rid of
> > > > this ABBA dependency unless we declare that s_umount ranks above filesystem
> > > > being frozen - but surely I'm open to suggestions.
> > >
> > > Not sure I understand what you are saying there - this is already
> > > the case, isn't it? i.e. it has to be held exclusive to freeze a
> > > filesystem...
> > Not really. We freeze the fs under s_umount but freezing essentially
> > implements trylock semantics while setting s_frozen so that does not really
> > establish any lock dependency. What establishes lock dependency is the
> > thawing path which blocks on s_umount while the filesystem is still frozen.
> > And this dependency is the other way around - i.e., freezing above
> > s_umount. This is why I was messing with thawing code to fix this...
>
> It's just the tip of the iceberg. If we allow s_umount to be held
> while waiting on a frozen filesystem, we open ouselves up to all
> manner of problems. Such as umount hanging on a frozen fs,
> (which means a shutdown with a frozen filesystem will hang), it can
> hang sync, it can hang memory reclaim, it can hang in any path that
> takes s_umount and hence do all sorts of bad things.
I see. The umount hang (especially in the shutdown case) is not nice.
Direct reclaim won't be blocked AFAICS if we stop dirtying pages while the
fs is frozen (which, as I already wrote, I agree is not a good thing to do
after some thought). Since you can block while accessing the frozen
filesystem anyway (because of atime updates or just because of writing
process waiting with i_mutex held for fs to be unfrozen) I'm not sure how
much worse it would be if s_umount lock would be another lock with which
we can wait for fs to get unfrozen...

> Yes, unthawing the filesystem will get things moving again with your
> patch, but my point is that it simply does not address the problems
> caused by the bad behaviour that has already occurred while the FS
> is frozen. Fixing the thaw code in this way is like shooting the
> messenger - it doesn't fix the problems being reported.
I don't there has been any too bad behavior - you tried to access frozen
filesystem and you got blocked. But OK, I'll invest some more thought into
how to not block with s_umount held without sprinkling frozen checks over
the tree...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-15 13:37:26

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi, sorry for my late response.

(2011/04/07 2:46), Jan Kara wrote:
> Hello,
>
> On Wed 06-04-11 16:40:15, Toshiyuki Okajima wrote:
>> (2011/04/06 14:57), Jan Kara wrote:
>>> On Wed 06-04-11 14:09:14, Toshiyuki Okajima wrote:
>>>> (2011/04/06 7:54), Jan Kara wrote:
>>>>> On Tue 05-04-11 19:25:44, Toshiyuki Okajima wrote:
>>>>>> (2011/03/31 21:03), Toshiyuki Okajima wrote:
>>>>>>> Hi, thanks for your reviewing.
>>>>>>>
>>>>>>> (2011/03/30 23:12), Jan Kara wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> On Mon 28-03-11 17:06:28, Toshiyuki Okajima wrote:
>>>>>>>>> On Thu, 17 Feb 2011 11:45:52 +0100
>>>>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>>>>> On Thu 17-02-11 12:50:51, Toshiyuki Okajima wrote:
>>>>>>>>>>> (2011/02/16 23:56), Jan Kara wrote:
>>>>>>>>>>>> On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
>>>>>>>>>>>>> On Tue, 15 Feb 2011 18:29:54 +0100
>>>>>>>>>>>>> Jan Kara<[email protected]> wrote:
>>>>>>>>>>>>>> On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
>>>>>>>>>>>>>>> On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
>>>>>>> <SNIP>
>>>>>>>>> I have deeply continued to examined the root cause of this problem, then
>>>>>>>>> I found it.
>>>>>>>>>
>>>>>>>>> It is that we can write a memory which is mmaped to a file. Then the memory
>>>>>>>>> becomes "DIRTY" so then the flusher thread (ex. wb_do_writeback) tries to
>>>>>>>>> "writeback" the memory.
>>>>>>>>>
>>>>>>>>> Therefore, the root cause of this hangup is not only ext4 component (with
>>>>>>>>> delayed allocation feature) but also writeback mechanism for mmap. If you
>>>>>>>>> use the other filesystem, you can write something to the filesystem though
>>>>>>>>> you have freezed the filesystem.
>>>>>>>
>>>>>>>> Well, you can write something only in the caches, not to the on disk
>>>>>>>> image. So it's not a problem as such.
>>>>>>> My reproducer uses the loopback device(/dev/loopX). By using it, I have confirmed that
>>>>>>> we can write in not only the caches but also the loopback device. However,
>>>>>>> I don't still confirm that we can write to the real device(/dev/sdaX).
>>>>>>>
>>>>>>>>
>>>>>>>>> A sample problem is attached on this mail. Try to execute it then you can
>>>>>>>>> confirm that we can write some data to your filesystem while freezing the
>>>>>>>>> filesystem.
>>>>>>>>> (If you change FS variable in go.sh from ext3 to ext4 and you execute
>>>>>>>>> "fsfreeze -u mnt" manually on other prompt, you can also confirm this deadlock.)
>>>>>>>>>
>>>>>>>>> I think the best approach to fix this problem is to let users not to write
>>>>>>>>> memory which is mapped to a certain file while the filesystem is freezing.
>>>>>>>>> However, it is very difficult to control users not to write memory which has
>>>>>>>>> been already mapped to the file.
>>>>>>>> It is actually possible. In case of ext4, you could add a check (+ wait)
>>>>>>>> in ext4_page_mkwrite() whether the filesystem is frozen or in the process
>>>>>>>> of being frozen and if so, wait for it to get unfrozen. The only tough
>>>>>>>> problem here might be the locking as ext4_page_mkwrite() is called with
>>>>>>>> mmap_sem held and I'm not sure we can take s_umount with mmap_sem held.
>>>>>>>> But you'd have to fix all filesystems (and all paths possibly creating
>>>>>>>> dirty data) in this way.
>>>>>>>>
>>>>>>>
>>>>>>>>> Therefore, I think there is only actual method that we stop writeback thread
>>>>>>>>> to resolve the mmap problem. Also, by this fix, the original problem
>>>>>>>>> (ext4 delayed write vs unfreeze) can be solved.
>>>>>>>> Hmm, I had a look at the code again and think we could fix the issue
>>>>>>>> cleanly (i.e. all possible users of s_umount) as follows: The lock
>>>>>>>> ordering will be
>>>>>>>> s_umount -> "fs frozen"
>>>>>>>> and there will be a new mutex s_freeze_mutex protecting changes of
>>>>>>>> s_frozen.
>>>>>>>>
>>>>>>>> freeze_bdev() already observes this lock ordering, it will only take
>>>>>>>> s_freeze_mutex for the changes of s_frozen values. The only other code
>>>>>>>> that is relevant for the lock ordering is thaw_super() (the freezing
>>>>>>>> process is not expected to reenter kernel for the frozen filesystem).
>>>>>>>> In thaw_super() we could take s_freeze_mutex, do all the thawing work,
>>>>>>>> set s_frozen, release s_freeze_mutex and put superblock reference.
>>>>>>>>
>>>>>>>
>>>>>>>> So something like the patch below - it seems to work for me, can you test
>>>>>>>> it please?
>>>>>>> I think your patch looks good, so, the original problem seems to be solved.
>>>>>>> OK, I will test your patch.
>>>>>>> This weekend I cannot test it. So, I will reply next week.
>>>>>> I have tested whether Mizuma-san's reproducer can cause to deadlock with your
>>>>>> patch. And then any problems didn't hit while the reproducer was running.
>>>>>>
>>>>>> I think your patch solves the original deadlock problem which is reported by
>>>>>> Mizuma-san.
>>>>> Good. Thanks.
>>>>>
>>>>>>> Reported-by: Toshiyuki Okajima<[email protected]>
>>>>>>> Signed-off-by: Jan Kara<[email protected]>
>>>>>>> ---
>>>>>>> fs/super.c | 40 ++++++++++++++++++++++++++++++++++------
>>>>>>> include/linux/fs.h | 1 +
>>>>>>> 2 files changed, 35 insertions(+), 6 deletions(-)
>>>>>>
>>>>
>>>>>> However, I think a write which causes the deadlock is from mmapped dirty
>>>>>> pages. So, I guess we also need to fix in the mmap path while fsfreezing.
>>>>> Why? If you dirty a page, writeback thread can come and try to write it -
>>>>> which blocks - but now that does not matter...
>>
>>>> I have not understood the code around writeback thread very much...
>>>> Please explain me the concrete function name which blocks some writes?
>>> It would block in ext4_da_writepages() function.
>> In ext4 with delayed allocation case, I understand it blocks.
>> (Original deadlock problem is just this case.)
>> But in ext4 without delayed allocation or other filesystems case, which function
>> can block writing?

> For ext3 or ext4 without delayed allocation we block inside writepage()
> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
> get modified to block while minor-faulting the page on frozen fs because
> when blocks are already allocated we may skip starting a transaction and so
> we could possibly modify the filesystem.
OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.

(minor-pagefault)
-> do_wp_page()
-> page_mkwrite(= ext4_mkwrite())
=> BLOCK!

(major-pagefault)
-> do_liner_fault()
-> page_mkwrite(= ext4_mkwrite())
=> BLOCK!

>
>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
>>>> The original problem happens after the fsfreeze operation is done.
>>>> I understand the normal write operation (not mmap) can be blocked while
>>>> fsfreezing. So, I guess we don't always block all the write operation
>>>> while fsfreezing.
>>> Technically speaking, we block all the transaction starts which means we
>>> end up blocking all the writes from going to disk. But that does not mean
>>> we block all the writes from going to in-memory cache - as you properly
>>> note the mmap case is one of such exceptions.
>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
>> the writes to disk while fsfreezing. I am considering that mmap path can
>> write to disk while fsfreezing because this deadlock problem happens after
>> fsfreeze operation is done...
> I'm sorry I don't understand now - are you speaking about the case above
> when writepage() does not wait for filesystem being frozen or something
> else?
Sorry, I didn't understand around the page fault path.
So, I had read the kernel source code around it, then I maybe understand...

I worry whether we can update the file data in mmap case while fsfreezing.
Of course, I understand that we can write to in-memory cache, and it is not a
problem. However, if we can write to disk while fsfreezing, it is a problem.
So, I summarize the cases whether we can write to disk or not.

--------------------------------------------------------------------------
Cases (Whether we can write the data mmapped to the file on the disk
while fsfreezing)

[1] One of the page which has been mmapped is not bound. And
the page is not allocated yet. (major fault?)

(1) user dirtys a page
(2) a page fault occurs (do_page_fault)
(3) __do_falut is called.
(4) ext4_page_mkwrite is called
(5) ext4_write_begin is called
(6) ext4_journal_start_sb => We can STOP!

[2] One of the page which has been mmapped is not bound. But
the page is already allocated, and the buffer_heads of the page
are not mapped (BH_Mapped). (minor fault?)

(1) user dirtys a page
(2) a page fault occurs (do_page_fault)
(3) do_wp_page is called.
(4) ext4_page_mkwrite is called
(5) ext4_write_begin is called
(6) ext4_journal_start_sb => We can STOP!

[3] One of the page which has been mmapped is not bound. But
the page is already allocated, and the buffer_heads of the page
are mapped (BH_Mapped). (minor fault?)

(1) user dirtys a page
(2) a page fault occurs (do_page_fault)
(3) do_wp_page is called.
(4) ext4_page_mkwrite is called
* Cannot block the dirty page to be written because all bh is mapped.
(5) user munmaps the page (munmap)
(6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
(7) writeback thread writes the page (struct page) to disk
=> We cannot STOP!

[4] One of the page which has been mmapped is bound. And
the page is already allocated.

(1) user dirtys a page
( ) no page fault occurs
(2) user munmaps the page (munmap)
(3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
(4) writeback thread writes the page (struct page) to disk
=> We cannot STOP!
--------------------------------------------------------------------------

So, we can block the cases [1], [2].
But I think we cannot block the cases [3], [4] now.
If fixing the page_mkwrite, we can also block the case [3].
But the case [4] is not blocked because no page fault occurs
when we dirty the mmapped page.

Therefore, to repair this problem, we need to fix the cases [3], [4].
I think we must modify the writeback thread to fix the case [4].

Thanks,
Toshiyuki Okajima

2011-04-15 17:13:13

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hello,

On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
> > For ext3 or ext4 without delayed allocation we block inside writepage()
> >function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
> >get modified to block while minor-faulting the page on frozen fs because
> >when blocks are already allocated we may skip starting a transaction and so
> >we could possibly modify the filesystem.
> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
>
> (minor-pagefault)
> -> do_wp_page()
> -> page_mkwrite(= ext4_mkwrite())
> => BLOCK!
>
> (major-pagefault)
> -> do_liner_fault()
> -> page_mkwrite(= ext4_mkwrite())
> => BLOCK!
>
> >
> >>>>Mizuma-san's reproducer also writes the data which maps to the file (mmap).
> >>>>The original problem happens after the fsfreeze operation is done.
> >>>>I understand the normal write operation (not mmap) can be blocked while
> >>>>fsfreezing. So, I guess we don't always block all the write operation
> >>>>while fsfreezing.
> >>> Technically speaking, we block all the transaction starts which means we
> >>>end up blocking all the writes from going to disk. But that does not mean
> >>>we block all the writes from going to in-memory cache - as you properly
> >>>note the mmap case is one of such exceptions.
> >>Hm, I also think we can allow the writes to in-memory cache but we can't allow
> >>the writes to disk while fsfreezing. I am considering that mmap path can
> >>write to disk while fsfreezing because this deadlock problem happens after
> >>fsfreeze operation is done...
> > I'm sorry I don't understand now - are you speaking about the case above
> >when writepage() does not wait for filesystem being frozen or something
> >else?
> Sorry, I didn't understand around the page fault path.
> So, I had read the kernel source code around it, then I maybe understand...
>
> I worry whether we can update the file data in mmap case while fsfreezing.
> Of course, I understand that we can write to in-memory cache, and it is not a
> problem. However, if we can write to disk while fsfreezing, it is a problem.
> So, I summarize the cases whether we can write to disk or not.
>
> --------------------------------------------------------------------------
> Cases (Whether we can write the data mmapped to the file on the disk
> while fsfreezing)
>
> [1] One of the page which has been mmapped is not bound. And
> the page is not allocated yet. (major fault?)
>
> (1) user dirtys a page
> (2) a page fault occurs (do_page_fault)
> (3) __do_falut is called.
> (4) ext4_page_mkwrite is called
> (5) ext4_write_begin is called
> (6) ext4_journal_start_sb => We can STOP!
>
> [2] One of the page which has been mmapped is not bound. But
> the page is already allocated, and the buffer_heads of the page
> are not mapped (BH_Mapped). (minor fault?)
>
> (1) user dirtys a page
> (2) a page fault occurs (do_page_fault)
> (3) do_wp_page is called.
> (4) ext4_page_mkwrite is called
> (5) ext4_write_begin is called
> (6) ext4_journal_start_sb => We can STOP!
>
> [3] One of the page which has been mmapped is not bound. But
> the page is already allocated, and the buffer_heads of the page
> are mapped (BH_Mapped). (minor fault?)
>
> (1) user dirtys a page
> (2) a page fault occurs (do_page_fault)
> (3) do_wp_page is called.
> (4) ext4_page_mkwrite is called
> * Cannot block the dirty page to be written because all bh is mapped.
> (5) user munmaps the page (munmap)
> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> (7) writeback thread writes the page (struct page) to disk
> => We cannot STOP!
>
> [4] One of the page which has been mmapped is bound. And
> the page is already allocated.
>
> (1) user dirtys a page
> ( ) no page fault occurs
> (2) user munmaps the page (munmap)
> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> (4) writeback thread writes the page (struct page) to disk
> => We cannot STOP!
> --------------------------------------------------------------------------
>
> So, we can block the cases [1], [2].
> But I think we cannot block the cases [3], [4] now.
> If fixing the page_mkwrite, we can also block the case [3].
> But the case [4] is not blocked because no page fault occurs
> when we dirty the mmapped page.
>
> Therefore, to repair this problem, we need to fix the cases [3], [4].
> I think we must modify the writeback thread to fix the case [4].
The trick here is that when we write a page to disk, we write-protect
the page (you seem to call this that "the page is bound", I'm not sure why).
So we are guaranteed to receive a minor fault (case [3]) if user tries to
modify a page after we finish writeback while freezing the filesystem.
So principially all we need to do is just wait in ext4_page_mkwrite().

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-15 17:17:17

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 4/15/11 12:13 PM, Jan Kara wrote:
> Hello,
>
> On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
>>> For ext3 or ext4 without delayed allocation we block inside writepage()
>>> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
>>> get modified to block while minor-faulting the page on frozen fs because
>>> when blocks are already allocated we may skip starting a transaction and so
>>> we could possibly modify the filesystem.
>> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
>>
>> (minor-pagefault)
>> -> do_wp_page()
>> -> page_mkwrite(= ext4_mkwrite())
>> => BLOCK!
>>
>> (major-pagefault)
>> -> do_liner_fault()
>> -> page_mkwrite(= ext4_mkwrite())
>> => BLOCK!
>>
>>>
>>>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
>>>>>> The original problem happens after the fsfreeze operation is done.
>>>>>> I understand the normal write operation (not mmap) can be blocked while
>>>>>> fsfreezing. So, I guess we don't always block all the write operation
>>>>>> while fsfreezing.
>>>>> Technically speaking, we block all the transaction starts which means we
>>>>> end up blocking all the writes from going to disk. But that does not mean
>>>>> we block all the writes from going to in-memory cache - as you properly
>>>>> note the mmap case is one of such exceptions.
>>>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
>>>> the writes to disk while fsfreezing. I am considering that mmap path can
>>>> write to disk while fsfreezing because this deadlock problem happens after
>>>> fsfreeze operation is done...
>>> I'm sorry I don't understand now - are you speaking about the case above
>>> when writepage() does not wait for filesystem being frozen or something
>>> else?
>> Sorry, I didn't understand around the page fault path.
>> So, I had read the kernel source code around it, then I maybe understand...
>>
>> I worry whether we can update the file data in mmap case while fsfreezing.
>> Of course, I understand that we can write to in-memory cache, and it is not a
>> problem. However, if we can write to disk while fsfreezing, it is a problem.
>> So, I summarize the cases whether we can write to disk or not.
>>
>> --------------------------------------------------------------------------
>> Cases (Whether we can write the data mmapped to the file on the disk
>> while fsfreezing)
>>
>> [1] One of the page which has been mmapped is not bound. And
>> the page is not allocated yet. (major fault?)
>>
>> (1) user dirtys a page
>> (2) a page fault occurs (do_page_fault)
>> (3) __do_falut is called.
>> (4) ext4_page_mkwrite is called
>> (5) ext4_write_begin is called
>> (6) ext4_journal_start_sb => We can STOP!
>>
>> [2] One of the page which has been mmapped is not bound. But
>> the page is already allocated, and the buffer_heads of the page
>> are not mapped (BH_Mapped). (minor fault?)
>>
>> (1) user dirtys a page
>> (2) a page fault occurs (do_page_fault)
>> (3) do_wp_page is called.
>> (4) ext4_page_mkwrite is called
>> (5) ext4_write_begin is called
>> (6) ext4_journal_start_sb => We can STOP!
>>
>> [3] One of the page which has been mmapped is not bound. But
>> the page is already allocated, and the buffer_heads of the page
>> are mapped (BH_Mapped). (minor fault?)
>>
>> (1) user dirtys a page
>> (2) a page fault occurs (do_page_fault)
>> (3) do_wp_page is called.
>> (4) ext4_page_mkwrite is called
>> * Cannot block the dirty page to be written because all bh is mapped.
>> (5) user munmaps the page (munmap)
>> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>> (7) writeback thread writes the page (struct page) to disk
>> => We cannot STOP!
>>
>> [4] One of the page which has been mmapped is bound. And
>> the page is already allocated.
>>
>> (1) user dirtys a page
>> ( ) no page fault occurs
>> (2) user munmaps the page (munmap)
>> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>> (4) writeback thread writes the page (struct page) to disk
>> => We cannot STOP!
>> --------------------------------------------------------------------------
>>
>> So, we can block the cases [1], [2].
>> But I think we cannot block the cases [3], [4] now.
>> If fixing the page_mkwrite, we can also block the case [3].
>> But the case [4] is not blocked because no page fault occurs
>> when we dirty the mmapped page.
>>
>> Therefore, to repair this problem, we need to fix the cases [3], [4].
>> I think we must modify the writeback thread to fix the case [4].
> The trick here is that when we write a page to disk, we write-protect
> the page (you seem to call this that "the page is bound", I'm not sure why).
> So we are guaranteed to receive a minor fault (case [3]) if user tries to
> modify a page after we finish writeback while freezing the filesystem.
> So principially all we need to do is just wait in ext4_page_mkwrite().

I've been kind of absent from this thread, sorry, but why would we wait in ext4_page_mkwrite(), rather than in mm/memory.c prior to any page_mkwrite call on any fs?

no frozen fs should be able to dirty & write pages via mmap, right?

-Eric

> Honza

2011-04-15 17:37:34

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Fri 15-04-11 12:17:06, Eric Sandeen wrote:
> On 4/15/11 12:13 PM, Jan Kara wrote:
> > Hello,
> >
> > On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
> >>> For ext3 or ext4 without delayed allocation we block inside writepage()
> >>> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
> >>> get modified to block while minor-faulting the page on frozen fs because
> >>> when blocks are already allocated we may skip starting a transaction and so
> >>> we could possibly modify the filesystem.
> >> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
> >>
> >> (minor-pagefault)
> >> -> do_wp_page()
> >> -> page_mkwrite(= ext4_mkwrite())
> >> => BLOCK!
> >>
> >> (major-pagefault)
> >> -> do_liner_fault()
> >> -> page_mkwrite(= ext4_mkwrite())
> >> => BLOCK!
> >>
> >>>
> >>>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
> >>>>>> The original problem happens after the fsfreeze operation is done.
> >>>>>> I understand the normal write operation (not mmap) can be blocked while
> >>>>>> fsfreezing. So, I guess we don't always block all the write operation
> >>>>>> while fsfreezing.
> >>>>> Technically speaking, we block all the transaction starts which means we
> >>>>> end up blocking all the writes from going to disk. But that does not mean
> >>>>> we block all the writes from going to in-memory cache - as you properly
> >>>>> note the mmap case is one of such exceptions.
> >>>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
> >>>> the writes to disk while fsfreezing. I am considering that mmap path can
> >>>> write to disk while fsfreezing because this deadlock problem happens after
> >>>> fsfreeze operation is done...
> >>> I'm sorry I don't understand now - are you speaking about the case above
> >>> when writepage() does not wait for filesystem being frozen or something
> >>> else?
> >> Sorry, I didn't understand around the page fault path.
> >> So, I had read the kernel source code around it, then I maybe understand...
> >>
> >> I worry whether we can update the file data in mmap case while fsfreezing.
> >> Of course, I understand that we can write to in-memory cache, and it is not a
> >> problem. However, if we can write to disk while fsfreezing, it is a problem.
> >> So, I summarize the cases whether we can write to disk or not.
> >>
> >> --------------------------------------------------------------------------
> >> Cases (Whether we can write the data mmapped to the file on the disk
> >> while fsfreezing)
> >>
> >> [1] One of the page which has been mmapped is not bound. And
> >> the page is not allocated yet. (major fault?)
> >>
> >> (1) user dirtys a page
> >> (2) a page fault occurs (do_page_fault)
> >> (3) __do_falut is called.
> >> (4) ext4_page_mkwrite is called
> >> (5) ext4_write_begin is called
> >> (6) ext4_journal_start_sb => We can STOP!
> >>
> >> [2] One of the page which has been mmapped is not bound. But
> >> the page is already allocated, and the buffer_heads of the page
> >> are not mapped (BH_Mapped). (minor fault?)
> >>
> >> (1) user dirtys a page
> >> (2) a page fault occurs (do_page_fault)
> >> (3) do_wp_page is called.
> >> (4) ext4_page_mkwrite is called
> >> (5) ext4_write_begin is called
> >> (6) ext4_journal_start_sb => We can STOP!
> >>
> >> [3] One of the page which has been mmapped is not bound. But
> >> the page is already allocated, and the buffer_heads of the page
> >> are mapped (BH_Mapped). (minor fault?)
> >>
> >> (1) user dirtys a page
> >> (2) a page fault occurs (do_page_fault)
> >> (3) do_wp_page is called.
> >> (4) ext4_page_mkwrite is called
> >> * Cannot block the dirty page to be written because all bh is mapped.
> >> (5) user munmaps the page (munmap)
> >> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> >> (7) writeback thread writes the page (struct page) to disk
> >> => We cannot STOP!
> >>
> >> [4] One of the page which has been mmapped is bound. And
> >> the page is already allocated.
> >>
> >> (1) user dirtys a page
> >> ( ) no page fault occurs
> >> (2) user munmaps the page (munmap)
> >> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> >> (4) writeback thread writes the page (struct page) to disk
> >> => We cannot STOP!
> >> --------------------------------------------------------------------------
> >>
> >> So, we can block the cases [1], [2].
> >> But I think we cannot block the cases [3], [4] now.
> >> If fixing the page_mkwrite, we can also block the case [3].
> >> But the case [4] is not blocked because no page fault occurs
> >> when we dirty the mmapped page.
> >>
> >> Therefore, to repair this problem, we need to fix the cases [3], [4].
> >> I think we must modify the writeback thread to fix the case [4].
> > The trick here is that when we write a page to disk, we write-protect
> > the page (you seem to call this that "the page is bound", I'm not sure why).
> > So we are guaranteed to receive a minor fault (case [3]) if user tries to
> > modify a page after we finish writeback while freezing the filesystem.
> > So principially all we need to do is just wait in ext4_page_mkwrite().
>
> I've been kind of absent from this thread, sorry, but why would we wait in ext4_page_mkwrite(), rather than in mm/memory.c prior to any page_mkwrite call on any fs?
>
> no frozen fs should be able to dirty & write pages via mmap, right?
I have not put that much thought to it but locking might be kind of
tricky in the generic code. We have to be sure that freezing waits for
the page which is just being faulted. That means we have to take page lock
(now writepage() called during fs_freeze will wait for us), check whether
fs is frozen. If yes, unlock page, do vfs_check_frozen(), and retry. This
call sequence is much better suited for block_page_mkwrite() than for
code in memory.c I think.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-18 09:03:16

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi,

(2011/04/16 2:13), Jan Kara wrote:
> Hello,
>
> On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
>>> For ext3 or ext4 without delayed allocation we block inside writepage()
>>> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
>>> get modified to block while minor-faulting the page on frozen fs because
>>> when blocks are already allocated we may skip starting a transaction and so
>>> we could possibly modify the filesystem.
>> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
>>
>> (minor-pagefault)
>> -> do_wp_page()
>> -> page_mkwrite(= ext4_mkwrite())
>> => BLOCK!
>>
>> (major-pagefault)
>> -> do_liner_fault()
>> -> page_mkwrite(= ext4_mkwrite())
>> => BLOCK!
>>
>>>
>>>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
>>>>>> The original problem happens after the fsfreeze operation is done.
>>>>>> I understand the normal write operation (not mmap) can be blocked while
>>>>>> fsfreezing. So, I guess we don't always block all the write operation
>>>>>> while fsfreezing.
>>>>> Technically speaking, we block all the transaction starts which means we
>>>>> end up blocking all the writes from going to disk. But that does not mean
>>>>> we block all the writes from going to in-memory cache - as you properly
>>>>> note the mmap case is one of such exceptions.
>>>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
>>>> the writes to disk while fsfreezing. I am considering that mmap path can
>>>> write to disk while fsfreezing because this deadlock problem happens after
>>>> fsfreeze operation is done...
>>> I'm sorry I don't understand now - are you speaking about the case above
>>> when writepage() does not wait for filesystem being frozen or something
>>> else?
>> Sorry, I didn't understand around the page fault path.
>> So, I had read the kernel source code around it, then I maybe understand...
>>
>> I worry whether we can update the file data in mmap case while fsfreezing.
>> Of course, I understand that we can write to in-memory cache, and it is not a
>> problem. However, if we can write to disk while fsfreezing, it is a problem.
>> So, I summarize the cases whether we can write to disk or not.
>>
>> --------------------------------------------------------------------------
>> Cases (Whether we can write the data mmapped to the file on the disk
>> while fsfreezing)
>>
>> [1] One of the page which has been mmapped is not bound. And
>> the page is not allocated yet. (major fault?)
>>
>> (1) user dirtys a page
>> (2) a page fault occurs (do_page_fault)
>> (3) __do_falut is called.
>> (4) ext4_page_mkwrite is called
>> (5) ext4_write_begin is called
>> (6) ext4_journal_start_sb => We can STOP!
>>
>> [2] One of the page which has been mmapped is not bound. But
>> the page is already allocated, and the buffer_heads of the page
>> are not mapped (BH_Mapped). (minor fault?)
>>
>> (1) user dirtys a page
>> (2) a page fault occurs (do_page_fault)
>> (3) do_wp_page is called.
>> (4) ext4_page_mkwrite is called
>> (5) ext4_write_begin is called
>> (6) ext4_journal_start_sb => We can STOP!
>>
>> [3] One of the page which has been mmapped is not bound. But
>> the page is already allocated, and the buffer_heads of the page
>> are mapped (BH_Mapped). (minor fault?)
>>
>> (1) user dirtys a page
>> (2) a page fault occurs (do_page_fault)
>> (3) do_wp_page is called.
>> (4) ext4_page_mkwrite is called
>> * Cannot block the dirty page to be written because all bh is mapped.
>> (5) user munmaps the page (munmap)
>> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>> (7) writeback thread writes the page (struct page) to disk
>> => We cannot STOP!
>>
>> [4] One of the page which has been mmapped is bound. And
>> the page is already allocated.
>>
>> (1) user dirtys a page
>> ( ) no page fault occurs
>> (2) user munmaps the page (munmap)
>> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>> (4) writeback thread writes the page (struct page) to disk
>> => We cannot STOP!
>> --------------------------------------------------------------------------
>>
>> So, we can block the cases [1], [2].
>> But I think we cannot block the cases [3], [4] now.
>> If fixing the page_mkwrite, we can also block the case [3].
>> But the case [4] is not blocked because no page fault occurs
>> when we dirty the mmapped page.
>>
>> Therefore, to repair this problem, we need to fix the cases [3], [4].
>> I think we must modify the writeback thread to fix the case [4].
> The trick here is that when we write a page to disk, we write-protect
> the page (you seem to call this that "the page is bound", I'm not sure why).
Hm, I want to understand how to write-protect the page under fsfreezing.
But, anyway, I understand we don't need to consider the case [4].

> So we are guaranteed to receive a minor fault (case [3]) if user tries to
> modify a page after we finish writeback while freezing the filesystem.
> So principially all we need to do is just wait in ext4_page_mkwrite().
OK. I understand.
Are there any concrete ideas to fix this?
For ext4, we can rescue from the case [3] by modifying ext4_page_mkwrite().
But for ext3 or other FSs, we must implement ->page_mkwrite() to prevent it?

Thanks,
Toshiyuki Okajima

2011-04-18 10:51:05

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Mon 18-04-11 18:05:01, Toshiyuki Okajima wrote:
> >On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
> >>> For ext3 or ext4 without delayed allocation we block inside writepage()
> >>>function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
> >>>get modified to block while minor-faulting the page on frozen fs because
> >>>when blocks are already allocated we may skip starting a transaction and so
> >>>we could possibly modify the filesystem.
> >>OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
> >>
> >>(minor-pagefault)
> >>-> do_wp_page()
> >> -> page_mkwrite(= ext4_mkwrite())
> >> => BLOCK!
> >>
> >>(major-pagefault)
> >>-> do_liner_fault()
> >> -> page_mkwrite(= ext4_mkwrite())
> >> => BLOCK!
> >>
> >>>
> >>>>>>Mizuma-san's reproducer also writes the data which maps to the file (mmap).
> >>>>>>The original problem happens after the fsfreeze operation is done.
> >>>>>>I understand the normal write operation (not mmap) can be blocked while
> >>>>>>fsfreezing. So, I guess we don't always block all the write operation
> >>>>>>while fsfreezing.
> >>>>> Technically speaking, we block all the transaction starts which means we
> >>>>>end up blocking all the writes from going to disk. But that does not mean
> >>>>>we block all the writes from going to in-memory cache - as you properly
> >>>>>note the mmap case is one of such exceptions.
> >>>>Hm, I also think we can allow the writes to in-memory cache but we can't allow
> >>>>the writes to disk while fsfreezing. I am considering that mmap path can
> >>>>write to disk while fsfreezing because this deadlock problem happens after
> >>>>fsfreeze operation is done...
> >>> I'm sorry I don't understand now - are you speaking about the case above
> >>>when writepage() does not wait for filesystem being frozen or something
> >>>else?
> >>Sorry, I didn't understand around the page fault path.
> >>So, I had read the kernel source code around it, then I maybe understand...
> >>
> >>I worry whether we can update the file data in mmap case while fsfreezing.
> >>Of course, I understand that we can write to in-memory cache, and it is not a
> >>problem. However, if we can write to disk while fsfreezing, it is a problem.
> >>So, I summarize the cases whether we can write to disk or not.
> >>
> >>--------------------------------------------------------------------------
> >>Cases (Whether we can write the data mmapped to the file on the disk
> >>while fsfreezing)
> >>
> >>[1] One of the page which has been mmapped is not bound. And
> >> the page is not allocated yet. (major fault?)
> >>
> >> (1) user dirtys a page
> >> (2) a page fault occurs (do_page_fault)
> >> (3) __do_falut is called.
> >> (4) ext4_page_mkwrite is called
> >> (5) ext4_write_begin is called
> >> (6) ext4_journal_start_sb => We can STOP!
> >>
> >>[2] One of the page which has been mmapped is not bound. But
> >> the page is already allocated, and the buffer_heads of the page
> >> are not mapped (BH_Mapped). (minor fault?)
> >>
> >> (1) user dirtys a page
> >> (2) a page fault occurs (do_page_fault)
> >> (3) do_wp_page is called.
> >> (4) ext4_page_mkwrite is called
> >> (5) ext4_write_begin is called
> >> (6) ext4_journal_start_sb => We can STOP!
> >>
> >>[3] One of the page which has been mmapped is not bound. But
> >> the page is already allocated, and the buffer_heads of the page
> >> are mapped (BH_Mapped). (minor fault?)
> >>
> >> (1) user dirtys a page
> >> (2) a page fault occurs (do_page_fault)
> >> (3) do_wp_page is called.
> >> (4) ext4_page_mkwrite is called
> >> * Cannot block the dirty page to be written because all bh is mapped.
> >> (5) user munmaps the page (munmap)
> >> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> >> (7) writeback thread writes the page (struct page) to disk
> >> => We cannot STOP!
> >>
> >>[4] One of the page which has been mmapped is bound. And
> >> the page is already allocated.
> >>
> >> (1) user dirtys a page
> >> ( ) no page fault occurs
> >> (2) user munmaps the page (munmap)
> >> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> >> (4) writeback thread writes the page (struct page) to disk
> >> => We cannot STOP!
> >>--------------------------------------------------------------------------
> >>
> >>So, we can block the cases [1], [2].
> >>But I think we cannot block the cases [3], [4] now.
> >>If fixing the page_mkwrite, we can also block the case [3].
> >>But the case [4] is not blocked because no page fault occurs
> >>when we dirty the mmapped page.
> >>
> >>Therefore, to repair this problem, we need to fix the cases [3], [4].
> >>I think we must modify the writeback thread to fix the case [4].
> > The trick here is that when we write a page to disk, we write-protect
> >the page (you seem to call this that "the page is bound", I'm not sure why).
> Hm, I want to understand how to write-protect the page under fsfreezing.
Look at what page_mkclean() called from clear_page_dirty_for_io() does...

> But, anyway, I understand we don't need to consider the case [4].
Yes.

> >So we are guaranteed to receive a minor fault (case [3]) if user tries to
> >modify a page after we finish writeback while freezing the filesystem.
> >So principially all we need to do is just wait in ext4_page_mkwrite().
> OK. I understand.
> Are there any concrete ideas to fix this?
> For ext4, we can rescue from the case [3] by modifying ext4_page_mkwrite().
Yes.

> But for ext3 or other FSs, we must implement ->page_mkwrite() to prevent it?
Sadly I don't see a simple way to fix this issue for all filesystems at
once. Implementing proper wait in block_page_mkwrite() should fix the issue
for xfs. Other filesystems like GFS2 or Btrfs will have to be fixed
separately as ext4. For ext3, we'd have to add ->page_mkwrite() support. I
have patches for this already for some time but I have to get to properly
testing them in more exotic conditions like 64k pages...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-19 09:43:16

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi,

(2011/04/18 19:51), Jan Kara wrote:
> On Mon 18-04-11 18:05:01, Toshiyuki Okajima wrote:
>>> On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
>>>>> For ext3 or ext4 without delayed allocation we block inside writepage()
>>>>> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
>>>>> get modified to block while minor-faulting the page on frozen fs because
>>>>> when blocks are already allocated we may skip starting a transaction and so
>>>>> we could possibly modify the filesystem.
>>>> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
>>>>
>>>> (minor-pagefault)
>>>> -> do_wp_page()
>>>> -> page_mkwrite(= ext4_mkwrite())
>>>> => BLOCK!
>>>>
>>>> (major-pagefault)
>>>> -> do_liner_fault()
>>>> -> page_mkwrite(= ext4_mkwrite())
>>>> => BLOCK!
>>>>
>>>>>
>>>>>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
>>>>>>>> The original problem happens after the fsfreeze operation is done.
>>>>>>>> I understand the normal write operation (not mmap) can be blocked while
>>>>>>>> fsfreezing. So, I guess we don't always block all the write operation
>>>>>>>> while fsfreezing.
>>>>>>> Technically speaking, we block all the transaction starts which means we
>>>>>>> end up blocking all the writes from going to disk. But that does not mean
>>>>>>> we block all the writes from going to in-memory cache - as you properly
>>>>>>> note the mmap case is one of such exceptions.
>>>>>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
>>>>>> the writes to disk while fsfreezing. I am considering that mmap path can
>>>>>> write to disk while fsfreezing because this deadlock problem happens after
>>>>>> fsfreeze operation is done...
>>>>> I'm sorry I don't understand now - are you speaking about the case above
>>>>> when writepage() does not wait for filesystem being frozen or something
>>>>> else?
>>>> Sorry, I didn't understand around the page fault path.
>>>> So, I had read the kernel source code around it, then I maybe understand...
>>>>
>>>> I worry whether we can update the file data in mmap case while fsfreezing.
>>>> Of course, I understand that we can write to in-memory cache, and it is not a
>>>> problem. However, if we can write to disk while fsfreezing, it is a problem.
>>>> So, I summarize the cases whether we can write to disk or not.
>>>>
>>>> --------------------------------------------------------------------------
>>>> Cases (Whether we can write the data mmapped to the file on the disk
>>>> while fsfreezing)
>>>>
>>>> [1] One of the page which has been mmapped is not bound. And
>>>> the page is not allocated yet. (major fault?)
>>>>
>>>> (1) user dirtys a page
>>>> (2) a page fault occurs (do_page_fault)
>>>> (3) __do_falut is called.
>>>> (4) ext4_page_mkwrite is called
>>>> (5) ext4_write_begin is called
>>>> (6) ext4_journal_start_sb => We can STOP!
>>>>
>>>> [2] One of the page which has been mmapped is not bound. But
>>>> the page is already allocated, and the buffer_heads of the page
>>>> are not mapped (BH_Mapped). (minor fault?)
>>>>
>>>> (1) user dirtys a page
>>>> (2) a page fault occurs (do_page_fault)
>>>> (3) do_wp_page is called.
>>>> (4) ext4_page_mkwrite is called
>>>> (5) ext4_write_begin is called
>>>> (6) ext4_journal_start_sb => We can STOP!
>>>>
>>>> [3] One of the page which has been mmapped is not bound. But
>>>> the page is already allocated, and the buffer_heads of the page
>>>> are mapped (BH_Mapped). (minor fault?)
>>>>
>>>> (1) user dirtys a page
>>>> (2) a page fault occurs (do_page_fault)
>>>> (3) do_wp_page is called.
>>>> (4) ext4_page_mkwrite is called
>>>> * Cannot block the dirty page to be written because all bh is mapped.
>>>> (5) user munmaps the page (munmap)
>>>> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>>>> (7) writeback thread writes the page (struct page) to disk
>>>> => We cannot STOP!
>>>>
>>>> [4] One of the page which has been mmapped is bound. And
>>>> the page is already allocated.
>>>>
>>>> (1) user dirtys a page
>>>> ( ) no page fault occurs
>>>> (2) user munmaps the page (munmap)
>>>> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>>>> (4) writeback thread writes the page (struct page) to disk
>>>> => We cannot STOP!
>>>> --------------------------------------------------------------------------
>>>>
>>>> So, we can block the cases [1], [2].
>>>> But I think we cannot block the cases [3], [4] now.
>>>> If fixing the page_mkwrite, we can also block the case [3].
>>>> But the case [4] is not blocked because no page fault occurs
>>>> when we dirty the mmapped page.
>>>>
>>>> Therefore, to repair this problem, we need to fix the cases [3], [4].
>>>> I think we must modify the writeback thread to fix the case [4].
>>> The trick here is that when we write a page to disk, we write-protect
>>> the page (you seem to call this that "the page is bound", I'm not sure why).
>> Hm, I want to understand how to write-protect the page under fsfreezing.
> Look at what page_mkclean() called from clear_page_dirty_for_io() does...
Thanks. I'll read that.

>
>> But, anyway, I understand we don't need to consider the case [4].
> Yes.
>
>>> So we are guaranteed to receive a minor fault (case [3]) if user tries to
>>> modify a page after we finish writeback while freezing the filesystem.
>>> So principially all we need to do is just wait in ext4_page_mkwrite().
>> OK. I understand.
>> Are there any concrete ideas to fix this?
>> For ext4, we can rescue from the case [3] by modifying ext4_page_mkwrite().
> Yes.
>
>> But for ext3 or other FSs, we must implement ->page_mkwrite() to prevent it?
> Sadly I don't see a simple way to fix this issue for all filesystems at
> once. Implementing proper wait in block_page_mkwrite() should fix the issue
> for xfs. Other filesystems like GFS2 or Btrfs will have to be fixed
> separately as ext4. For ext3, we'd have to add ->page_mkwrite() support. I
> have patches for this already for some time but I have to get to properly
> testing them in more exotic conditions like 64k pages...
OK. I understand the current status of your works to fix the problem which
can be written with some data at mmap path while fsfreezing.

Thanks,
Toshiyuki Okajima

2011-04-22 06:58:39

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi,

On Tue, 19 Apr 2011 18:43:16 +0900
Toshiyuki Okajima <[email protected]> wrote:
> Hi,
>
> (2011/04/18 19:51), Jan Kara wrote:
> > On Mon 18-04-11 18:05:01, Toshiyuki Okajima wrote:
> >>> On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
> >>>>> For ext3 or ext4 without delayed allocation we block inside writepage()
> >>>>> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
> >>>>> get modified to block while minor-faulting the page on frozen fs because
> >>>>> when blocks are already allocated we may skip starting a transaction and so
> >>>>> we could possibly modify the filesystem.
> >>>> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
> >>>>
> >>>> (minor-pagefault)
> >>>> -> do_wp_page()
> >>>> -> page_mkwrite(= ext4_mkwrite())
> >>>> => BLOCK!
> >>>>
> >>>> (major-pagefault)
> >>>> -> do_liner_fault()
> >>>> -> page_mkwrite(= ext4_mkwrite())
> >>>> => BLOCK!
> >>>>
> >>>>>
> >>>>>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
> >>>>>>>> The original problem happens after the fsfreeze operation is done.
> >>>>>>>> I understand the normal write operation (not mmap) can be blocked while
> >>>>>>>> fsfreezing. So, I guess we don't always block all the write operation
> >>>>>>>> while fsfreezing.
> >>>>>>> Technically speaking, we block all the transaction starts which means we
> >>>>>>> end up blocking all the writes from going to disk. But that does not mean
> >>>>>>> we block all the writes from going to in-memory cache - as you properly
> >>>>>>> note the mmap case is one of such exceptions.
> >>>>>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
> >>>>>> the writes to disk while fsfreezing. I am considering that mmap path can
> >>>>>> write to disk while fsfreezing because this deadlock problem happens after
> >>>>>> fsfreeze operation is done...
> >>>>> I'm sorry I don't understand now - are you speaking about the case above
> >>>>> when writepage() does not wait for filesystem being frozen or something
> >>>>> else?
> >>>> Sorry, I didn't understand around the page fault path.
> >>>> So, I had read the kernel source code around it, then I maybe understand...
> >>>>
> >>>> I worry whether we can update the file data in mmap case while fsfreezing.
> >>>> Of course, I understand that we can write to in-memory cache, and it is not a
> >>>> problem. However, if we can write to disk while fsfreezing, it is a problem.
> >>>> So, I summarize the cases whether we can write to disk or not.
> >>>>
> >>>> --------------------------------------------------------------------------
> >>>> Cases (Whether we can write the data mmapped to the file on the disk
> >>>> while fsfreezing)
> >>>>
> >>>> [1] One of the page which has been mmapped is not bound. And
> >>>> the page is not allocated yet. (major fault?)
> >>>>
> >>>> (1) user dirtys a page
> >>>> (2) a page fault occurs (do_page_fault)
> >>>> (3) __do_falut is called.
> >>>> (4) ext4_page_mkwrite is called
> >>>> (5) ext4_write_begin is called
> >>>> (6) ext4_journal_start_sb => We can STOP!
> >>>>
> >>>> [2] One of the page which has been mmapped is not bound. But
> >>>> the page is already allocated, and the buffer_heads of the page
> >>>> are not mapped (BH_Mapped). (minor fault?)
> >>>>
> >>>> (1) user dirtys a page
> >>>> (2) a page fault occurs (do_page_fault)
> >>>> (3) do_wp_page is called.
> >>>> (4) ext4_page_mkwrite is called
> >>>> (5) ext4_write_begin is called
> >>>> (6) ext4_journal_start_sb => We can STOP!
> >>>>
> >>>> [3] One of the page which has been mmapped is not bound. But
> >>>> the page is already allocated, and the buffer_heads of the page
> >>>> are mapped (BH_Mapped). (minor fault?)
> >>>>
> >>>> (1) user dirtys a page
> >>>> (2) a page fault occurs (do_page_fault)
> >>>> (3) do_wp_page is called.
> >>>> (4) ext4_page_mkwrite is called
> >>>> * Cannot block the dirty page to be written because all bh is mapped.
> >>>> (5) user munmaps the page (munmap)
> >>>> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> >>>> (7) writeback thread writes the page (struct page) to disk
> >>>> => We cannot STOP!
> >>>>
> >>>> [4] One of the page which has been mmapped is bound. And
> >>>> the page is already allocated.
> >>>>
> >>>> (1) user dirtys a page
> >>>> ( ) no page fault occurs
> >>>> (2) user munmaps the page (munmap)
> >>>> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
> >>>> (4) writeback thread writes the page (struct page) to disk
> >>>> => We cannot STOP!
> >>>> --------------------------------------------------------------------------
> >>>>
> >>>> So, we can block the cases [1], [2].
> >>>> But I think we cannot block the cases [3], [4] now.
> >>>> If fixing the page_mkwrite, we can also block the case [3].
> >>>> But the case [4] is not blocked because no page fault occurs
> >>>> when we dirty the mmapped page.
> >>>>
> >>>> Therefore, to repair this problem, we need to fix the cases [3], [4].
> >>>> I think we must modify the writeback thread to fix the case [4].
> >>> The trick here is that when we write a page to disk, we write-protect
> >>> the page (you seem to call this that "the page is bound", I'm not sure why).
> >> Hm, I want to understand how to write-protect the page under fsfreezing.
> > Look at what page_mkclean() called from clear_page_dirty_for_io() does...
> Thanks. I'll read that.
>
> >
> >> But, anyway, I understand we don't need to consider the case [4].
> > Yes.
> >
> >>> So we are guaranteed to receive a minor fault (case [3]) if user tries to
> >>> modify a page after we finish writeback while freezing the filesystem.
> >>> So principially all we need to do is just wait in ext4_page_mkwrite().
> >> OK. I understand.
> >> Are there any concrete ideas to fix this?
> >> For ext4, we can rescue from the case [3] by modifying ext4_page_mkwrite().
> > Yes.
> >
> >> But for ext3 or other FSs, we must implement ->page_mkwrite() to prevent it?
> > Sadly I don't see a simple way to fix this issue for all filesystems at
> > once. Implementing proper wait in block_page_mkwrite() should fix the issue
> > for xfs. Other filesystems like GFS2 or Btrfs will have to be fixed
> > separately as ext4. For ext3, we'd have to add ->page_mkwrite() support. I
> > have patches for this already for some time but I have to get to properly
> > testing them in more exotic conditions like 64k pages...
> OK. I understand the current status of your works to fix the problem which
> can be written with some data at mmap path while fsfreezing.
I have confirmed that the following patch works fine while my or
Mizuma-san's reproducer is running. Therefore,
we can block to write the data, which is mmapped to a file, into a disk
by a page-fault while fsfreezing.

I think this patch fixes the following two problems:
- A deadlock occurs between ext4_da_writepages() (called from
writeback_inodes_wb) and thaw_super(). (reported by Mizuma-san)
- We can also write the data, which is mmapped to a file,
into a disk while fsfreezing (ext3/ext4).
(reported by me)

Please examine this patch.

Thanks,
Toshiyuki Okajima
---
fs/ext3/file.c | 19 ++++++++++++-
fs/ext3/inode.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++
fs/ext4/inode.c | 4 ++-
include/linux/ext3_fs.h | 1 +
4 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/fs/ext3/file.c b/fs/ext3/file.c
index f55df0e..6d376ef 100644
--- a/fs/ext3/file.c
+++ b/fs/ext3/file.c
@@ -52,6 +52,23 @@ static int ext3_release_file (struct inode * inode, struct file * filp)
return 0;
}

+static const struct vm_operations_struct ext3_file_vm_ops = {
+ .fault = filemap_fault,
+ .page_mkwrite = ext3_page_mkwrite,
+};
+
+static int ext3_file_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct address_space *mapping = file->f_mapping;
+
+ if (!mapping->a_ops->readpage)
+ return -ENOEXEC;
+ file_accessed(file);
+ vma->vm_ops = &ext3_file_vm_ops;
+ vma->vm_flags |= VM_CAN_NONLINEAR;
+ return 0;
+}
+
const struct file_operations ext3_file_operations = {
.llseek = generic_file_llseek,
.read = do_sync_read,
@@ -62,7 +79,7 @@ const struct file_operations ext3_file_operations = {
#ifdef CONFIG_COMPAT
.compat_ioctl = ext3_compat_ioctl,
#endif
- .mmap = generic_file_mmap,
+ .mmap = ext3_file_mmap,
.open = dquot_file_open,
.release = ext3_release_file,
.fsync = ext3_sync_file,
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 68b2e43..66c31dd 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -3496,3 +3496,74 @@ int ext3_change_inode_journal_flag(struct inode *inode, int val)

return err;
}
+
+int ext3_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct page *page = vmf->page;
+ loff_t size;
+ unsigned long len;
+ int ret = -EINVAL;
+ void *fsdata;
+ struct file *file = vma->vm_file;
+ struct inode *inode = file->f_path.dentry->d_inode;
+ struct address_space *mapping = inode->i_mapping;
+
+ /*
+ * Get i_alloc_sem to stop truncates messing with the inode. We cannot
+ * get i_mutex because we are already holding mmap_sem.
+ */
+ down_read(&inode->i_alloc_sem);
+ size = i_size_read(inode);
+ if (page->mapping != mapping || size <= page_offset(page)
+ || !PageUptodate(page)) {
+ /* page got truncated from under us? */
+ goto out_unlock;
+ }
+ ret = 0;
+ if (PageMappedToDisk(page))
+ goto out_frozen;
+
+ if (page->index == size >> PAGE_CACHE_SHIFT)
+ len = size & ~PAGE_CACHE_MASK;
+ else
+ len = PAGE_CACHE_SIZE;
+
+ lock_page(page);
+ /*
+ * return if we have all the buffers mapped. This avoid
+ * the need to call write_begin/write_end which does a
+ * journal_start/journal_stop which can block and take
+ * long time
+ */
+ if (page_has_buffers(page)) {
+ if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
+ buffer_unmapped)) {
+ unlock_page(page);
+out_frozen:
+ vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
+ goto out_unlock;
+ }
+ }
+ unlock_page(page);
+ /*
+ * OK, we need to fill the hole... Do write_begin write_end
+ * to do block allocation/reservation.We are not holding
+ * inode.i__mutex here. That allow * parallel write_begin,
+ * write_end call. lock_page prevent this from happening
+ * on the same page though
+ */
+ ret = mapping->a_ops->write_begin(file, mapping, page_offset(page),
+ len, AOP_FLAG_UNINTERRUPTIBLE, &page, &fsdata);
+ if (ret < 0)
+ goto out_unlock;
+ ret = mapping->a_ops->write_end(file, mapping, page_offset(page),
+ len, len, page, fsdata);
+ if (ret < 0)
+ goto out_unlock;
+ ret = 0;
+out_unlock:
+ if (ret)
+ ret = VM_FAULT_SIGBUS;
+ up_read(&inode->i_alloc_sem);
+ return ret;
+}
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f2fa5e8..44979ae 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5812,7 +5812,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
}
ret = 0;
if (PageMappedToDisk(page))
- goto out_unlock;
+ goto out_frozen;

if (page->index == size >> PAGE_CACHE_SHIFT)
len = size & ~PAGE_CACHE_MASK;
@@ -5830,6 +5830,8 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
ext4_bh_unmapped)) {
unlock_page(page);
+out_frozen:
+ vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
goto out_unlock;
}
}
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 85c1d30..a0e39ca 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -919,6 +919,7 @@ extern void ext3_get_inode_flags(struct ext3_inode_info *);
extern void ext3_set_aops(struct inode *inode);
extern int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
u64 start, u64 len);
+extern int ext3_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);

/* ioctl.c */
extern long ext3_ioctl(struct file *, unsigned int, unsigned long);
--
1.5.5.6

2011-04-22 21:26:07

by Peter M. Petrakis

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hi All,

On 04/22/2011 02:58 AM, Toshiyuki Okajima wrote:
> Hi,
>
> On Tue, 19 Apr 2011 18:43:16 +0900
> Toshiyuki Okajima <[email protected]> wrote:
>> Hi,
>>
>> (2011/04/18 19:51), Jan Kara wrote:
>>> On Mon 18-04-11 18:05:01, Toshiyuki Okajima wrote:
>>>>> On Fri 15-04-11 22:39:07, Toshiyuki Okajima wrote:
>>>>>>> For ext3 or ext4 without delayed allocation we block inside writepage()
>>>>>>> function. But as I wrote to Dave Chinner, ->page_mkwrite() should probably
>>>>>>> get modified to block while minor-faulting the page on frozen fs because
>>>>>>> when blocks are already allocated we may skip starting a transaction and so
>>>>>>> we could possibly modify the filesystem.
>>>>>> OK. I think ->page_mkwrite() should also block writing the minor-faulting pages.
>>>>>>
>>>>>> (minor-pagefault)
>>>>>> -> do_wp_page()
>>>>>> -> page_mkwrite(= ext4_mkwrite())
>>>>>> => BLOCK!
>>>>>>
>>>>>> (major-pagefault)
>>>>>> -> do_liner_fault()
>>>>>> -> page_mkwrite(= ext4_mkwrite())
>>>>>> => BLOCK!
>>>>>>
>>>>>>>
>>>>>>>>>> Mizuma-san's reproducer also writes the data which maps to the file (mmap).
>>>>>>>>>> The original problem happens after the fsfreeze operation is done.
>>>>>>>>>> I understand the normal write operation (not mmap) can be blocked while
>>>>>>>>>> fsfreezing. So, I guess we don't always block all the write operation
>>>>>>>>>> while fsfreezing.
>>>>>>>>> Technically speaking, we block all the transaction starts which means we
>>>>>>>>> end up blocking all the writes from going to disk. But that does not mean
>>>>>>>>> we block all the writes from going to in-memory cache - as you properly
>>>>>>>>> note the mmap case is one of such exceptions.
>>>>>>>> Hm, I also think we can allow the writes to in-memory cache but we can't allow
>>>>>>>> the writes to disk while fsfreezing. I am considering that mmap path can
>>>>>>>> write to disk while fsfreezing because this deadlock problem happens after
>>>>>>>> fsfreeze operation is done...
>>>>>>> I'm sorry I don't understand now - are you speaking about the case above
>>>>>>> when writepage() does not wait for filesystem being frozen or something
>>>>>>> else?
>>>>>> Sorry, I didn't understand around the page fault path.
>>>>>> So, I had read the kernel source code around it, then I maybe understand...
>>>>>>
>>>>>> I worry whether we can update the file data in mmap case while fsfreezing.
>>>>>> Of course, I understand that we can write to in-memory cache, and it is not a
>>>>>> problem. However, if we can write to disk while fsfreezing, it is a problem.
>>>>>> So, I summarize the cases whether we can write to disk or not.
>>>>>>
>>>>>> --------------------------------------------------------------------------
>>>>>> Cases (Whether we can write the data mmapped to the file on the disk
>>>>>> while fsfreezing)
>>>>>>
>>>>>> [1] One of the page which has been mmapped is not bound. And
>>>>>> the page is not allocated yet. (major fault?)
>>>>>>
>>>>>> (1) user dirtys a page
>>>>>> (2) a page fault occurs (do_page_fault)
>>>>>> (3) __do_falut is called.
>>>>>> (4) ext4_page_mkwrite is called
>>>>>> (5) ext4_write_begin is called
>>>>>> (6) ext4_journal_start_sb => We can STOP!
>>>>>>
>>>>>> [2] One of the page which has been mmapped is not bound. But
>>>>>> the page is already allocated, and the buffer_heads of the page
>>>>>> are not mapped (BH_Mapped). (minor fault?)
>>>>>>
>>>>>> (1) user dirtys a page
>>>>>> (2) a page fault occurs (do_page_fault)
>>>>>> (3) do_wp_page is called.
>>>>>> (4) ext4_page_mkwrite is called
>>>>>> (5) ext4_write_begin is called
>>>>>> (6) ext4_journal_start_sb => We can STOP!
>>>>>>
>>>>>> [3] One of the page which has been mmapped is not bound. But
>>>>>> the page is already allocated, and the buffer_heads of the page
>>>>>> are mapped (BH_Mapped). (minor fault?)
>>>>>>
>>>>>> (1) user dirtys a page
>>>>>> (2) a page fault occurs (do_page_fault)
>>>>>> (3) do_wp_page is called.
>>>>>> (4) ext4_page_mkwrite is called
>>>>>> * Cannot block the dirty page to be written because all bh is mapped.
>>>>>> (5) user munmaps the page (munmap)
>>>>>> (6) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>>>>>> (7) writeback thread writes the page (struct page) to disk
>>>>>> => We cannot STOP!
>>>>>>
>>>>>> [4] One of the page which has been mmapped is bound. And
>>>>>> the page is already allocated.
>>>>>>
>>>>>> (1) user dirtys a page
>>>>>> ( ) no page fault occurs
>>>>>> (2) user munmaps the page (munmap)
>>>>>> (3) zap_pte_range dirtys the page (struct page) which is pte_dirtyed.
>>>>>> (4) writeback thread writes the page (struct page) to disk
>>>>>> => We cannot STOP!
>>>>>> --------------------------------------------------------------------------
>>>>>>
>>>>>> So, we can block the cases [1], [2].
>>>>>> But I think we cannot block the cases [3], [4] now.
>>>>>> If fixing the page_mkwrite, we can also block the case [3].
>>>>>> But the case [4] is not blocked because no page fault occurs
>>>>>> when we dirty the mmapped page.
>>>>>>
>>>>>> Therefore, to repair this problem, we need to fix the cases [3], [4].
>>>>>> I think we must modify the writeback thread to fix the case [4].
>>>>> The trick here is that when we write a page to disk, we write-protect
>>>>> the page (you seem to call this that "the page is bound", I'm not sure why).
>>>> Hm, I want to understand how to write-protect the page under fsfreezing.
>>> Look at what page_mkclean() called from clear_page_dirty_for_io() does...
>> Thanks. I'll read that.
>>
>>>
>>>> But, anyway, I understand we don't need to consider the case [4].
>>> Yes.
>>>
>>>>> So we are guaranteed to receive a minor fault (case [3]) if user tries to
>>>>> modify a page after we finish writeback while freezing the filesystem.
>>>>> So principially all we need to do is just wait in ext4_page_mkwrite().
>>>> OK. I understand.
>>>> Are there any concrete ideas to fix this?
>>>> For ext4, we can rescue from the case [3] by modifying ext4_page_mkwrite().
>>> Yes.
>>>
>>>> But for ext3 or other FSs, we must implement ->page_mkwrite() to prevent it?
>>> Sadly I don't see a simple way to fix this issue for all filesystems at
>>> once. Implementing proper wait in block_page_mkwrite() should fix the issue
>>> for xfs. Other filesystems like GFS2 or Btrfs will have to be fixed
>>> separately as ext4. For ext3, we'd have to add ->page_mkwrite() support. I
>>> have patches for this already for some time but I have to get to properly
>>> testing them in more exotic conditions like 64k pages...
>> OK. I understand the current status of your works to fix the problem which
>> can be written with some data at mmap path while fsfreezing.
> I have confirmed that the following patch works fine while my or
> Mizuma-san's reproducer is running. Therefore,
> we can block to write the data, which is mmapped to a file, into a disk
> by a page-fault while fsfreezing.
>
> I think this patch fixes the following two problems:
> - A deadlock occurs between ext4_da_writepages() (called from
> writeback_inodes_wb) and thaw_super(). (reported by Mizuma-san)
> - We can also write the data, which is mmapped to a file,
> into a disk while fsfreezing (ext3/ext4).
> (reported by me)
>
> Please examine this patch.

We've recently identified the same root cause in 2.6.32 though the hit rate
is much much higher. The configuration is a SAN ALUA Active/Standby using
multipath. The s_wait_unfrozen/s_umount deadlock is regularly encountered
when a path comes back into service, as a result of a kpartx invocation on
behalf of this udev rule.

/lib/udev/rules.d/95-kpartx.rules

# Create dm tables for partitions
ENV{DM_STATE}=="ACTIVE", ENV{DM_UUID}=="mpath-*", \
RUN+="/sbin/dmsetup ls --target multipath --exec '/sbin/kpartx -a -p -part' -j %M -m %m"

Below are the logs of the current incarntion of the fault with your current patch against 2.6.38.
Still working to obtain a viable crashdump.

[ 1898.017614] mptsas: ioc0: mptsas_add_fw_event: add (fw_event=0xffff880c3c815200)

[ 1898.025995] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c814780)

[ 1898.034625] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c814b40), event = (0x12)

[ 1898.044803] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c814b40)

[ 1898.053475] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c815c80), event = (0x12)

[ 1898.063690] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c815c80)

[ 1898.072316] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c815200), event = (0x0f)

[ 1898.082544] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c815200)

[ 1898.571426] sd 0:0:1:0: alua: port group 01 state S supports toluSnA

[ 1898.578635] device-mapper: multipath: Failing path 8:32.

[ 2041.345645] INFO: task kjournald:595 blocked for more than 120 seconds.

[ 2041.353075] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2041.361891] kjournald D ffff88063acb9a90 0 595 2 0x00000000

[ 2041.369891] ffff88063ace1c30 0000000000000046 ffff88063c282140 ffff880600000000

[ 2041.378416] 0000000000013cc0 ffff88063acb96e0 ffff88063acb9a90 ffff88063ace1fd8

[ 2041.386954] ffff88063acb9a98 0000000000013cc0 ffff88063ace0010 0000000000013cc0

[ 2041.395561] Call Trace:

[ 2041.398358] [<ffffffff81192380>] ? sync_buffer+0x0/0x50

[ 2041.404342] [<ffffffff815d3120>] io_schedule+0x70/0xc0

[ 2041.410227] [<ffffffff811923c5>] sync_buffer+0x45/0x50

[ 2041.416179] [<ffffffff815d378f>] __wait_on_bit+0x5f/0x90

[ 2041.422258] [<ffffffff81192380>] ? sync_buffer+0x0/0x50

[ 2041.428275] [<ffffffff815d3838>] out_of_line_wait_on_bit+0x78/0x90

[ 2041.435324] [<ffffffff81086b90>] ? wake_bit_function+0x0/0x40

[ 2041.441958] [<ffffffff8119237e>] __wait_on_buffer+0x2e/0x30

[ 2041.448333] [<ffffffff8123ab14>] journal_commit_transaction+0x7e4/0xec0

[ 2041.455873] [<ffffffff81038d09>] ? default_spin_lock_flags+0x9/0x10

[ 2041.463020] [<ffffffff8107443c>] ? lock_timer_base+0x3c/0x70

[ 2041.469514] [<ffffffff81074e33>] ? try_to_del_timer_sync+0x83/0xe0

[ 2041.476563] [<ffffffff8123df7d>] kjournald+0xed/0x250

[ 2041.482349] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40

[ 2041.489624] [<ffffffff8123de90>] ? kjournald+0x0/0x250

[ 2041.495504] [<ffffffff810865e6>] kthread+0x96/0xa0

[ 2041.501003] [<ffffffff8100ce64>] kernel_thread_helper+0x4/0x10

[ 2041.507667] [<ffffffff81086550>] ? kthread+0x0/0xa0

[ 2041.513301] [<ffffffff8100ce60>] ? kernel_thread_helper+0x0/0x10

[ 2041.520247] INFO: task rsyslogd:1854 blocked for more than 120 seconds.

[ 2041.527677] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2041.536499] rsyslogd D ffff88063c513170 0 1854 1 0x00000000

[ 2041.544533] ffff88063d0e3cd8 0000000000000082 ffff88063c479180 0000000000000000

[ 2041.553108] 0000000000013cc0 ffff88063c512dc0 ffff88063c513170 ffff88063d0e3fd8

[ 2041.561691] ffff88063c513178 0000000000013cc0 ffff88063d0e2010 0000000000013cc0

[ 2041.570323] Call Trace:

[ 2041.573108] [<ffffffff8110c78d>] __generic_file_aio_write+0xbd/0x470

[ 2041.580447] [<ffffffff8108a82d>] ? hrtimer_try_to_cancel+0x3d/0xd0

[ 2041.587496] [<ffffffff81097e3d>] ? futex_wait_queue_me+0xcd/0x110

[ 2041.594489] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40

[ 2041.601833] [<ffffffff8110cba2>] generic_file_aio_write+0x62/0xd0

[ 2041.608831] [<ffffffff81163a9a>] do_sync_write+0xda/0x120

[ 2041.615165] [<ffffffff812de756>] ? rb_erase+0xd6/0x160

[ 2041.621050] [<ffffffff812ac918>] ? apparmor_file_permission+0x18/0x20

[ 2041.628395] [<ffffffff81279b23>] ? security_file_permission+0x23/0x90

[ 2041.635827] [<ffffffff81164018>] vfs_write+0xc8/0x190

[ 2041.641649] [<ffffffff811641d1>] sys_write+0x51/0x90

[ 2041.647337] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2041.654091] INFO: task multipathd:1337 blocked for more than 120 seconds.

[ 2041.661750] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2041.670669] multipathd D ffff88063e3303b0 0 1337 1 0x00000000

[ 2041.678746] ffff88063c0fda18 0000000000000082 0000000000000000 ffff880600000000

[ 2041.687219] 0000000000013cc0 ffff88063e330000 ffff88063e3303b0 ffff88063c0fdfd8

[ 2041.695818] ffff88063e3303b8 0000000000013cc0 ffff88063c0fc010 0000000000013cc0

[ 2041.704369] Call Trace:

[ 2041.707128] [<ffffffff815d349d>] schedule_timeout+0x21d/0x300

[ 2041.713679] [<ffffffff8104c8ec>] ? resched_task+0x2c/0x90

[ 2041.719846] [<ffffffff8105f763>] ? try_to_wake_up+0xc3/0x410

[ 2041.726301] [<ffffffff815d2436>] wait_for_common+0xd6/0x180

[ 2041.732685] [<ffffffff8105fb05>] ? wake_up_process+0x15/0x20

[ 2041.739138] [<ffffffff8105fab0>] ? default_wake_function+0x0/0x20

[ 2041.746079] [<ffffffff815d25bd>] wait_for_completion+0x1d/0x20

[ 2041.752716] [<ffffffff8107de18>] call_usermodehelper_exec+0xd8/0xe0

[ 2041.759853] [<ffffffff814a3110>] ? parse_hw_handler+0xb0/0x240

[ 2041.766503] [<ffffffff8107e060>] __request_module+0x190/0x210

[ 2041.773054] [<ffffffff812e0c28>] ? sscanf+0x38/0x40

[ 2041.778636] [<ffffffff814a3110>] parse_hw_handler+0xb0/0x240

[ 2041.785121] [<ffffffff814a38c3>] multipath_ctr+0x83/0x1d0

[ 2041.791312] [<ffffffff8149abd5>] ? dm_split_args+0x75/0x140

[ 2041.797671] [<ffffffff8149b9af>] dm_table_add_target+0xff/0x250

[ 2041.804413] [<ffffffff8149de3a>] table_load+0xca/0x2f0

[ 2041.810317] [<ffffffff8149dd70>] ? table_load+0x0/0x2f0

[ 2041.816316] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240

[ 2041.822184] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20

[ 2041.828188] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0

[ 2041.834250] [<ffffffff8109ae6b>] ? sys_futex+0x7b/0x170

[ 2041.840219] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0

[ 2041.845898] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2041.852639] INFO: task iozone:1871 blocked for more than 120 seconds.

[ 2041.859921] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2041.868760] iozone D ffff880c3bc21a90 0 1871 1869 0x00000000

[ 2041.876728] ffff880c3e743e20 0000000000000086 0000000000000001 ffff880c00000000

[ 2041.885177] 0000000000013cc0 ffff880c3bc216e0 ffff880c3bc21a90 ffff880c3e743fd8

[ 2041.893647] ffff880c3bc21a98 0000000000013cc0 ffff880c3e742010 0000000000013cc0

[ 2041.902112] Call Trace:

[ 2041.906302] [<ffffffff8104c8ec>] ? resched_task+0x2c/0x90

[ 2041.912494] [<ffffffff815d4ddd>] rwsem_down_failed_common+0xcd/0x170

[ 2041.919718] [<ffffffff8118f480>] ? sync_one_sb+0x0/0x30

[ 2041.925719] [<ffffffff815d4eb5>] rwsem_down_read_failed+0x15/0x17

[ 2041.932690] [<ffffffff812e41a4>] call_rwsem_down_read_failed+0x14/0x30

[ 2041.940116] [<ffffffff815d4207>] ? down_read+0x17/0x20

[ 2041.945990] [<ffffffff811665e1>] iterate_supers+0x71/0xf0

[ 2041.952149] [<ffffffff8118f4df>] sys_sync+0x2f/0x70

[ 2041.957763] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2041.964575] INFO: task kpartx:1897 blocked for more than 120 seconds.

[ 2041.971801] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2041.980626] kpartx D ffff88063d05df30 0 1897 1896 0x00000000

[ 2041.988607] ffff88063c3a5b58 0000000000000082 0000000e3c3a5ac8 ffff880c00000000

[ 2041.997056] 0000000000013cc0 ffff88063d05db80 ffff88063d05df30 ffff88063c3a5fd8

[ 2042.005496] ffff88063d05df38 0000000000013cc0 ffff88063c3a4010 0000000000013cc0

[ 2042.013939] Call Trace:

[ 2042.016702] [<ffffffff8123dc85>] log_wait_commit+0xc5/0x150

[ 2042.023089] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40

[ 2042.030321] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20

[ 2042.036584] [<ffffffff811e6256>] ext3_sync_fs+0x66/0x70

[ 2042.042552] [<ffffffff811ba7c1>] dquot_quota_sync+0x1c1/0x330

[ 2042.049133] [<ffffffff81115391>] ? do_writepages+0x21/0x40

[ 2042.055423] [<ffffffff8110ae8b>] ? __filemap_fdatawrite_range+0x5b/0x60

[ 2042.062944] [<ffffffff8118f42c>] __sync_filesystem+0x3c/0x90

[ 2042.069430] [<ffffffff8118f56b>] sync_filesystem+0x4b/0x70

[ 2042.075690] [<ffffffff81166a85>] freeze_super+0x55/0x100

[ 2042.081754] [<ffffffff811993b8>] freeze_bdev+0x98/0xe0

[ 2042.087625] [<ffffffff81499001>] dm_suspend+0xa1/0x2e0

[ 2042.093495] [<ffffffff8149ced9>] ? __get_name_cell+0x99/0xb0

[ 2042.099948] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0

[ 2042.105916] [<ffffffff8149e29b>] do_resume+0x17b/0x1b0

[ 2042.111784] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0

[ 2042.117753] [<ffffffff8149e365>] dev_suspend+0x95/0xb0

[ 2042.123621] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0

[ 2042.129591] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240

[ 2042.135493] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20

[ 2042.141770] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20

[ 2042.147739] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0

[ 2042.153801] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0

[ 2042.159478] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2161.971321] INFO: task rsyslogd:1854 blocked for more than 120 seconds.

[ 2161.978798] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2161.987656] rsyslogd D ffff88063c513170 0 1854 1 0x00000000

[ 2161.995718] ffff88063d0e3cd8 0000000000000082 ffff88063c479180 0000000000000000

[ 2162.004340] 0000000000013cc0 ffff88063c512dc0 ffff88063c513170 ffff88063d0e3fd8

[ 2162.012932] ffff88063c513178 0000000000013cc0 ffff88063d0e2010 0000000000013cc0

[ 2162.021481] Call Trace:

[ 2162.024290] [<ffffffff8110c78d>] __generic_file_aio_write+0xbd/0x470

[ 2162.031627] [<ffffffff8108a82d>] ? hrtimer_try_to_cancel+0x3d/0xd0

[ 2162.038711] [<ffffffff81097e3d>] ? futex_wait_queue_me+0xcd/0x110

[ 2162.045662] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40

[ 2162.053007] [<ffffffff8110cba2>] generic_file_aio_write+0x62/0xd0

[ 2162.059962] [<ffffffff81163a9a>] do_sync_write+0xda/0x120

[ 2162.066165] [<ffffffff812de756>] ? rb_erase+0xd6/0x160

[ 2162.072048] [<ffffffff812ac918>] ? apparmor_file_permission+0x18/0x20

[ 2162.079387] [<ffffffff81279b23>] ? security_file_permission+0x23/0x90

[ 2162.086761] [<ffffffff81164018>] vfs_write+0xc8/0x190

[ 2162.092552] [<ffffffff811641d1>] sys_write+0x51/0x90

[ 2162.098247] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2162.105042] INFO: task multipathd:1337 blocked for more than 120 seconds.

[ 2162.112667] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2162.121487] multipathd D ffff88063e3303b0 0 1337 1 0x00000000

[ 2162.129517] ffff88063c0fda18 0000000000000082 0000000000000000 ffff880600000000

[ 2162.138112] 0000000000013cc0 ffff88063e330000 ffff88063e3303b0 ffff88063c0fdfd8

[ 2162.146688] ffff88063e3303b8 0000000000013cc0 ffff88063c0fc010 0000000000013cc0

[ 2162.155253] Call Trace:

[ 2162.158073] [<ffffffff815d349d>] schedule_timeout+0x21d/0x300

[ 2162.164639] [<ffffffff8104c8ec>] ? resched_task+0x2c/0x90

[ 2162.170886] [<ffffffff8105f763>] ? try_to_wake_up+0xc3/0x410

[ 2162.177389] [<ffffffff815d2436>] wait_for_common+0xd6/0x180

[ 2162.183852] [<ffffffff8105fb05>] ? wake_up_process+0x15/0x20

[ 2162.190317] [<ffffffff8105fab0>] ? default_wake_function+0x0/0x20

[ 2162.197304] [<ffffffff815d25bd>] wait_for_completion+0x1d/0x20

[ 2162.203968] [<ffffffff8107de18>] call_usermodehelper_exec+0xd8/0xe0

[ 2162.211111] [<ffffffff814a3110>] ? parse_hw_handler+0xb0/0x240

[ 2162.217807] [<ffffffff8107e060>] __request_module+0x190/0x210

[ 2162.224461] [<ffffffff812e0c28>] ? sscanf+0x38/0x40

[ 2162.230054] [<ffffffff814a3110>] parse_hw_handler+0xb0/0x240

[ 2162.236503] [<ffffffff814a38c3>] multipath_ctr+0x83/0x1d0

[ 2162.242673] [<ffffffff8149abd5>] ? dm_split_args+0x75/0x140

[ 2162.249079] [<ffffffff8149b9af>] dm_table_add_target+0xff/0x250

[ 2162.255840] [<ffffffff8149de3a>] table_load+0xca/0x2f0

[ 2162.261719] [<ffffffff8149dd70>] ? table_load+0x0/0x2f0

[ 2162.267701] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240

[ 2162.273621] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20

[ 2162.279592] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0

[ 2162.285710] [<ffffffff8109ae6b>] ? sys_futex+0x7b/0x170

[ 2162.291694] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0

[ 2162.297383] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2162.304169] INFO: task iozone:1871 blocked for more than 120 seconds.

[ 2162.311407] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2162.320229] iozone D ffff880c3bc21a90 0 1871 1869 0x00000000

[ 2162.328317] ffff880c3e743e20 0000000000000086 0000000000000001 ffff880c00000000

[ 2162.336901] 0000000000013cc0 ffff880c3bc216e0 ffff880c3bc21a90 ffff880c3e743fd8

[ 2162.345415] ffff880c3bc21a98 0000000000013cc0 ffff880c3e742010 0000000000013cc0

[ 2162.353887] Call Trace:

[ 2162.356650] [<ffffffff8104c8ec>] ? resched_task+0x2c/0x90

[ 2162.362815] [<ffffffff815d4ddd>] rwsem_down_failed_common+0xcd/0x170

[ 2162.370042] [<ffffffff8118f480>] ? sync_one_sb+0x0/0x30

[ 2162.376121] [<ffffffff815d4eb5>] rwsem_down_read_failed+0x15/0x17

[ 2162.383075] [<ffffffff812e41a4>] call_rwsem_down_read_failed+0x14/0x30

[ 2162.390575] [<ffffffff815d4207>] ? down_read+0x17/0x20

[ 2162.396501] [<ffffffff811665e1>] iterate_supers+0x71/0xf0

[ 2162.402768] [<ffffffff8118f4df>] sys_sync+0x2f/0x70

[ 2162.408360] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2162.415159] INFO: task kpartx:1897 blocked for more than 120 seconds.

[ 2162.422493] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.

[ 2162.431405] kpartx D ffff88063d05df30 0 1897 1896 0x00000000

[ 2162.439440] ffff88063c3a5b58 0000000000000082 0000000e3c3a5ac8 ffff880c00000000

[ 2162.448021] 0000000000013cc0 ffff88063d05db80 ffff88063d05df30 ffff88063c3a5fd8

[ 2162.456468] ffff88063d05df38 0000000000013cc0 ffff88063c3a4010 0000000000013cc0

[ 2162.464962] Call Trace:

[ 2162.467724] [<ffffffff8123dc85>] log_wait_commit+0xc5/0x150

[ 2162.474088] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40

[ 2162.481319] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20

[ 2162.487577] [<ffffffff811e6256>] ext3_sync_fs+0x66/0x70

[ 2162.493548] [<ffffffff811ba7c1>] dquot_quota_sync+0x1c1/0x330

[ 2162.500107] [<ffffffff81115391>] ? do_writepages+0x21/0x40

[ 2162.506415] [<ffffffff8110ae8b>] ? __filemap_fdatawrite_range+0x5b/0x60

[ 2162.513947] [<ffffffff8118f42c>] __sync_filesystem+0x3c/0x90

[ 2162.520514] [<ffffffff8118f56b>] sync_filesystem+0x4b/0x70

[ 2162.526783] [<ffffffff81166a85>] freeze_super+0x55/0x100

[ 2162.532896] [<ffffffff811993b8>] freeze_bdev+0x98/0xe0

[ 2162.538819] [<ffffffff81499001>] dm_suspend+0xa1/0x2e0

[ 2162.544705] [<ffffffff8149ced9>] ? __get_name_cell+0x99/0xb0

[ 2162.551174] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0

[ 2162.557160] [<ffffffff8149e29b>] do_resume+0x17b/0x1b0

[ 2162.563082] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0

[ 2162.569102] [<ffffffff8149e365>] dev_suspend+0x95/0xb0

[ 2162.574987] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0

[ 2162.581068] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240

[ 2162.586954] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20

[ 2162.593217] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20

[ 2162.599190] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0

[ 2162.605298] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0

[ 2162.610990] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b

[ 2191.336354] Uhhuh. NMI received for unknown reason 21 on CPU 0.

[ 2191.343064] Do you have a strange power saving mode enabled?

[ 2191.349476] Kernel panic - not syncing: NMI: Not continuing

[ 2191.355753] Pid: 0, comm: swapper Not tainted 2.6.38-8-server #43

[ 2191.362593] Call Trace:

[ 2191.365380] <NMI> [<ffffffff815d2083>] ? panic+0x91/0x19e

[ 2191.371779] [<ffffffff815d21f8>] ? printk+0x68/0x70

[ 2191.377381] [<ffffffff815d6333>] ? default_do_nmi+0x1f3/0x200

[ 2191.383929] [<ffffffff815d63c0>] ? do_nmi+0x80/0x90

[ 2191.389526] [<ffffffff815d5b50>] ? nmi+0x20/0x30

[ 2191.394816] [<ffffffff81332d74>] ? intel_idle+0x94/0x120

[ 2191.400897] <<EOE>> [<ffffffff814b3472>] ? cpuidle_idle_call+0xb2/0x1b0

[ 2191.408606] [<ffffffff8100b067>] ? cpu_idle+0xb7/0x110

[ 2191.414497] [<ffffffff815b7682>] ? rest_init+0x72/0x80

[ 2191.420367] [<ffffffff81ae2c95>] ? start_kernel+0x374/0x37b

[ 2191.426780] [<ffffffff81ae2346>] ? x86_64_start_reservations+0x131/0x135

[ 2191.434457] [<ffffffff81ae244d>] ? x86_64_start_kernel+0x103/0x112

Thanks.

Peter

>
> Thanks,
> Toshiyuki Okajima
> ---
> fs/ext3/file.c | 19 ++++++++++++-
> fs/ext3/inode.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/ext4/inode.c | 4 ++-
> include/linux/ext3_fs.h | 1 +
> 4 files changed, 93 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext3/file.c b/fs/ext3/file.c
> index f55df0e..6d376ef 100644
> --- a/fs/ext3/file.c
> +++ b/fs/ext3/file.c
> @@ -52,6 +52,23 @@ static int ext3_release_file (struct inode * inode, struct file * filp)
> return 0;
> }
>
> +static const struct vm_operations_struct ext3_file_vm_ops = {
> + .fault = filemap_fault,
> + .page_mkwrite = ext3_page_mkwrite,
> +};
> +
> +static int ext3_file_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct address_space *mapping = file->f_mapping;
> +
> + if (!mapping->a_ops->readpage)
> + return -ENOEXEC;
> + file_accessed(file);
> + vma->vm_ops = &ext3_file_vm_ops;
> + vma->vm_flags |= VM_CAN_NONLINEAR;
> + return 0;
> +}
> +
> const struct file_operations ext3_file_operations = {
> .llseek = generic_file_llseek,
> .read = do_sync_read,
> @@ -62,7 +79,7 @@ const struct file_operations ext3_file_operations = {
> #ifdef CONFIG_COMPAT
> .compat_ioctl = ext3_compat_ioctl,
> #endif
> - .mmap = generic_file_mmap,
> + .mmap = ext3_file_mmap,
> .open = dquot_file_open,
> .release = ext3_release_file,
> .fsync = ext3_sync_file,
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 68b2e43..66c31dd 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -3496,3 +3496,74 @@ int ext3_change_inode_journal_flag(struct inode *inode, int val)
>
> return err;
> }
> +
> +int ext3_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> +{
> + struct page *page = vmf->page;
> + loff_t size;
> + unsigned long len;
> + int ret = -EINVAL;
> + void *fsdata;
> + struct file *file = vma->vm_file;
> + struct inode *inode = file->f_path.dentry->d_inode;
> + struct address_space *mapping = inode->i_mapping;
> +
> + /*
> + * Get i_alloc_sem to stop truncates messing with the inode. We cannot
> + * get i_mutex because we are already holding mmap_sem.
> + */
> + down_read(&inode->i_alloc_sem);
> + size = i_size_read(inode);
> + if (page->mapping != mapping || size <= page_offset(page)
> + || !PageUptodate(page)) {
> + /* page got truncated from under us? */
> + goto out_unlock;
> + }
> + ret = 0;
> + if (PageMappedToDisk(page))
> + goto out_frozen;
> +
> + if (page->index == size >> PAGE_CACHE_SHIFT)
> + len = size & ~PAGE_CACHE_MASK;
> + else
> + len = PAGE_CACHE_SIZE;
> +
> + lock_page(page);
> + /*
> + * return if we have all the buffers mapped. This avoid
> + * the need to call write_begin/write_end which does a
> + * journal_start/journal_stop which can block and take
> + * long time
> + */
> + if (page_has_buffers(page)) {
> + if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
> + buffer_unmapped)) {
> + unlock_page(page);
> +out_frozen:
> + vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
> + goto out_unlock;
> + }
> + }
> + unlock_page(page);
> + /*
> + * OK, we need to fill the hole... Do write_begin write_end
> + * to do block allocation/reservation.We are not holding
> + * inode.i__mutex here. That allow * parallel write_begin,
> + * write_end call. lock_page prevent this from happening
> + * on the same page though
> + */
> + ret = mapping->a_ops->write_begin(file, mapping, page_offset(page),
> + len, AOP_FLAG_UNINTERRUPTIBLE, &page, &fsdata);
> + if (ret < 0)
> + goto out_unlock;
> + ret = mapping->a_ops->write_end(file, mapping, page_offset(page),
> + len, len, page, fsdata);
> + if (ret < 0)
> + goto out_unlock;
> + ret = 0;
> +out_unlock:
> + if (ret)
> + ret = VM_FAULT_SIGBUS;
> + up_read(&inode->i_alloc_sem);
> + return ret;
> +}
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index f2fa5e8..44979ae 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5812,7 +5812,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> }
> ret = 0;
> if (PageMappedToDisk(page))
> - goto out_unlock;
> + goto out_frozen;
>
> if (page->index == size >> PAGE_CACHE_SHIFT)
> len = size & ~PAGE_CACHE_MASK;
> @@ -5830,6 +5830,8 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
> ext4_bh_unmapped)) {
> unlock_page(page);
> +out_frozen:
> + vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
> goto out_unlock;
> }
> }
> diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
> index 85c1d30..a0e39ca 100644
> --- a/include/linux/ext3_fs.h
> +++ b/include/linux/ext3_fs.h
> @@ -919,6 +919,7 @@ extern void ext3_get_inode_flags(struct ext3_inode_info *);
> extern void ext3_set_aops(struct inode *inode);
> extern int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> u64 start, u64 len);
> +extern int ext3_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);
>
> /* ioctl.c */
> extern long ext3_ioctl(struct file *, unsigned int, unsigned long);

2011-04-22 21:40:49

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

Hello,

On Fri 22-04-11 17:26:07, Peter M. Petrakis wrote:
> On 04/22/2011 02:58 AM, Toshiyuki Okajima wrote:
> > On Tue, 19 Apr 2011 18:43:16 +0900
> > I have confirmed that the following patch works fine while my or
> > Mizuma-san's reproducer is running. Therefore,
> > we can block to write the data, which is mmapped to a file, into a disk
> > by a page-fault while fsfreezing.
> >
> > I think this patch fixes the following two problems:
> > - A deadlock occurs between ext4_da_writepages() (called from
> > writeback_inodes_wb) and thaw_super(). (reported by Mizuma-san)
> > - We can also write the data, which is mmapped to a file,
> > into a disk while fsfreezing (ext3/ext4).
> > (reported by me)
> >
> > Please examine this patch.
>
> We've recently identified the same root cause in 2.6.32 though the hit rate
> is much much higher. The configuration is a SAN ALUA Active/Standby using
> multipath. The s_wait_unfrozen/s_umount deadlock is regularly encountered
> when a path comes back into service, as a result of a kpartx invocation on
> behalf of this udev rule.
>
> /lib/udev/rules.d/95-kpartx.rules
>
> # Create dm tables for partitions
> ENV{DM_STATE}=="ACTIVE", ENV{DM_UUID}=="mpath-*", \
> RUN+="/sbin/dmsetup ls --target multipath --exec '/sbin/kpartx -a -p -part' -j %M -m %m"
Hmm, I don't think this is the same problem... See:

> [ 1898.017614] mptsas: ioc0: mptsas_add_fw_event: add (fw_event=0xffff880c3c815200)
> [ 1898.025995] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c814780)
> [ 1898.034625] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c814b40), event = (0x12)
> [ 1898.044803] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c814b40)
> [ 1898.053475] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c815c80), event = (0x12)
> [ 1898.063690] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c815c80)
> [ 1898.072316] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c815200), event = (0x0f)
> [ 1898.082544] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c815200)
> [ 1898.571426] sd 0:0:1:0: alua: port group 01 state S supports toluSnA
> [ 1898.578635] device-mapper: multipath: Failing path 8:32.
> [ 2041.345645] INFO: task kjournald:595 blocked for more than 120 seconds.
> [ 2041.353075] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 2041.361891] kjournald D ffff88063acb9a90 0 595 2 0x00000000
> [ 2041.369891] ffff88063ace1c30 0000000000000046 ffff88063c282140 ffff880600000000
> [ 2041.378416] 0000000000013cc0 ffff88063acb96e0 ffff88063acb9a90 ffff88063ace1fd8
> [ 2041.386954] ffff88063acb9a98 0000000000013cc0 ffff88063ace0010 0000000000013cc0
>
> [ 2041.395561] Call Trace:
> [ 2041.398358] [<ffffffff81192380>] ? sync_buffer+0x0/0x50
> [ 2041.404342] [<ffffffff815d3120>] io_schedule+0x70/0xc0
> [ 2041.410227] [<ffffffff811923c5>] sync_buffer+0x45/0x50
> [ 2041.416179] [<ffffffff815d378f>] __wait_on_bit+0x5f/0x90
> [ 2041.422258] [<ffffffff81192380>] ? sync_buffer+0x0/0x50
> [ 2041.428275] [<ffffffff815d3838>] out_of_line_wait_on_bit+0x78/0x90
> [ 2041.435324] [<ffffffff81086b90>] ? wake_bit_function+0x0/0x40
> [ 2041.441958] [<ffffffff8119237e>] __wait_on_buffer+0x2e/0x30
> [ 2041.448333] [<ffffffff8123ab14>] journal_commit_transaction+0x7e4/0xec0
So kjournald is committing a transaction and waiting for IO to complete.
Which maybe never happens because of multipath being in transition? That
would be a bug...

> [ 2041.670669] multipathd D ffff88063e3303b0 0 1337 1 0x00000000
> [ 2041.678746] ffff88063c0fda18 0000000000000082 0000000000000000 ffff880600000000
> [ 2041.687219] 0000000000013cc0 ffff88063e330000 ffff88063e3303b0 ffff88063c0fdfd8
> [ 2041.695818] ffff88063e3303b8 0000000000013cc0 ffff88063c0fc010 0000000000013cc0
> [ 2041.704369] Call Trace:
> [ 2041.707128] [<ffffffff815d349d>] schedule_timeout+0x21d/0x300
> [ 2041.713679] [<ffffffff8104c8ec>] ? resched_task+0x2c/0x90
> [ 2041.719846] [<ffffffff8105f763>] ? try_to_wake_up+0xc3/0x410
> [ 2041.726301] [<ffffffff815d2436>] wait_for_common+0xd6/0x180
> [ 2041.732685] [<ffffffff8105fb05>] ? wake_up_process+0x15/0x20
> [ 2041.739138] [<ffffffff8105fab0>] ? default_wake_function+0x0/0x20
> [ 2041.746079] [<ffffffff815d25bd>] wait_for_completion+0x1d/0x20
> [ 2041.752716] [<ffffffff8107de18>] call_usermodehelper_exec+0xd8/0xe0
> [ 2041.759853] [<ffffffff814a3110>] ? parse_hw_handler+0xb0/0x240
> [ 2041.766503] [<ffffffff8107e060>] __request_module+0x190/0x210
> [ 2041.773054] [<ffffffff812e0c28>] ? sscanf+0x38/0x40
> [ 2041.778636] [<ffffffff814a3110>] parse_hw_handler+0xb0/0x240
> [ 2041.785121] [<ffffffff814a38c3>] multipath_ctr+0x83/0x1d0
> [ 2041.791312] [<ffffffff8149abd5>] ? dm_split_args+0x75/0x140
> [ 2041.797671] [<ffffffff8149b9af>] dm_table_add_target+0xff/0x250
> [ 2041.804413] [<ffffffff8149de3a>] table_load+0xca/0x2f0
> [ 2041.810317] [<ffffffff8149dd70>] ? table_load+0x0/0x2f0
> [ 2041.816316] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240
> [ 2041.822184] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20
> [ 2041.828188] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0
> [ 2041.834250] [<ffffffff8109ae6b>] ? sys_futex+0x7b/0x170
> [ 2041.840219] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0
> [ 2041.845898] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b
multipathd is hung waiting for module to be loaded? How come?

> [ 2041.964575] INFO: task kpartx:1897 blocked for more than 120 seconds.
> [ 2041.971801] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 2041.980626] kpartx D ffff88063d05df30 0 1897 1896 0x00000000
> [ 2041.988607] ffff88063c3a5b58 0000000000000082 0000000e3c3a5ac8 ffff880c00000000
> [ 2041.997056] 0000000000013cc0 ffff88063d05db80 ffff88063d05df30 ffff88063c3a5fd8
> [ 2042.005496] ffff88063d05df38 0000000000013cc0 ffff88063c3a4010 0000000000013cc0
> [ 2042.013939] Call Trace:
> [ 2042.016702] [<ffffffff8123dc85>] log_wait_commit+0xc5/0x150
> [ 2042.023089] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40
> [ 2042.030321] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20
> [ 2042.036584] [<ffffffff811e6256>] ext3_sync_fs+0x66/0x70
> [ 2042.042552] [<ffffffff811ba7c1>] dquot_quota_sync+0x1c1/0x330
> [ 2042.049133] [<ffffffff81115391>] ? do_writepages+0x21/0x40
> [ 2042.055423] [<ffffffff8110ae8b>] ? __filemap_fdatawrite_range+0x5b/0x60
> [ 2042.062944] [<ffffffff8118f42c>] __sync_filesystem+0x3c/0x90
> [ 2042.069430] [<ffffffff8118f56b>] sync_filesystem+0x4b/0x70
> [ 2042.075690] [<ffffffff81166a85>] freeze_super+0x55/0x100
> [ 2042.081754] [<ffffffff811993b8>] freeze_bdev+0x98/0xe0
> [ 2042.087625] [<ffffffff81499001>] dm_suspend+0xa1/0x2e0
> [ 2042.093495] [<ffffffff8149ced9>] ? __get_name_cell+0x99/0xb0
> [ 2042.099948] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0
> [ 2042.105916] [<ffffffff8149e29b>] do_resume+0x17b/0x1b0
> [ 2042.111784] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0
> [ 2042.117753] [<ffffffff8149e365>] dev_suspend+0x95/0xb0
> [ 2042.123621] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0
> [ 2042.129591] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240
> [ 2042.135493] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20
> [ 2042.141770] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20
> [ 2042.147739] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0
> [ 2042.153801] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0
> [ 2042.159478] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b
kpartx is waiting for kjournald to finish transaction commit and it is
holding s_umount but that doesn't really seem to be a problem...

So as I say, find a reason why kjournald is not able to finish committing a
transaction and you should solve this riddle ;).

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-22 22:10:28

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Fri 22-04-11 15:58:39, Toshiyuki Okajima wrote:
> I have confirmed that the following patch works fine while my or
> Mizuma-san's reproducer is running. Therefore,
> we can block to write the data, which is mmapped to a file, into a disk
> by a page-fault while fsfreezing.
>
> I think this patch fixes the following two problems:
> - A deadlock occurs between ext4_da_writepages() (called from
> writeback_inodes_wb) and thaw_super(). (reported by Mizuma-san)
> - We can also write the data, which is mmapped to a file,
> into a disk while fsfreezing (ext3/ext4).
> (reported by me)
>
> Please examine this patch.
Thanks for the patch. The ext3 part is not as easy as this. You cannot
really get i_alloc_sem in ext3_page_mkwrite() because mmap_sem is already
held by page fault code and i_alloc_sem should be acquired before it (yes I
know, ext4 already has this bug which should be fixed when I get to it).
Also you'll find that performance of random writers via mmap (which is
relatively common) is going to be rather bad with this patch (because the
file will be heavily fragmented). We have to be more clever which is
exactly why it's taking me so long with my patch :) But tests are already
running so if everything goes fine, I should have patches to submit next
week.

The ext4 part looks correct. I'd just also like to have some comments about
how freeze handling is done because it's kind of subtle.

Honza

> diff --git a/fs/ext3/file.c b/fs/ext3/file.c
> index f55df0e..6d376ef 100644
> --- a/fs/ext3/file.c
> +++ b/fs/ext3/file.c
> @@ -52,6 +52,23 @@ static int ext3_release_file (struct inode * inode, struct file * filp)
> return 0;
> }
>
> +static const struct vm_operations_struct ext3_file_vm_ops = {
> + .fault = filemap_fault,
> + .page_mkwrite = ext3_page_mkwrite,
> +};
> +
> +static int ext3_file_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct address_space *mapping = file->f_mapping;
> +
> + if (!mapping->a_ops->readpage)
> + return -ENOEXEC;
> + file_accessed(file);
> + vma->vm_ops = &ext3_file_vm_ops;
> + vma->vm_flags |= VM_CAN_NONLINEAR;
> + return 0;
> +}
> +
> const struct file_operations ext3_file_operations = {
> .llseek = generic_file_llseek,
> .read = do_sync_read,
> @@ -62,7 +79,7 @@ const struct file_operations ext3_file_operations = {
> #ifdef CONFIG_COMPAT
> .compat_ioctl = ext3_compat_ioctl,
> #endif
> - .mmap = generic_file_mmap,
> + .mmap = ext3_file_mmap,
> .open = dquot_file_open,
> .release = ext3_release_file,
> .fsync = ext3_sync_file,
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 68b2e43..66c31dd 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -3496,3 +3496,74 @@ int ext3_change_inode_journal_flag(struct inode *inode, int val)
>
> return err;
> }
> +
> +int ext3_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> +{
> + struct page *page = vmf->page;
> + loff_t size;
> + unsigned long len;
> + int ret = -EINVAL;
> + void *fsdata;
> + struct file *file = vma->vm_file;
> + struct inode *inode = file->f_path.dentry->d_inode;
> + struct address_space *mapping = inode->i_mapping;
> +
> + /*
> + * Get i_alloc_sem to stop truncates messing with the inode. We cannot
> + * get i_mutex because we are already holding mmap_sem.
> + */
> + down_read(&inode->i_alloc_sem);
> + size = i_size_read(inode);
> + if (page->mapping != mapping || size <= page_offset(page)
> + || !PageUptodate(page)) {
> + /* page got truncated from under us? */
> + goto out_unlock;
> + }
> + ret = 0;
> + if (PageMappedToDisk(page))
> + goto out_frozen;
> +
> + if (page->index == size >> PAGE_CACHE_SHIFT)
> + len = size & ~PAGE_CACHE_MASK;
> + else
> + len = PAGE_CACHE_SIZE;
> +
> + lock_page(page);
> + /*
> + * return if we have all the buffers mapped. This avoid
> + * the need to call write_begin/write_end which does a
> + * journal_start/journal_stop which can block and take
> + * long time
> + */
> + if (page_has_buffers(page)) {
> + if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
> + buffer_unmapped)) {
> + unlock_page(page);
> +out_frozen:
> + vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
> + goto out_unlock;
> + }
> + }
> + unlock_page(page);
> + /*
> + * OK, we need to fill the hole... Do write_begin write_end
> + * to do block allocation/reservation.We are not holding
> + * inode.i__mutex here. That allow * parallel write_begin,
> + * write_end call. lock_page prevent this from happening
> + * on the same page though
> + */
> + ret = mapping->a_ops->write_begin(file, mapping, page_offset(page),
> + len, AOP_FLAG_UNINTERRUPTIBLE, &page, &fsdata);
> + if (ret < 0)
> + goto out_unlock;
> + ret = mapping->a_ops->write_end(file, mapping, page_offset(page),
> + len, len, page, fsdata);
> + if (ret < 0)
> + goto out_unlock;
> + ret = 0;
> +out_unlock:
> + if (ret)
> + ret = VM_FAULT_SIGBUS;
> + up_read(&inode->i_alloc_sem);
> + return ret;
> +}
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index f2fa5e8..44979ae 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5812,7 +5812,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> }
> ret = 0;
> if (PageMappedToDisk(page))
> - goto out_unlock;
> + goto out_frozen;
>
> if (page->index == size >> PAGE_CACHE_SHIFT)
> len = size & ~PAGE_CACHE_MASK;
> @@ -5830,6 +5830,8 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL,
> ext4_bh_unmapped)) {
> unlock_page(page);
> +out_frozen:
> + vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
> goto out_unlock;
> }
> }
> diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
> index 85c1d30..a0e39ca 100644
> --- a/include/linux/ext3_fs.h
> +++ b/include/linux/ext3_fs.h
> @@ -919,6 +919,7 @@ extern void ext3_get_inode_flags(struct ext3_inode_info *);
> extern void ext3_set_aops(struct inode *inode);
> extern int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> u64 start, u64 len);
> +extern int ext3_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);
>
> /* ioctl.c */
> extern long ext3_ioctl(struct file *, unsigned int, unsigned long);
> --
> 1.5.5.6
--
Jan Kara <[email protected]>
SUSE Labs, CR

2011-04-22 22:57:51

by Peter M. Petrakis

[permalink] [raw]

Subject: Re: [RFC][PATCH] Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 04/22/2011 05:40 PM, Jan Kara wrote:
> Hello,
>
> On Fri 22-04-11 17:26:07, Peter M. Petrakis wrote:
>> On 04/22/2011 02:58 AM, Toshiyuki Okajima wrote:
>>> On Tue, 19 Apr 2011 18:43:16 +0900
>>> I have confirmed that the following patch works fine while my or
>>> Mizuma-san's reproducer is running. Therefore,
>>> we can block to write the data, which is mmapped to a file, into a disk
>>> by a page-fault while fsfreezing.
>>>
>>> I think this patch fixes the following two problems:
>>> - A deadlock occurs between ext4_da_writepages() (called from
>>> writeback_inodes_wb) and thaw_super(). (reported by Mizuma-san)
>>> - We can also write the data, which is mmapped to a file,
>>> into a disk while fsfreezing (ext3/ext4).
>>> (reported by me)
>>>
>>> Please examine this patch.
>>
>> We've recently identified the same root cause in 2.6.32 though the hit rate
>> is much much higher. The configuration is a SAN ALUA Active/Standby using
>> multipath. The s_wait_unfrozen/s_umount deadlock is regularly encountered
>> when a path comes back into service, as a result of a kpartx invocation on
>> behalf of this udev rule.
>>
>> /lib/udev/rules.d/95-kpartx.rules
>>
>> # Create dm tables for partitions
>> ENV{DM_STATE}=="ACTIVE", ENV{DM_UUID}=="mpath-*", \
>> RUN+="/sbin/dmsetup ls --target multipath --exec '/sbin/kpartx -a -p -part' -j %M -m %m"
> Hmm, I don't think this is the same problem... See:

Figures :)

>> [ 1898.017614] mptsas: ioc0: mptsas_add_fw_event: add (fw_event=0xffff880c3c815200)
>> [ 1898.025995] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c814780)
>> [ 1898.034625] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c814b40), event = (0x12)
>> [ 1898.044803] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c814b40)
>> [ 1898.053475] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c815c80), event = (0x12)
>> [ 1898.063690] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c815c80)
>> [ 1898.072316] mptsas: ioc0: mptsas_firmware_event_work: fw_event=(0xffff880c3c815200), event = (0x0f)
>> [ 1898.082544] mptsas: ioc0: mptsas_free_fw_event: kfree (fw_event=0xffff880c3c815200)
>> [ 1898.571426] sd 0:0:1:0: alua: port group 01 state S supports toluSnA
>> [ 1898.578635] device-mapper: multipath: Failing path 8:32.
>> [ 2041.345645] INFO: task kjournald:595 blocked for more than 120 seconds.
>> [ 2041.353075] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [ 2041.361891] kjournald D ffff88063acb9a90 0 595 2 0x00000000
>> [ 2041.369891] ffff88063ace1c30 0000000000000046 ffff88063c282140 ffff880600000000
>> [ 2041.378416] 0000000000013cc0 ffff88063acb96e0 ffff88063acb9a90 ffff88063ace1fd8
>> [ 2041.386954] ffff88063acb9a98 0000000000013cc0 ffff88063ace0010 0000000000013cc0
>>
>> [ 2041.395561] Call Trace:
>> [ 2041.398358] [<ffffffff81192380>] ? sync_buffer+0x0/0x50
>> [ 2041.404342] [<ffffffff815d3120>] io_schedule+0x70/0xc0
>> [ 2041.410227] [<ffffffff811923c5>] sync_buffer+0x45/0x50
>> [ 2041.416179] [<ffffffff815d378f>] __wait_on_bit+0x5f/0x90
>> [ 2041.422258] [<ffffffff81192380>] ? sync_buffer+0x0/0x50
>> [ 2041.428275] [<ffffffff815d3838>] out_of_line_wait_on_bit+0x78/0x90
>> [ 2041.435324] [<ffffffff81086b90>] ? wake_bit_function+0x0/0x40
>> [ 2041.441958] [<ffffffff8119237e>] __wait_on_buffer+0x2e/0x30
>> [ 2041.448333] [<ffffffff8123ab14>] journal_commit_transaction+0x7e4/0xec0
> So kjournald is committing a transaction and waiting for IO to complete.
> Which maybe never happens because of multipath being in transition? That
> would be a bug...
>

and it would be a new one for us. It's entirely possible the original deadlock
is resolved, and this is new. With only the tracebacks to consult, and general
unfamiliarity with this area, it looked like the same fault to me.
In 2.6.32 it's a dead ringer per the thread parent:

http://permalink.gmane.org/gmane.comp.file-systems.ext4/23171

[Ubuntu 10.04 - 2.6.32 crashdump]

crash-5.0> ps | grep UN
992 2 7 ffff8802678a8000 UN 0.0 0 0 [flush-251:5]
17295 2537 2 ffff880267be0000 UN 0.2 47060 17368 iozone
17314 2477 5 ffff88026a952010 UN 0.2 47060 17364 iozone
17447 2573 0 ffff880268bd2010 UN 0.2 47060 17340 iozone
17460 1 13 ffff88026b3c4020 UN 0.0 191564 1992 rsyslogd
17606 17597 15 ffff880268420000 UN 0.0 10436 808 kpartx
17738 2268 13 ffff88016908a010 UN 0.0 17756 1616 dhclient-script
17747 2223 15 ffff88026a950000 UN 0.0 151460 1596 multipathd
17748 2284 1 ffff88016908c020 UN 0.0 49260 688 sshd
17749 2284 1 ffff880169088000 UN 0.0 49260 692 sshd
17750 2284 1 ffff88016a628000 UN 0.0 49260 688 sshd
17751 2284 0 ffff88026a3cc020 UN 0.0 49260 688 sshd
17752 2284 0 ffff88026a3ca010 UN 0.0 49260 688 sshd
17753 2284 0 ffff88026a3c8000 UN 0.0 49260 688 sshd
17754 2284 0 ffff880268f60000 UN 0.0 49260 692 sshd
17755 2284 0 ffff880268f62010 UN 0.0 49260 688 sshd
crash-5.0> bt 17606
PID: 17606 TASK: ffff880268420000 CPU: 15 COMMAND: "kpartx"
#0 [ffff88026aac3b18] schedule at ffffffff8158bcbd
#1 [ffff88026aac3bd0] rwsem_down_failed_common at ffffffff8158df2d
#2 [ffff88026aac3c30] rwsem_down_write_failed at ffffffff8158e0b3
#3 [ffff88026aac3c70] call_rwsem_down_write_failed at ffffffff812d9903
#4 [ffff88026aac3ce0] thaw_bdev at ffffffff81186d5a
#5 [ffff88026aac3d40] unlock_fs at ffffffff8145e46d
#6 [ffff88026aac3d60] dm_resume at ffffffff8145fb38
#7 [ffff88026aac3db0] do_resume at ffffffff81465c98
#8 [ffff88026aac3de0] dev_suspend at ffffffff81465d65
#9 [ffff88026aac3e20] ctl_ioctl at ffffffff814665f5
#10 [ffff88026aac3e90] dm_ctl_ioctl at ffffffff814666a3
#11 [ffff88026aac3ea0] vfs_ioctl at ffffffff81165e92
#12 [ffff88026aac3ee0] do_vfs_ioctl at ffffffff81166140
#13 [ffff88026aac3f30] sys_ioctl at ffffffff811664b1
#14 [ffff88026aac3f80] system_call_fastpath at ffffffff810131b2
RIP: 00007fa798b04197 RSP: 00007fff4cf1c6e8 RFLAGS: 00010202
RAX: 0000000000000010 RBX: ffffffff810131b2 RCX: 0000000000000000
RDX: 0000000000bcf310 RSI: 00000000c138fd06 RDI: 0000000000000004
RBP: 0000000000bcf340 R8: 00007fa798dc2528 R9: 00007fff4cf1c640
R10: 00007fa798dc1dc0 R11: 0000000000000246 R12: 00007fa798dc1dc0
R13: 0000000000004000 R14: 0000000000bce0f0 R15: 00007fa798dc1dc0
ORIG_RAX: 0000000000000010 CS: 0033 SS: 002b
crash-5.0> bt 992

PID: 992 TASK: ffff8802678a8000 CPU: 7 COMMAND: "flush-251:5"
#0 [ffff880267bddb00] schedule at ffffffff8158bcbd
#1 [ffff880267bddbb8] ext4_force_commit at ffffffff8120b16d
#2 [ffff880267bddc18] ext4_write_inode at ffffffff811f29e5
#3 [ffff880267bddc68] writeback_single_inode at ffffffff81178964
#4 [ffff880267bddcb8] writeback_sb_inodes at ffffffff81178f09
#5 [ffff880267bddd18] wb_writeback at ffffffff8117995c
#6 [ffff880267bdddc8] wb_do_writeback at ffffffff81179b6b
#7 [ffff880267bdde58] bdi_writeback_task at ffffffff81179cc3
#8 [ffff880267bdde98] bdi_start_fn at ffffffff8111e816
#9 [ffff880267bddec8] kthread at ffffffff81088a06
#10 [ffff880267bddf48] kernel_thread at ffffffff810142ea

crash-5.0> super_block.s_frozen ffff880268a4e000
s_frozen = 0x2,

int ext4_force_commit(struct super_block *sb)
{
journal_t *journal;
int ret = 0;

if (sb->s_flags & MS_RDONLY)
return 0;

journal = EXT4_SB(sb)->s_journal;
if (journal) {
vfs_check_frozen(sb, SB_FREEZE_TRANS); <=== this is where sleep
ret = ext4_journal_force_commit(journal);
}

return ret;
}

I have tried the previous versions of the patch, backporting
to 2.6.32 without any success. I thought I would just go for it
this time with the latest.

>> [ 2041.670669] multipathd D ffff88063e3303b0 0 1337 1 0x00000000
>> [ 2041.678746] ffff88063c0fda18 0000000000000082 0000000000000000 ffff880600000000
>> [ 2041.687219] 0000000000013cc0 ffff88063e330000 ffff88063e3303b0 ffff88063c0fdfd8
>> [ 2041.695818] ffff88063e3303b8 0000000000013cc0 ffff88063c0fc010 0000000000013cc0
>> [ 2041.704369] Call Trace:
>> [ 2041.707128] [<ffffffff815d349d>] schedule_timeout+0x21d/0x300
>> [ 2041.713679] [<ffffffff8104c8ec>] ? resched_task+0x2c/0x90
>> [ 2041.719846] [<ffffffff8105f763>] ? try_to_wake_up+0xc3/0x410
>> [ 2041.726301] [<ffffffff815d2436>] wait_for_common+0xd6/0x180
>> [ 2041.732685] [<ffffffff8105fb05>] ? wake_up_process+0x15/0x20
>> [ 2041.739138] [<ffffffff8105fab0>] ? default_wake_function+0x0/0x20
>> [ 2041.746079] [<ffffffff815d25bd>] wait_for_completion+0x1d/0x20
>> [ 2041.752716] [<ffffffff8107de18>] call_usermodehelper_exec+0xd8/0xe0
>> [ 2041.759853] [<ffffffff814a3110>] ? parse_hw_handler+0xb0/0x240
>> [ 2041.766503] [<ffffffff8107e060>] __request_module+0x190/0x210
>> [ 2041.773054] [<ffffffff812e0c28>] ? sscanf+0x38/0x40
>> [ 2041.778636] [<ffffffff814a3110>] parse_hw_handler+0xb0/0x240
>> [ 2041.785121] [<ffffffff814a38c3>] multipath_ctr+0x83/0x1d0
>> [ 2041.791312] [<ffffffff8149abd5>] ? dm_split_args+0x75/0x140
>> [ 2041.797671] [<ffffffff8149b9af>] dm_table_add_target+0xff/0x250
>> [ 2041.804413] [<ffffffff8149de3a>] table_load+0xca/0x2f0
>> [ 2041.810317] [<ffffffff8149dd70>] ? table_load+0x0/0x2f0
>> [ 2041.816316] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240
>> [ 2041.822184] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20
>> [ 2041.828188] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0
>> [ 2041.834250] [<ffffffff8109ae6b>] ? sys_futex+0x7b/0x170
>> [ 2041.840219] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0
>> [ 2041.845898] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b
> multipathd is hung waiting for module to be loaded? How come?

It shouldn't, dh_alua is already loaded.

>> [ 2041.964575] INFO: task kpartx:1897 blocked for more than 120 seconds.
>> [ 2041.971801] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> [ 2041.980626] kpartx D ffff88063d05df30 0 1897 1896 0x00000000
>> [ 2041.988607] ffff88063c3a5b58 0000000000000082 0000000e3c3a5ac8 ffff880c00000000
>> [ 2041.997056] 0000000000013cc0 ffff88063d05db80 ffff88063d05df30 ffff88063c3a5fd8
>> [ 2042.005496] ffff88063d05df38 0000000000013cc0 ffff88063c3a4010 0000000000013cc0
>> [ 2042.013939] Call Trace:
>> [ 2042.016702] [<ffffffff8123dc85>] log_wait_commit+0xc5/0x150
>> [ 2042.023089] [<ffffffff81086b50>] ? autoremove_wake_function+0x0/0x40
>> [ 2042.030321] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20
>> [ 2042.036584] [<ffffffff811e6256>] ext3_sync_fs+0x66/0x70
>> [ 2042.042552] [<ffffffff811ba7c1>] dquot_quota_sync+0x1c1/0x330
>> [ 2042.049133] [<ffffffff81115391>] ? do_writepages+0x21/0x40
>> [ 2042.055423] [<ffffffff8110ae8b>] ? __filemap_fdatawrite_range+0x5b/0x60
>> [ 2042.062944] [<ffffffff8118f42c>] __sync_filesystem+0x3c/0x90
>> [ 2042.069430] [<ffffffff8118f56b>] sync_filesystem+0x4b/0x70
>> [ 2042.075690] [<ffffffff81166a85>] freeze_super+0x55/0x100
>> [ 2042.081754] [<ffffffff811993b8>] freeze_bdev+0x98/0xe0
>> [ 2042.087625] [<ffffffff81499001>] dm_suspend+0xa1/0x2e0
>> [ 2042.093495] [<ffffffff8149ced9>] ? __get_name_cell+0x99/0xb0
>> [ 2042.099948] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0
>> [ 2042.105916] [<ffffffff8149e29b>] do_resume+0x17b/0x1b0
>> [ 2042.111784] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0
>> [ 2042.117753] [<ffffffff8149e365>] dev_suspend+0x95/0xb0
>> [ 2042.123621] [<ffffffff8149e2d0>] ? dev_suspend+0x0/0xb0
>> [ 2042.129591] [<ffffffff8149f0d5>] ctl_ioctl+0x1a5/0x240
>> [ 2042.135493] [<ffffffff815d4eee>] ? _raw_spin_lock+0xe/0x20
>> [ 2042.141770] [<ffffffff8149f183>] dm_ctl_ioctl+0x13/0x20
>> [ 2042.147739] [<ffffffff81175245>] do_vfs_ioctl+0x95/0x3c0
>> [ 2042.153801] [<ffffffff81175611>] sys_ioctl+0xa1/0xb0
>> [ 2042.159478] [<ffffffff8100c042>] system_call_fastpath+0x16/0x1b
> kpartx is waiting for kjournald to finish transaction commit and it is
> holding s_umount but that doesn't really seem to be a problem...
>
> So as I say, find a reason why kjournald is not able to finish committing a
> transaction and you should solve this riddle ;).

Cool, thanks!

Peter

>
> Honza

2011-04-25 08:03:33