* [PATCH] writeback: Avoid grabbing the wb if the we don't add it to dirty list
@ 2022-06-14 12:46 Suzuki K Poulose
2022-06-14 14:29 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Suzuki K Poulose @ 2022-06-14 12:46 UTC (permalink / raw)
To: Alexander Viro
Cc: linux-fsdevel, linux-kernel, linux-arm-kernel, Suzuki K Poulose,
stable, Jchao Sun, Jan Kara
Commit 10e14073107d moved grabbing the wb for an inode early enough,
skipping the checks whether if this inode needs to be really added
to the dirty list (backed by blockdev or unhashed inode). This causes
a crash with kdevtmpfs as below, on an arm64 Juno board, as below:
[ 1.446493] printk: console [ttyAMA0] printing thread started
[ 1.447195] printk: bootconsole [pl11] printing thread stopped
[ 1.467193] Unable to handle kernel paging request at virtual address ffff800871242000
[ 1.467793] Mem abort info:
[ 1.468093] ESR = 0x0000000096000005
[ 1.468413] EC = 0x25: DABT (current EL), IL = 32 bits
[ 1.468741] SET = 0, FnV = 0
[ 1.469093] EA = 0, S1PTW = 0
[ 1.469396] FSC = 0x05: level 1 translation fault
[ 1.470493] Data abort info:
[ 1.470793] ISV = 0, ISS = 0x00000005
[ 1.471093] CM = 0, WnR = 0
[ 1.471444] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000081c10000
[ 1.471798] [ffff800871242000] pgd=10000008fffff003,
p4d=10000008fffff003, pud=0000000000000000
[ 1.472836] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[ 1.472918] Modules linked in:
[ 1.473085] CPU: 1 PID: 35 Comm: kdevtmpfs Tainted: G T 5.19.0-rc1+ #49
[ 1.473246] Hardware name: Foundation-v8A (DT)
[ 1.473345] pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 1.473493] pc : locked_inode_to_wb_and_lock_list+0xbc/0x2a4
[ 1.473656] lr : locked_inode_to_wb_and_lock_list+0x8c/0x2a4
[ 1.473820] sp : ffff80000b77bc10
[ 1.473901] x29: ffff80000b77bc10 x28: 0000000000000001 x27: 0000000000000004
[ 1.474193] x26: 0000000000000000 x25: ffff000800888600 x24: ffff0008008885e8
[ 1.474393] x23: ffff80000848ddd4 x22: ffff80000a754f30 x21: ffff80000a7eaaf0
[ 1.474693] x20: ffff000800888150 x19: ffff80000b6a4150 x18: ffff80000ac3ac00
[ 1.474917] x17: 0000000070526bee x16: 000000003ac581ee x15: ffff80000ac42660
[ 1.475195] x14: 0000000000000000 x13: 0000000000007a60 x12: 0000000000000002
[ 1.475428] x11: ffff80000a7eaaf0 x10: 0000000000000004 x9 : 000000008845fe88
[ 1.475622] x8 : ffff000800868000 x7 : ffff80000ab98000 x6 : 00000000114514e2
[ 1.475893] x5 : 0000000000000000 x4 : 0000000000020019 x3 : 0000000000000001
[ 1.476113] x2 : ffff800871242000 x1 : ffff800871242000 x0 : ffff000800868000
[ 1.476393] Call trace:
[ 1.476493] locked_inode_to_wb_and_lock_list+0xbc/0x2a4
[ 1.476605] __mark_inode_dirty+0x3d8/0x6e0
[ 1.476793] simple_setattr+0x5c/0x84
[ 1.476933] notify_change+0x3ec/0x470
[ 1.477096] handle_create+0x1b8/0x224
[ 1.477193] devtmpfsd+0x98/0xf8
[ 1.477342] kthread+0x124/0x130
[ 1.477512] ret_from_fork+0x10/0x20
[ 1.477670] Code: b9000802 d2800023 d53cd042 8b020021 (f823003f)
[ 1.477793] ---[ end trace 0000000000000000 ]---
[ 1.478093] note: kdevtmpfs[35] exited with preempt_count 2
The problem was bisected to the above commit and moving the bail check
early solves the problem for me.
Fixes: 10e14073107d ("writeback: Fix inode->i_io_list not be protected by inode->i_lock error")
CC: stable@vger.kernel.org
Cc: Jchao Sun <sunjunchao2870@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
fs/fs-writeback.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 05221366a16d..cf68114af68b 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -2416,6 +2416,14 @@ void __mark_inode_dirty(struct inode *inode, int flags)
inode->i_state &= ~I_DIRTY_TIME;
inode->i_state |= flags;
+ /*
+ * Only add valid (hashed) inodes to the superblock's
+ * dirty list. Add blockdev inodes as well.
+ */
+ if (!S_ISBLK(inode->i_mode)) {
+ if (inode_unhashed(inode))
+ goto out_unlock_inode;
+ }
/*
* Grab inode's wb early because it requires dropping i_lock and we
* need to make sure following checks happen atomically with dirty
@@ -2436,14 +2444,6 @@ void __mark_inode_dirty(struct inode *inode, int flags)
if (inode->i_state & I_SYNC_QUEUED)
goto out_unlock;
- /*
- * Only add valid (hashed) inodes to the superblock's
- * dirty list. Add blockdev inodes as well.
- */
- if (!S_ISBLK(inode->i_mode)) {
- if (inode_unhashed(inode))
- goto out_unlock;
- }
if (inode->i_state & I_FREEING)
goto out_unlock;
--
2.35.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] writeback: Avoid grabbing the wb if the we don't add it to dirty list
2022-06-14 12:46 [PATCH] writeback: Avoid grabbing the wb if the we don't add it to dirty list Suzuki K Poulose
@ 2022-06-14 14:29 ` Jan Kara
2022-06-14 16:40 ` Suzuki K Poulose
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2022-06-14 14:29 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Alexander Viro, linux-fsdevel, linux-kernel, linux-arm-kernel,
stable, Jchao Sun, Jan Kara
On Tue 14-06-22 13:46:18, Suzuki K Poulose wrote:
> Commit 10e14073107d moved grabbing the wb for an inode early enough,
> skipping the checks whether if this inode needs to be really added
> to the dirty list (backed by blockdev or unhashed inode). This causes
> a crash with kdevtmpfs as below, on an arm64 Juno board, as below:
>
> [ 1.446493] printk: console [ttyAMA0] printing thread started
> [ 1.447195] printk: bootconsole [pl11] printing thread stopped
> [ 1.467193] Unable to handle kernel paging request at virtual address ffff800871242000
> [ 1.467793] Mem abort info:
> [ 1.468093] ESR = 0x0000000096000005
> [ 1.468413] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 1.468741] SET = 0, FnV = 0
> [ 1.469093] EA = 0, S1PTW = 0
> [ 1.469396] FSC = 0x05: level 1 translation fault
> [ 1.470493] Data abort info:
> [ 1.470793] ISV = 0, ISS = 0x00000005
> [ 1.471093] CM = 0, WnR = 0
> [ 1.471444] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000081c10000
> [ 1.471798] [ffff800871242000] pgd=10000008fffff003,
> p4d=10000008fffff003, pud=0000000000000000
> [ 1.472836] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> [ 1.472918] Modules linked in:
> [ 1.473085] CPU: 1 PID: 35 Comm: kdevtmpfs Tainted: G T 5.19.0-rc1+ #49
> [ 1.473246] Hardware name: Foundation-v8A (DT)
> [ 1.473345] pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 1.473493] pc : locked_inode_to_wb_and_lock_list+0xbc/0x2a4
> [ 1.473656] lr : locked_inode_to_wb_and_lock_list+0x8c/0x2a4
> [ 1.473820] sp : ffff80000b77bc10
> [ 1.473901] x29: ffff80000b77bc10 x28: 0000000000000001 x27: 0000000000000004
> [ 1.474193] x26: 0000000000000000 x25: ffff000800888600 x24: ffff0008008885e8
> [ 1.474393] x23: ffff80000848ddd4 x22: ffff80000a754f30 x21: ffff80000a7eaaf0
> [ 1.474693] x20: ffff000800888150 x19: ffff80000b6a4150 x18: ffff80000ac3ac00
> [ 1.474917] x17: 0000000070526bee x16: 000000003ac581ee x15: ffff80000ac42660
> [ 1.475195] x14: 0000000000000000 x13: 0000000000007a60 x12: 0000000000000002
> [ 1.475428] x11: ffff80000a7eaaf0 x10: 0000000000000004 x9 : 000000008845fe88
> [ 1.475622] x8 : ffff000800868000 x7 : ffff80000ab98000 x6 : 00000000114514e2
> [ 1.475893] x5 : 0000000000000000 x4 : 0000000000020019 x3 : 0000000000000001
> [ 1.476113] x2 : ffff800871242000 x1 : ffff800871242000 x0 : ffff000800868000
> [ 1.476393] Call trace:
> [ 1.476493] locked_inode_to_wb_and_lock_list+0xbc/0x2a4
> [ 1.476605] __mark_inode_dirty+0x3d8/0x6e0
> [ 1.476793] simple_setattr+0x5c/0x84
> [ 1.476933] notify_change+0x3ec/0x470
> [ 1.477096] handle_create+0x1b8/0x224
> [ 1.477193] devtmpfsd+0x98/0xf8
> [ 1.477342] kthread+0x124/0x130
> [ 1.477512] ret_from_fork+0x10/0x20
> [ 1.477670] Code: b9000802 d2800023 d53cd042 8b020021 (f823003f)
> [ 1.477793] ---[ end trace 0000000000000000 ]---
> [ 1.478093] note: kdevtmpfs[35] exited with preempt_count 2
>
> The problem was bisected to the above commit and moving the bail check
> early solves the problem for me.
>
> Fixes: 10e14073107d ("writeback: Fix inode->i_io_list not be protected by inode->i_lock error")
> CC: stable@vger.kernel.org
> Cc: Jchao Sun <sunjunchao2870@gmail.com>
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Thanks for debugging this! The problem actually is not that we cannot call
locked_inode_to_wb_and_lock_list() for devtmpfs inode. The problem is that
we get called so early during boot that noop_backing_dev_info is not
initialized yet and that breaks the code. But I agree the quick fix for
this breakage is to exclude unhashed inodes early in __mark_inode_dirty().
I'll update the changelog and code comment (and cleanup the condition when
moving it) and push the result to my tree.
Honza
> ---
> fs/fs-writeback.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 05221366a16d..cf68114af68b 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -2416,6 +2416,14 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> inode->i_state &= ~I_DIRTY_TIME;
> inode->i_state |= flags;
>
> + /*
> + * Only add valid (hashed) inodes to the superblock's
> + * dirty list. Add blockdev inodes as well.
> + */
> + if (!S_ISBLK(inode->i_mode)) {
> + if (inode_unhashed(inode))
> + goto out_unlock_inode;
> + }
> /*
> * Grab inode's wb early because it requires dropping i_lock and we
> * need to make sure following checks happen atomically with dirty
> @@ -2436,14 +2444,6 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> if (inode->i_state & I_SYNC_QUEUED)
> goto out_unlock;
>
> - /*
> - * Only add valid (hashed) inodes to the superblock's
> - * dirty list. Add blockdev inodes as well.
> - */
> - if (!S_ISBLK(inode->i_mode)) {
> - if (inode_unhashed(inode))
> - goto out_unlock;
> - }
> if (inode->i_state & I_FREEING)
> goto out_unlock;
>
> --
> 2.35.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] writeback: Avoid grabbing the wb if the we don't add it to dirty list
2022-06-14 14:29 ` Jan Kara
@ 2022-06-14 16:40 ` Suzuki K Poulose
0 siblings, 0 replies; 3+ messages in thread
From: Suzuki K Poulose @ 2022-06-14 16:40 UTC (permalink / raw)
To: Jan Kara
Cc: Alexander Viro, linux-fsdevel, linux-kernel, linux-arm-kernel,
stable, Jchao Sun
On 14/06/2022 15:29, Jan Kara wrote:
> On Tue 14-06-22 13:46:18, Suzuki K Poulose wrote:
>> Commit 10e14073107d moved grabbing the wb for an inode early enough,
>> skipping the checks whether if this inode needs to be really added
>> to the dirty list (backed by blockdev or unhashed inode). This causes
>> a crash with kdevtmpfs as below, on an arm64 Juno board, as below:
>>
>> [ 1.446493] printk: console [ttyAMA0] printing thread started
>> [ 1.447195] printk: bootconsole [pl11] printing thread stopped
>> [ 1.467193] Unable to handle kernel paging request at virtual address ffff800871242000
>> [ 1.467793] Mem abort info:
>> [ 1.468093] ESR = 0x0000000096000005
>> [ 1.468413] EC = 0x25: DABT (current EL), IL = 32 bits
>> [ 1.468741] SET = 0, FnV = 0
>> [ 1.469093] EA = 0, S1PTW = 0
>> [ 1.469396] FSC = 0x05: level 1 translation fault
>> [ 1.470493] Data abort info:
>> [ 1.470793] ISV = 0, ISS = 0x00000005
>> [ 1.471093] CM = 0, WnR = 0
>> [ 1.471444] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000081c10000
>> [ 1.471798] [ffff800871242000] pgd=10000008fffff003,
>> p4d=10000008fffff003, pud=0000000000000000
>> [ 1.472836] Internal error: Oops: 96000005 [#1] PREEMPT SMP
>> [ 1.472918] Modules linked in:
>> [ 1.473085] CPU: 1 PID: 35 Comm: kdevtmpfs Tainted: G T 5.19.0-rc1+ #49
>> [ 1.473246] Hardware name: Foundation-v8A (DT)
>> [ 1.473345] pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [ 1.473493] pc : locked_inode_to_wb_and_lock_list+0xbc/0x2a4
>> [ 1.473656] lr : locked_inode_to_wb_and_lock_list+0x8c/0x2a4
>> [ 1.473820] sp : ffff80000b77bc10
>> [ 1.473901] x29: ffff80000b77bc10 x28: 0000000000000001 x27: 0000000000000004
>> [ 1.474193] x26: 0000000000000000 x25: ffff000800888600 x24: ffff0008008885e8
>> [ 1.474393] x23: ffff80000848ddd4 x22: ffff80000a754f30 x21: ffff80000a7eaaf0
>> [ 1.474693] x20: ffff000800888150 x19: ffff80000b6a4150 x18: ffff80000ac3ac00
>> [ 1.474917] x17: 0000000070526bee x16: 000000003ac581ee x15: ffff80000ac42660
>> [ 1.475195] x14: 0000000000000000 x13: 0000000000007a60 x12: 0000000000000002
>> [ 1.475428] x11: ffff80000a7eaaf0 x10: 0000000000000004 x9 : 000000008845fe88
>> [ 1.475622] x8 : ffff000800868000 x7 : ffff80000ab98000 x6 : 00000000114514e2
>> [ 1.475893] x5 : 0000000000000000 x4 : 0000000000020019 x3 : 0000000000000001
>> [ 1.476113] x2 : ffff800871242000 x1 : ffff800871242000 x0 : ffff000800868000
>> [ 1.476393] Call trace:
>> [ 1.476493] locked_inode_to_wb_and_lock_list+0xbc/0x2a4
>> [ 1.476605] __mark_inode_dirty+0x3d8/0x6e0
>> [ 1.476793] simple_setattr+0x5c/0x84
>> [ 1.476933] notify_change+0x3ec/0x470
>> [ 1.477096] handle_create+0x1b8/0x224
>> [ 1.477193] devtmpfsd+0x98/0xf8
>> [ 1.477342] kthread+0x124/0x130
>> [ 1.477512] ret_from_fork+0x10/0x20
>> [ 1.477670] Code: b9000802 d2800023 d53cd042 8b020021 (f823003f)
>> [ 1.477793] ---[ end trace 0000000000000000 ]---
>> [ 1.478093] note: kdevtmpfs[35] exited with preempt_count 2
>>
>> The problem was bisected to the above commit and moving the bail check
>> early solves the problem for me.
>>
>> Fixes: 10e14073107d ("writeback: Fix inode->i_io_list not be protected by inode->i_lock error")
>> CC: stable@vger.kernel.org
>> Cc: Jchao Sun <sunjunchao2870@gmail.com>
>> Cc: Jan Kara <jack@suse.cz>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>
> Thanks for debugging this! The problem actually is not that we cannot call
> locked_inode_to_wb_and_lock_list() for devtmpfs inode. The problem is that
> we get called so early during boot that noop_backing_dev_info is not
> initialized yet and that breaks the code. But I agree the quick fix for
> this breakage is to exclude unhashed inodes early in __mark_inode_dirty().
> I'll update the changelog and code comment (and cleanup the condition when
> moving it) and push the result to my tree.
Thanks for the clarification. Btw, here is another report, that is fixed
with the same patch [0].
[0]
https://lore.kernel.org/linux-arm-kernel/YqiJH1phG%2FLWu9bs@monolith.localdoman/
Suzuki
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-14 16:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-14 12:46 [PATCH] writeback: Avoid grabbing the wb if the we don't add it to dirty list Suzuki K Poulose
2022-06-14 14:29 ` Jan Kara
2022-06-14 16:40 ` Suzuki K Poulose
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).