linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mateusz Guzik <mjguzik@gmail.com>
To: brauner@kernel.org, neil@brown.name
Cc: agruenba@redhat.com, almaz.alexandrovich@paragon-software.com,
	 dhowells@redhat.com, gfs2@lists.linux.dev, jack@suse.cz,
	 linux-afs@lists.infradead.org, linux-fsdevel@vger.kernel.org,
	 linux-kernel@vger.kernel.org, marc.dionne@auristor.com,
	ntfs3@lists.linux.dev,  syzkaller-bugs@googlegroups.com,
	viro@zeniv.linux.org.uk,
	 syzbot+2fefb910d2c20c0698d8@syzkaller.appspotmail.com
Subject: Re: [syzbot] [ntfs3?] INFO: task hung in __start_renaming
Date: Mon, 24 Nov 2025 10:01:53 +0100	[thread overview]
Message-ID: <CAGudoHHfGndcMwXMupOs82HM6c_ajMw8GETxPdkqzORrEq0btA@mail.gmail.com> (raw)
In-Reply-To: <wtgy54dfpiapekffjjkonkkhpnxiktfp24wdmwdzf4gslrtact@pongm7vm3l2y>

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 <mjguzik@gmail.com> 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.

  reply	other threads:[~2025-11-24  9:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-23 22:44 [syzbot] [ntfs3?] INFO: task hung in __start_renaming syzbot
2025-11-23 23:29 ` Mateusz Guzik
2025-11-23 23:30   ` Mateusz Guzik
2025-11-24  0:27 ` Mateusz Guzik
2025-11-24  0:57   ` syzbot
2025-11-24  3:29 ` Mateusz Guzik
2025-11-24  4:47   ` syzbot
2025-11-24  6:28 ` Mateusz Guzik
2025-11-24  7:13   ` syzbot
2025-11-24  7:54     ` Mateusz Guzik
2025-11-24  9:01       ` Mateusz Guzik [this message]
2025-11-24  9:21         ` Mateusz Guzik
2025-11-25  9:35           ` Christian Brauner
2025-11-24  8:07 ` Mateusz Guzik
2025-11-24  8:38   ` syzbot
2025-11-24  8:08 ` Mateusz Guzik
2025-11-24  8:57   ` syzbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGudoHHfGndcMwXMupOs82HM6c_ajMw8GETxPdkqzORrEq0btA@mail.gmail.com \
    --to=mjguzik@gmail.com \
    --cc=agruenba@redhat.com \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=gfs2@lists.linux.dev \
    --cc=jack@suse.cz \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=neil@brown.name \
    --cc=ntfs3@lists.linux.dev \
    --cc=syzbot+2fefb910d2c20c0698d8@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).