Re: [PATCH] writeback: Avoid grabbing the wb if the we don't add it to dirty list

[Suzuki K Poulose]

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@xxxxxxxxxxxxxxx
> > Cc: Jchao Sun <sunjunchao2870@xxxxxxxxx>
> > Cc: Jan Kara <jack@xxxxxxx>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>
> 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