Re: [syzbot] [ntfs3?] INFO: task hung in __start_renaming
Mateusz Guzik <[email protected]>
| Newsgroups | dev.linux.lists.gfs2,dev.linux.lists.ntfs3,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAGudoHHfGndcMwXMupOs82HM6c_ajMw8GETxPdkqzORrEq0btA@mail.gmail.com> |
sigh, so it *is* my patch, based on syzbot testing specifically on directory locking vs inode branches, but I don't see why. I take it the open() codepath took the rwsem, hence the rename is sleeping. Given that all reproducers find it *on* cpu, it may be this is busy looping for some reason. I don't have time to dig more into it right now, so I think it would be best to *drop* my patch for the time being. Once I figure it out I'll send a v2. On Mon, Nov 24, 2025 at 8:54 AM Mateusz Guzik <[email protected]> wrote: > > On Sun, Nov 23, 2025 at 11:13:03PM -0800, syzbot wrote: > > syzbot has tested the proposed patch but the reproducer is still triggering an issue: > > INFO: task hung in __start_renaming > > > > INFO: task syz.0.17:6473 blocked for more than 143 seconds. > > Not tainted syzkaller #0 > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > > task:syz.0.17 state:D stack:27936 pid:6473 tgid:6471 ppid:6352 task_flags:0x400040 flags:0x00080002 > > Call Trace: > > <TASK> > > context_switch kernel/sched/core.c:5257 [inline] > > __schedule+0x14bc/0x5030 kernel/sched/core.c:6864 > > __schedule_loop kernel/sched/core.c:6946 [inline] > > schedule+0x165/0x360 kernel/sched/core.c:6961 > > schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:7018 > > rwsem_down_write_slowpath+0x872/0xfe0 kernel/locking/rwsem.c:1185 > > __down_write_common kernel/locking/rwsem.c:1317 [inline] > > __down_write kernel/locking/rwsem.c:1326 [inline] > > down_write_nested+0x1b5/0x200 kernel/locking/rwsem.c:1707 > > inode_lock_nested include/linux/fs.h:1072 [inline] > > lock_rename fs/namei.c:3681 [inline] > > __start_renaming+0x148/0x410 fs/namei.c:3777 > > do_renameat2+0x399/0x8e0 fs/namei.c:5991 > > __do_sys_rename fs/namei.c:6059 [inline] > > __se_sys_rename fs/namei.c:6057 [inline] > > __x64_sys_rename+0x82/0x90 fs/namei.c:6057 > > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] > > do_syscall_64+0xfa/0xfa0 arch/x86/entry/syscall_64.c:94 > > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > RIP: 0033:0x7f2425d8f749 > > RSP: 002b:00007f2426c44038 EFLAGS: 00000246 ORIG_RAX: 0000000000000052 > > RAX: ffffffffffffffda RBX: 00007f2425fe6090 RCX: 00007f2425d8f749 > > RDX: 0000000000000000 RSI: 0000200000000080 RDI: 0000200000000340 > > RBP: 00007f2425e13f91 R08: 0000000000000000 R09: 0000000000000000 > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 > > R13: 00007f2425fe6128 R14: 00007f2425fe6090 R15: 00007ffcd5a91138 > > </TASK> > > > > So at the end of the day I think this erroneously bisected to my patch. > While it may sound like famous last words, hear me out. > > The reproducer fails to trigger the problem on my test jig, thus I > resorted to asking syzbot. > > First patch I sent for testing was merely a sanity check -- prior to my > patch inode_unhashed() checks were *always* executing, with my patch > they only happen if I_NEW was spotted. This patch "solved" the problem. > > Second patch added some asserts on inode_unhashed() and syzbot had some > internal issues, ultimately it was not tested. > > Third patch added even *more* asserts and t0his time around things > failed the previously reported way about half an hour of testing. But if > the first patch indeed solved something, the BUG_ONs would have > triggered instead. > > So that's for testing. > > In my first response I made a remark in my first reply that ntfs is at > fault. After a quick skim I spotted d_instantiate() instead of > d_instantiate_new() and jumped to conclusions. It calls unlock_new_inode() > later which does the right thing, so it is fine AFAICS. > > So what about correctness of my patch? > > My patch lifted the go-to-sleep code from inode_wait_for_lru_isolating(). > In principle that can be buggy but is just used rarely enough that it > went unnoticed. I don't see anything wrong with it though, including > after comparing it with __wait_on_freeing_inode(). Notably both > synchronize with the ->i_lock. No games played. > > I figure maybe there is something fucky going on with ordering on wakeup > side: > > inode_state_clear(inode, I_NEW | I_CREATING); > inode_wake_up_bit(inode, __I_NEW); > > Going through __wake_up_common_lock takes a spinlock, which on amd64 > would have a side effect of publishing that I_NEW store, even ignoring > therest of the ordering. > > On going to sleep side to the flag is only ever tested with ->i_lock > held anyway, so it can't be an ordering issue on that front. The thread > could not have been missed from the sleepers list as going to sleep is > again ->i_lock protected, with the lock only dropped around the call to > schedule(). > > So I don't see how this can be buggy. > > At the same time the traces report the thing off cpu is playing around > with rwsems with the __start_renaming et al patchset, while the code for > inode hash manipulation is decidedly *ON* cpu -- reported by NMIs, not > hung test detector. > > In principle this still can be a thread hung waiting on I_NEW somewhere, > but syzbot did not produce a collection of backtraces for other threads. > > However, given that the __start_renaming et al patchset is complicated > *and* that syzbot could mistakenly report (or not) a bug I'm led to > conclude the reproducer is highly unreliable and my commit landed as a > random victim. > > All that said, I think the folk working on that patchset should take > over. > > My patch is a minor optimization and can be skipped in this merge window > at no real loss, > > My take is that the big patchset *should* be skipped in this merge > window given the above, unless the problem is uickly identified.