* [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs
@ 2020-12-26 9:56 Shijie Luo
2020-12-26 15:55 ` Al Viro
0 siblings, 1 reply; 6+ messages in thread
From: Shijie Luo @ 2020-12-26 9:56 UTC (permalink / raw)
To: linux-fsdevel; +Cc: viro, yangerkun, yi.zhang, linfeilong, jack
We found a hungtask problem as described following:
Running xfstests generic/390 with ext4 filesystem, and simutaneously
offline/onlines the disk we tested. It will cause a hungtask problem
whose call trace is like this,
[369.857104] INFO: task fsstress:11672 blocked for more than 120 seconds.
[ 369.875724] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 369.885168] fsstress D 0 11672 11625 0x00000080
[ 369.885169] Call Trace:
[ 369.885171] ? __schedule+0x2fc/0x930
[ 369.885173] ? filename_parentat+0x10b/0x1a0
[ 369.885175] schedule+0x28/0x70
[ 369.885176] rwsem_down_read_failed+0x102/0x1c0
[ 369.885178] ? __percpu_down_read+0x93/0xb0
[ 369.885179] __percpu_down_read+0x93/0xb0
[ 369.885182] __sb_start_write+0x5f/0x70
[ 369.885183] mnt_want_write+0x20/0x50
[ 369.885184] do_renameat2+0x1f3/0x550
[ 369.885186] __x64_sys_rename+0x1c/0x20
[ 369.885187] do_syscall_64+0x5b/0x1b0
[ 369.885188] entry_SYSCALL_64_after_hwframe+0x65/0xca
[ 369.885189] RIP: 0033:0x7f5e6e34ccb7
[ 369.885190] Code: Bad RIP value.
[ 369.885191] RSP: 002b:00007ffef4a83788 EFLAGS: 00000206 ORIG_RAX: 0000000000000052
[ 369.885191] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5e6e34ccb7
[ 369.885192] RDX: 0000000000000000 RSI: 0000000001b09500 RDI: 0000000001b09f90
[ 369.885192] RBP: 0000000000000000 R08: 0000000000000021 R09: 0000000000000000
[ 369.885193] R10: 0000000000000692 R11: 0000000000000206 R12: 00007ffef4a83a30
[ 369.885193] R13: 00007ffef4a83a40 R14: 00007ffef4a83a40 R15: 0000000000000001
The root cause is that when offline/onlines disks, the filesystem can easily get into
a error state and this makes it change to be read-only. Function freeze_super() will hold
all sb_writers rwsems including rwsem of SB_FREEZE_WRITE when filesystem not read-only,
but thaw_super_locked() cannot release these while the filesystem suddenly become read-only,
because the logic will go to out.
freeze_super
hold sb_writers rwsems
sb->s_writers.frozen = SB_FREEZE_COMPLETE
thaw_super_locked
sb_rdonly
sb->s_writers.frozen = SB_UNFROZEN;
goto out // not release rwsems
And at this time, if we call mnt_want_write(), the process will be blocked.
This patch fixes this problem, when filesystem is read-only, just not to set sb_writers.frozen
be SB_FREEZE_COMPLETE in freeze_super() and then release all rwsems in thaw_super_locked.
Signed-off-by: Shijie Luo <luoshijie1@huawei.com>
Signed-off-by: yangerkun <yangerkun@huawei.com>
Fix some descriptions errors and resend the patch.
---
fs/super.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 2c6cdea2ab2d..50d79213f678 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1672,9 +1672,7 @@ int freeze_super(struct super_block *sb)
}
if (sb_rdonly(sb)) {
- /* Nothing to do really... */
- sb->s_writers.frozen = SB_FREEZE_COMPLETE;
- up_write(&sb->s_umount);
+ deactivate_locked_super(sb);
return 0;
}
@@ -1733,13 +1731,11 @@ static int thaw_super_locked(struct super_block *sb)
return -EINVAL;
}
- if (sb_rdonly(sb)) {
- sb->s_writers.frozen = SB_UNFROZEN;
- goto out;
- }
-
lockdep_sb_freeze_acquire(sb);
+ if (sb_rdonly(sb))
+ goto out;
+
if (sb->s_op->unfreeze_fs) {
error = sb->s_op->unfreeze_fs(sb);
if (error) {
@@ -1751,9 +1747,9 @@ static int thaw_super_locked(struct super_block *sb)
}
}
+out:
sb->s_writers.frozen = SB_UNFROZEN;
sb_freeze_unlock(sb);
-out:
wake_up(&sb->s_writers.wait_unfrozen);
deactivate_locked_super(sb);
return 0;
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs
2020-12-26 9:56 [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs Shijie Luo
@ 2020-12-26 15:55 ` Al Viro
2020-12-28 2:15 ` Shijie Luo
0 siblings, 1 reply; 6+ messages in thread
From: Al Viro @ 2020-12-26 15:55 UTC (permalink / raw)
To: Shijie Luo; +Cc: linux-fsdevel, yangerkun, yi.zhang, linfeilong, jack
On Sat, Dec 26, 2020 at 04:56:41AM -0500, Shijie Luo wrote:
> The root cause is that when offline/onlines disks, the filesystem can easily get into
> a error state and this makes it change to be read-only. Function freeze_super() will hold
> all sb_writers rwsems including rwsem of SB_FREEZE_WRITE when filesystem not read-only,
> but thaw_super_locked() cannot release these while the filesystem suddenly become read-only,
> because the logic will go to out.
>
> freeze_super
> hold sb_writers rwsems
> sb->s_writers.frozen = SB_FREEZE_COMPLETE
> thaw_super_locked
> sb_rdonly
> sb->s_writers.frozen = SB_UNFROZEN;
> goto out // not release rwsems
>
> And at this time, if we call mnt_want_write(), the process will be blocked.
>
> This patch fixes this problem, when filesystem is read-only, just not to set sb_writers.frozen
> be SB_FREEZE_COMPLETE in freeze_super() and then release all rwsems in thaw_super_locked.
I really don't like that - you end up with a case when freeze_super() returns 0 *and*
consumes the reference it had been give.
> if (sb_rdonly(sb)) {
> - /* Nothing to do really... */
> - sb->s_writers.frozen = SB_FREEZE_COMPLETE;
> - up_write(&sb->s_umount);
> + deactivate_locked_super(sb);
> return 0;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs
2020-12-26 15:55 ` Al Viro
@ 2020-12-28 2:15 ` Shijie Luo
2021-01-04 16:04 ` Jan Kara
0 siblings, 1 reply; 6+ messages in thread
From: Shijie Luo @ 2020-12-28 2:15 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, yangerkun, yi.zhang, linfeilong, jack
On 2020/12/26 23:55, Al Viro wrote:
> On Sat, Dec 26, 2020 at 04:56:41AM -0500, Shijie Luo wrote:
>
>> The root cause is that when offline/onlines disks, the filesystem can easily get into
>> a error state and this makes it change to be read-only. Function freeze_super() will hold
>> all sb_writers rwsems including rwsem of SB_FREEZE_WRITE when filesystem not read-only,
>> but thaw_super_locked() cannot release these while the filesystem suddenly become read-only,
>> because the logic will go to out.
>>
>> freeze_super
>> hold sb_writers rwsems
>> sb->s_writers.frozen = SB_FREEZE_COMPLETE
>> thaw_super_locked
>> sb_rdonly
>> sb->s_writers.frozen = SB_UNFROZEN;
>> goto out // not release rwsems
>>
>> And at this time, if we call mnt_want_write(), the process will be blocked.
>>
>> This patch fixes this problem, when filesystem is read-only, just not to set sb_writers.frozen
>> be SB_FREEZE_COMPLETE in freeze_super() and then release all rwsems in thaw_super_locked.
> I really don't like that - you end up with a case when freeze_super() returns 0 *and*
> consumes the reference it had been give.
Consuming the reference here because we won't "set frozen =
SB_FREEZE_COMPLETE" in thaw_super_locked() now.
If don't do that, we never have a chance to consume it,
thaw_super_locked() will directly return "-EINVAL". But I do
agree that it's not a good idea to return 0. How about returning
"-EINVAL or -ENOTSUPP" ?
And, If we keep "frozen = SB_FREEZE_COMPLETE" in freeze_super() here, it
will cause another problem, thaw_super_locked()
will always release rwsems in my logic, but freeze_super() won't acquire
the rwsems when filesystem is read-only.
Thanks.
>> if (sb_rdonly(sb)) {
>> - /* Nothing to do really... */
>> - sb->s_writers.frozen = SB_FREEZE_COMPLETE;
>> - up_write(&sb->s_umount);
>> + deactivate_locked_super(sb);
>> return 0;
>> }
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs
2020-12-28 2:15 ` Shijie Luo
@ 2021-01-04 16:04 ` Jan Kara
2021-01-05 2:48 ` Shijie Luo
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2021-01-04 16:04 UTC (permalink / raw)
To: Shijie Luo; +Cc: Al Viro, linux-fsdevel, yangerkun, yi.zhang, linfeilong, jack
[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]
On Mon 28-12-20 10:15:16, Shijie Luo wrote:
>
> On 2020/12/26 23:55, Al Viro wrote:
> > On Sat, Dec 26, 2020 at 04:56:41AM -0500, Shijie Luo wrote:
> >
> > > The root cause is that when offline/onlines disks, the filesystem can easily get into
> > > a error state and this makes it change to be read-only. Function freeze_super() will hold
> > > all sb_writers rwsems including rwsem of SB_FREEZE_WRITE when filesystem not read-only,
> > > but thaw_super_locked() cannot release these while the filesystem suddenly become read-only,
> > > because the logic will go to out.
> > >
> > > freeze_super
> > > hold sb_writers rwsems
> > > sb->s_writers.frozen = SB_FREEZE_COMPLETE
> > > thaw_super_locked
> > > sb_rdonly
> > > sb->s_writers.frozen = SB_UNFROZEN;
> > > goto out // not release rwsems
> > >
> > > And at this time, if we call mnt_want_write(), the process will be blocked.
> > >
> > > This patch fixes this problem, when filesystem is read-only, just not to set sb_writers.frozen
> > > be SB_FREEZE_COMPLETE in freeze_super() and then release all rwsems in thaw_super_locked.
> > I really don't like that - you end up with a case when freeze_super() returns 0 *and*
> > consumes the reference it had been give.
>
> Consuming the reference here because we won't "set frozen =
> SB_FREEZE_COMPLETE" in thaw_super_locked() now.
>
> If don't do that, we never have a chance to consume it, thaw_super_locked()
> will directly return "-EINVAL". But I do
>
> agree that it's not a good idea to return 0. How about returning "-EINVAL or
> -ENOTSUPP" ?
>
> And, If we keep "frozen = SB_FREEZE_COMPLETE" in freeze_super() here, it
> will cause another problem, thaw_super_locked()
>
> will always release rwsems in my logic, but freeze_super() won't acquire the
> rwsems when filesystem is read-only.
I was thinking about this for a while. I think the best solution would be
to track whether the fs was read only (and thus frozen without locking
percpu semaphores) at the time of freezing. I'm attaching that patch. Does
it fix your problem?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[-- Attachment #2: 0001-fs-fix-a-hungtask-problem-when-freeze-unfreeze-fs.patch --]
[-- Type: text/x-patch, Size: 2871 bytes --]
From f9df0208f3c9cdc973bd6a7ff86282bf31893352 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Mon, 4 Jan 2021 16:49:13 +0100
Subject: [PATCH] fs: fix a hungtask problem when freeze/unfreeze fs
We found the following deadlock when running xfstests generic/390 with ext4
filesystem, and simutaneously offlining/onlining the disk we tested. It will
cause a deadlock whose call trace is like this:
fsstress D 0 11672 11625 0x00000080
Call Trace:
? __schedule+0x2fc/0x930
? filename_parentat+0x10b/0x1a0
schedule+0x28/0x70
rwsem_down_read_failed+0x102/0x1c0
? __percpu_down_read+0x93/0xb0
__percpu_down_read+0x93/0xb0
__sb_start_write+0x5f/0x70
mnt_want_write+0x20/0x50
do_renameat2+0x1f3/0x550
__x64_sys_rename+0x1c/0x20
do_syscall_64+0x5b/0x1b0
entry_SYSCALL_64_after_hwframe+0x65/0xca
The root cause is that when ext4 hits IO error due to disk being
offline, it will switch itself into read-only state. When it is frozen
at that moment, following thaw_super() call will not unlock percpu
freeze semaphores (as the fs is read-only) causing the deadlock.
Fix the problem by tracking whether the superblock was read-only at the
time we were freezing it.
Reported-by: Shijie Luo <luoshijie1@huawei.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/super.c | 9 ++++++++-
include/linux/fs.h | 4 +++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 2c6cdea2ab2d..c35a2938ee99 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1674,10 +1674,12 @@ int freeze_super(struct super_block *sb)
if (sb_rdonly(sb)) {
/* Nothing to do really... */
sb->s_writers.frozen = SB_FREEZE_COMPLETE;
+ sb->s_writers.frozen_ro = 1;
up_write(&sb->s_umount);
return 0;
}
+ sb->s_writers.frozen_ro = 0;
sb->s_writers.frozen = SB_FREEZE_WRITE;
/* Release s_umount to preserve sb_start_write -> s_umount ordering */
up_write(&sb->s_umount);
@@ -1733,7 +1735,12 @@ static int thaw_super_locked(struct super_block *sb)
return -EINVAL;
}
- if (sb_rdonly(sb)) {
+ /*
+ * Was the fs frozen in read-only state? Note that we cannot just check
+ * sb_rdonly(sb) as the filesystem might have switched to read-only
+ * state due to internal errors or so.
+ */
+ if (sb->s_writers.frozen_ro) {
sb->s_writers.frozen = SB_UNFROZEN;
goto out;
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ad4cf1bae586..346ab981128f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1406,7 +1406,9 @@ enum {
#define SB_FREEZE_LEVELS (SB_FREEZE_COMPLETE - 1)
struct sb_writers {
- int frozen; /* Is sb frozen? */
+ unsigned short frozen; /* Is sb frozen? */
+ unsigned short frozen_ro; /* Was sb read-only
+ * when frozen? */
wait_queue_head_t wait_unfrozen; /* wait for thaw */
struct percpu_rw_semaphore rw_sem[SB_FREEZE_LEVELS];
};
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs
2021-01-04 16:04 ` Jan Kara
@ 2021-01-05 2:48 ` Shijie Luo
2021-01-05 13:46 ` Jan Kara
0 siblings, 1 reply; 6+ messages in thread
From: Shijie Luo @ 2021-01-05 2:48 UTC (permalink / raw)
To: Jan Kara; +Cc: Al Viro, linux-fsdevel, yangerkun, yi.zhang, linfeilong
Hi!
On 2021/1/5 0:04, Jan Kara wrote:
>> Consuming the reference here because we won't "set frozen =
>> SB_FREEZE_COMPLETE" in thaw_super_locked() now.
>>
>> If don't do that, we never have a chance to consume it, thaw_super_locked()
>> will directly return "-EINVAL". But I do
>>
>> agree that it's not a good idea to return 0. How about returning "-EINVAL or
>> -ENOTSUPP" ?
>>
>> And, If we keep "frozen = SB_FREEZE_COMPLETE" in freeze_super() here, it
>> will cause another problem, thaw_super_locked()
>>
>> will always release rwsems in my logic, but freeze_super() won't acquire the
>> rwsems when filesystem is read-only.
> I was thinking about this for a while. I think the best solution would be
> to track whether the fs was read only (and thus frozen without locking
> percpu semaphores) at the time of freezing. I'm attaching that patch. Does
> it fix your problem?
>
> Honza
I tested your patch, and it can indeed fix this deadlock problem.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs
2021-01-05 2:48 ` Shijie Luo
@ 2021-01-05 13:46 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2021-01-05 13:46 UTC (permalink / raw)
To: Shijie Luo
Cc: Jan Kara, Al Viro, linux-fsdevel, yangerkun, yi.zhang, linfeilong
On Tue 05-01-21 10:48:48, Shijie Luo wrote:
> Hi!
>
> On 2021/1/5 0:04, Jan Kara wrote:
> > > Consuming the reference here because we won't "set frozen =
> > > SB_FREEZE_COMPLETE" in thaw_super_locked() now.
> > >
> > > If don't do that, we never have a chance to consume it, thaw_super_locked()
> > > will directly return "-EINVAL". But I do
> > >
> > > agree that it's not a good idea to return 0. How about returning "-EINVAL or
> > > -ENOTSUPP" ?
> > >
> > > And, If we keep "frozen = SB_FREEZE_COMPLETE" in freeze_super() here, it
> > > will cause another problem, thaw_super_locked()
> > >
> > > will always release rwsems in my logic, but freeze_super() won't acquire the
> > > rwsems when filesystem is read-only.
> > I was thinking about this for a while. I think the best solution would be
> > to track whether the fs was read only (and thus frozen without locking
> > percpu semaphores) at the time of freezing. I'm attaching that patch. Does
> > it fix your problem?
> >
> > Honza
>
> I tested your patch, and it can indeed fix this deadlock problem.
Thanks for testing! I've sent the patch to Al for inclusion.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-05 13:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-26 9:56 [RFC PATCH RESEND] fs: fix a hungtask problem when freeze/unfreeze fs Shijie Luo
2020-12-26 15:55 ` Al Viro
2020-12-28 2:15 ` Shijie Luo
2021-01-04 16:04 ` Jan Kara
2021-01-05 2:48 ` Shijie Luo
2021-01-05 13:46 ` Jan Kara
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.