All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Wei Chen <harperchen1110@gmail.com>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzkaller-bugs@googlegroups.com,
	syzbot <syzkaller@googlegroups.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: possible deadlock in __ata_sff_interrupt
Date: Fri, 16 Dec 2022 03:41:07 +0000	[thread overview]
Message-ID: <Y5vo00v2F4zVKeug@ZenIV> (raw)
In-Reply-To: <80dc24c5-2c4c-b8da-5017-31aae65a4dfa@opensource.wdc.com>

On Fri, Dec 16, 2022 at 10:44:06AM +0900, Damien Le Moal wrote:

> The original & complete lockdep splat is in the report email here:
> 
> https://marc.info/?l=linux-ide&m=167094379710177&w=2
> 
> It looks like a spinlock is taken for the fasync stuff without irq
> disabled and that same spinlock is needed in kill_fasync() which is
> itself called (potentially) with IRQ disabled. Hence the splat. In any
> case, that is how I understand the issue. But as mentioned above, given
> that I can see many drivers calling kill_fasync() with irq disabled, I
> wonder if this is a genuine potential problem or a false negative.

OK, I'm about to fall asleep, so I might very well be missing something
obvious, but...

CPU1: ptrace(2)
	ptrace_check_attach()
		read_lock(&tasklist_lock);

CPU2: setpgid(2)
        write_lock_irq(&tasklist_lock);
	spins

CPU1: takes an interrupt that would call kill_fasync().  grep and the
first instance of kill_fasync() is in hpet_interrupt() - it's not
something exotic.  IRQs disabled on CPU2 won't stop it.
	kill_fasync(..., SIGIO, ...)
		kill_fasync_rcu()
			read_lock_irqsave(&fa->fa_lock, flags);
			send_sigio()
			        read_lock_irqsave(&fown->lock, flags);
		                read_lock(&tasklist_lock);

... and CPU1 spins as well.

It's not a matter of kill_fasync() called with IRQs disabled; the
problem is kill_fasync() called from interrupt taken while holding
tasklist_lock at least shared.  Somebody trying to grab it on another
CPU exclusive before we get to send_sigio() from kill_fasync() will
end up spinning and will make us spin as well.

I really hope that's just me not seeing something obvious - we had
kill_fasync() called in IRQ handlers since way back and we had
tasklist_lock taken shared without disabling IRQs for just as long.

<goes to sleep, hoping to find "Al, you are a moron, it's obviously OK
for such and such reasons" in the mailbox tomorrow morning>

  reply	other threads:[~2022-12-16  3:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-13 15:09 possible deadlock in __ata_sff_interrupt Wei Chen
2022-12-15  9:48 ` Damien Le Moal
2022-12-15 15:19   ` Al Viro
2022-12-16  1:44     ` Damien Le Moal
2022-12-16  3:41       ` Al Viro [this message]
2022-12-16 11:26         ` Linus Torvalds
2022-12-16 23:39           ` Al Viro
2022-12-16 23:54             ` Boqun Feng
2022-12-17  1:59               ` Al Viro
2022-12-17  3:25                 ` Boqun Feng
2022-12-17  2:31               ` Linus Torvalds
2022-12-17  2:59                 ` Boqun Feng
2022-12-17  3:05                 ` Al Viro
2022-12-17  4:41                   ` Waiman Long

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=Y5vo00v2F4zVKeug@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=chuck.lever@oracle.com \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=harperchen1110@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=syzkaller@googlegroups.com \
    --cc=torvalds@linux-foundation.org \
    /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 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.