Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190628091528.17059-1-duyuyang@gmail.com> <20190628091528.17059-31-duyuyang@gmail.com>
 <20190710053002.GC14490@tardis>
In-Reply-To: <20190710053002.GC14490@tardis>
From:   Yuyang Du <duyuyang@gmail.com>
Date:   Wed, 10 Jul 2019 14:30:07 +0800
Message-ID: <CAHttsrZz866PnVX=GSxQOjqYL_E4HNjtrUfCcEFs7FMtxK8O0Q@mail.gmail.com>
Subject: Re: [PATCH v3 30/30] locking/lockdep: Remove irq-safe to irq-unsafe
 read check
To:     Boqun Feng <boqun.feng@gmail.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Will Deacon <will.deacon@arm.com>,
        Ingo Molnar <mingo@kernel.org>,
        Bart Van Assche <bvanassche@acm.org>, ming.lei@redhat.com,
        Frederic Weisbecker <frederic@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>,
        Waiman Long <longman@redhat.com>, paulmck@linux.vnet.ibm.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Thanks for review.

On Wed, 10 Jul 2019 at 13:30, Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Fri, Jun 28, 2019 at 05:15:28PM +0800, Yuyang Du wrote:
> > We have a lockdep warning:
> >
> >   ========================================================
> >   WARNING: possible irq lock inversion dependency detected
> >   5.1.0-rc7+ #141 Not tainted
> >   --------------------------------------------------------
> >   kworker/8:2/328 just changed the state of lock:
> >   0000000007f1a95b (&(&host->lock)->rlock){-...}, at: ata_bmdma_interrupt+0x27/0x1c0 [libata]
> >   but this lock took another, HARDIRQ-READ-unsafe lock in the past:
> >    (&trig->leddev_list_lock){.+.?}
> >
> > and interrupts could create inverse lock ordering between them.
> >
> > other info that might help us debug this:
> >    Possible interrupt unsafe locking scenario:
> >
> >          CPU0                    CPU1
> >          ----                    ----
> >     lock(&trig->leddev_list_lock);
> >                                  local_irq_disable();
> >                                  lock(&(&host->lock)->rlock);
> >                                  lock(&trig->leddev_list_lock);
> >     <Interrupt>
> >       lock(&(&host->lock)->rlock);
> >
> >  *** DEADLOCK ***
> >
> > This splat is a false positive, which is enabled by the addition of
>
> If so, I think the better way is to reorder this patch before recursive
> read lock suppport, for better bisect-ability.

Good suggestion.

Thanks,
Yuyang