2020-10-26 13:06:38

by Peter Zijlstra

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon, Oct 26, 2020 at 11:26:49AM +0000, Filipe Manana wrote:
> Hello,
>
> I've recently started to hit a warning followed by tasks hanging after
> attempts to freeze a filesystem. A git bisection pointed to the
> following commit:
>
> commit 4d004099a668c41522242aa146a38cc4eb59cb1e
> Author: Peter Zijlstra <[email protected]>
> Date: Fri Oct 2 11:04:21 2020 +0200
>
> lockdep: Fix lockdep recursion
>
> This happens very reliably when running all xfstests with lockdep
> enabled, and the tested filesystem is btrfs (haven't tried other
> filesystems, but it shouldn't matter). The warning and task hangs always
> happen at either test generic/068 or test generic/390, and (oddly)
> always have to run all tests for it to trigger, running those tests
> individually on an infinite loop doesn't seem to trigger it (at least
> for a couple hours).
>
> The warning triggered is at fs/super.c:__sb_start_write() which always
> results later in several tasks hanging on a percpu rw_sem:
>
> https://pastebin.com/qnLvf94E
>
> What happens is percpu_rwsem_is_held() is apparently returning a false
> positive,

That smells like the same issue reported here:

https://lkml.kernel.org/r/[email protected]

Make sure you have commit:

f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")

(in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?





2020-10-26 16:03:30

by Jan Kara

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon 26-10-20 12:40:09, Peter Zijlstra wrote:
> On Mon, Oct 26, 2020 at 11:26:49AM +0000, Filipe Manana wrote:
> > Hello,
> >
> > I've recently started to hit a warning followed by tasks hanging after
> > attempts to freeze a filesystem. A git bisection pointed to the
> > following commit:
> >
> > commit 4d004099a668c41522242aa146a38cc4eb59cb1e
> > Author: Peter Zijlstra <[email protected]>
> > Date: Fri Oct 2 11:04:21 2020 +0200
> >
> > lockdep: Fix lockdep recursion
> >
> > This happens very reliably when running all xfstests with lockdep
> > enabled, and the tested filesystem is btrfs (haven't tried other
> > filesystems, but it shouldn't matter). The warning and task hangs always
> > happen at either test generic/068 or test generic/390, and (oddly)
> > always have to run all tests for it to trigger, running those tests
> > individually on an infinite loop doesn't seem to trigger it (at least
> > for a couple hours).
> >
> > The warning triggered is at fs/super.c:__sb_start_write() which always
> > results later in several tasks hanging on a percpu rw_sem:
> >
> > https://pastebin.com/qnLvf94E
> >
> > What happens is percpu_rwsem_is_held() is apparently returning a false
> > positive,
>
> That smells like the same issue reported here:
>
> https://lkml.kernel.org/r/[email protected]
>
> Make sure you have commit:
>
> f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
>
> (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?

Hum, I am at 5.10-rc1 now and above mentioned commit doesn't appear to be
there? Also googling for the title doesn't help...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2020-10-26 16:14:27

by Filipe Manana

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")



On 26/10/20 11:40, Peter Zijlstra wrote:
> On Mon, Oct 26, 2020 at 11:26:49AM +0000, Filipe Manana wrote:
>> Hello,
>>
>> I've recently started to hit a warning followed by tasks hanging after
>> attempts to freeze a filesystem. A git bisection pointed to the
>> following commit:
>>
>> commit 4d004099a668c41522242aa146a38cc4eb59cb1e
>> Author: Peter Zijlstra <[email protected]>
>> Date: Fri Oct 2 11:04:21 2020 +0200
>>
>> lockdep: Fix lockdep recursion
>>
>> This happens very reliably when running all xfstests with lockdep
>> enabled, and the tested filesystem is btrfs (haven't tried other
>> filesystems, but it shouldn't matter). The warning and task hangs always
>> happen at either test generic/068 or test generic/390, and (oddly)
>> always have to run all tests for it to trigger, running those tests
>> individually on an infinite loop doesn't seem to trigger it (at least
>> for a couple hours).
>>
>> The warning triggered is at fs/super.c:__sb_start_write() which always
>> results later in several tasks hanging on a percpu rw_sem:
>>
>> https://pastebin.com/qnLvf94E
>>
>> What happens is percpu_rwsem_is_held() is apparently returning a false
>> positive,
>
> That smells like the same issue reported here:
>
> https://lkml.kernel.org/r/[email protected]
>
> Make sure you have commit:
>
> f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
>
> (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?

Yes, CONFIG_DEBUG_PREEMPT is enabled.
I'll try with that commit and let you know, however it's gonna take a
few hours to build a kernel and run all fstests (on that test box it
takes over 3 hours) to confirm that fixes the issue.

Thanks for the quick reply!

>
>
>
>

2020-10-26 16:15:19

by Peter Zijlstra

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon, Oct 26, 2020 at 12:55:41PM +0100, Jan Kara wrote:
> > Make sure you have commit:
> >
> > f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
> >
> > (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
>
> Hum, I am at 5.10-rc1 now and above mentioned commit doesn't appear to be
> there? Also googling for the title doesn't help...

*groan*... I seem to have forgotten to push it out to tip/locking/urgent on
Friday :/

Find it here:

git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git locking/urgent


2020-10-26 17:03:22

by Peter Zijlstra

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
> > That smells like the same issue reported here:
> >
> > https://lkml.kernel.org/r/[email protected]
> >
> > Make sure you have commit:
> >
> > f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
> >
> > (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
>
> Yes, CONFIG_DEBUG_PREEMPT is enabled.

Bummer :/

> I'll try with that commit and let you know, however it's gonna take a
> few hours to build a kernel and run all fstests (on that test box it
> takes over 3 hours) to confirm that fixes the issue.

*ouch*, 3 hours is painful. How long to make it sick with the current
kernel? quicker I would hope?

> Thanks for the quick reply!

Anyway, I don't think that commit can actually explain the issue :/

The false positive on lockdep_assert_held() happens when the recursion
count is !0, however we _should_ be having IRQs disabled when
lockdep_recursion > 0, so that should never be observable.

My hope was that DEBUG_PREEMPT would trigger on one of the
__this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
then be a clear violation.

And you're seeing this on x86, right?

Let me puzzle moar..

2020-10-26 18:16:50

by Peter Zijlstra

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon, Oct 26, 2020 at 01:55:24PM +0100, Peter Zijlstra wrote:
> On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
> > > That smells like the same issue reported here:
> > >
> > > https://lkml.kernel.org/r/[email protected]
> > >
> > > Make sure you have commit:
> > >
> > > f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
> > >
> > > (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
> >
> > Yes, CONFIG_DEBUG_PREEMPT is enabled.
>
> Bummer :/
>
> > I'll try with that commit and let you know, however it's gonna take a
> > few hours to build a kernel and run all fstests (on that test box it
> > takes over 3 hours) to confirm that fixes the issue.
>
> *ouch*, 3 hours is painful. How long to make it sick with the current
> kernel? quicker I would hope?
>
> > Thanks for the quick reply!
>
> Anyway, I don't think that commit can actually explain the issue :/
>
> The false positive on lockdep_assert_held() happens when the recursion
> count is !0, however we _should_ be having IRQs disabled when
> lockdep_recursion > 0, so that should never be observable.
>
> My hope was that DEBUG_PREEMPT would trigger on one of the
> __this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
> then be a clear violation.
>
> And you're seeing this on x86, right?
>
> Let me puzzle moar..

So I might have an explanation for the Sparc64 fail, but that can't
explain x86 :/

I initially thought raw_cpu_read() was OK, since if it is !0 we have
IRQs disabled and can't get migrated, so if we get migrated both CPUs
must have 0 and it doesn't matter which 0 we read.

And while that is true; it isn't the whole store, on pretty much all
architectures (except x86) this can result in computing the address for
one CPU, getting migrated, the old CPU continuing execution with another
task (possibly setting recursion) and then the new CPU reading the value
of the old CPU, which is no longer 0.

I already fixed a bunch of that in:

baffd723e44d ("lockdep: Revert "lockdep: Use raw_cpu_*() for per-cpu variables"")

but clearly this one got crossed.

Still, that leaves me puzzled over you seeing this on x86 :/

Anatoly, could you try linus+tip/locking/urgent and the below on your
Sparc, please?

---
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 3e99dfef8408..a3041463e42d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -84,7 +84,7 @@ static inline bool lockdep_enabled(void)
if (!debug_locks)
return false;

- if (raw_cpu_read(lockdep_recursion))
+ if (this_cpu_read(lockdep_recursion))
return false;

if (current->lockdep_recursion)

2020-10-27 00:02:28

by David Sterba

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon, Oct 26, 2020 at 01:55:24PM +0100, Peter Zijlstra wrote:
> On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
> > > That smells like the same issue reported here:
> > >
> > > https://lkml.kernel.org/r/[email protected]
> > >
> > > Make sure you have commit:
> > >
> > > f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
> > >
> > > (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
> >
> > Yes, CONFIG_DEBUG_PREEMPT is enabled.
>
> Bummer :/

My builds don't have that enabled (CONFIG_PREEMPT_NONE=y) but I still
see the warning (same scenario as for Filipe). That is with today's
master branch + your fix from locking/urgent.

> > I'll try with that commit and let you know, however it's gonna take a
> > few hours to build a kernel and run all fstests (on that test box it
> > takes over 3 hours) to confirm that fixes the issue.
>
> *ouch*, 3 hours is painful. How long to make it sick with the current
> kernel? quicker I would hope?
>
> > Thanks for the quick reply!
>
> Anyway, I don't think that commit can actually explain the issue :/
>
> The false positive on lockdep_assert_held() happens when the recursion
> count is !0, however we _should_ be having IRQs disabled when
> lockdep_recursion > 0, so that should never be observable.
>
> My hope was that DEBUG_PREEMPT would trigger on one of the
> __this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
> then be a clear violation.

I can start another round (in my case it's more than 4 hours to
reproduce it) with DEBUG_PREEMPT, unless you have something else to
test.

2020-10-28 06:23:31

by Anatoly Pugachev

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon, Oct 26, 2020 at 6:23 PM Peter Zijlstra <[email protected]> wrote:
> On Mon, Oct 26, 2020 at 01:55:24PM +0100, Peter Zijlstra wrote:
> > On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
> > > > That smells like the same issue reported here:
> > > >
> > > > https://lkml.kernel.org/r/[email protected]
> > > >
> > > > Make sure you have commit:
> > > >
> > > > f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
> > > >
> > > > (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
> > >
> > > Yes, CONFIG_DEBUG_PREEMPT is enabled.
> >
> > Bummer :/
> >
> > > I'll try with that commit and let you know, however it's gonna take a
> > > few hours to build a kernel and run all fstests (on that test box it
> > > takes over 3 hours) to confirm that fixes the issue.
> >
> > *ouch*, 3 hours is painful. How long to make it sick with the current
> > kernel? quicker I would hope?
> >
> > > Thanks for the quick reply!
> >
> > Anyway, I don't think that commit can actually explain the issue :/
> >
> > The false positive on lockdep_assert_held() happens when the recursion
> > count is !0, however we _should_ be having IRQs disabled when
> > lockdep_recursion > 0, so that should never be observable.
> >
> > My hope was that DEBUG_PREEMPT would trigger on one of the
> > __this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
> > then be a clear violation.
> >
> > And you're seeing this on x86, right?
> >
> > Let me puzzle moar..
>
> So I might have an explanation for the Sparc64 fail, but that can't
> explain x86 :/
>
> I initially thought raw_cpu_read() was OK, since if it is !0 we have
> IRQs disabled and can't get migrated, so if we get migrated both CPUs
> must have 0 and it doesn't matter which 0 we read.
>
> And while that is true; it isn't the whole store, on pretty much all
> architectures (except x86) this can result in computing the address for
> one CPU, getting migrated, the old CPU continuing execution with another
> task (possibly setting recursion) and then the new CPU reading the value
> of the old CPU, which is no longer 0.
>
> I already fixed a bunch of that in:
>
> baffd723e44d ("lockdep: Revert "lockdep: Use raw_cpu_*() for per-cpu variables"")
>
> but clearly this one got crossed.
>
> Still, that leaves me puzzled over you seeing this on x86 :/
>
> Anatoly, could you try linus+tip/locking/urgent and the below on your
> Sparc, please?

Peter,
let me test first. Thanks.

PS: sorry for the delay, a weekend and got ill a bit ...

2020-10-31 11:32:30

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: locking/urgent] locking/lockdep: Remove more raw_cpu_read() usage

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID: d48e3850030623e1c20785bceaaf78f916d0b1a3
Gitweb: https://git.kernel.org/tip/d48e3850030623e1c20785bceaaf78f916d0b1a3
Author: Peter Zijlstra <[email protected]>
AuthorDate: Mon, 26 Oct 2020 16:22:56 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 30 Oct 2020 17:07:18 +01:00

locking/lockdep: Remove more raw_cpu_read() usage

I initially thought raw_cpu_read() was OK, since if it is !0 we have
IRQs disabled and can't get migrated, so if we get migrated both CPUs
must have 0 and it doesn't matter which 0 we read.

And while that is true; it isn't the whole store, on pretty much all
architectures (except x86) this can result in computing the address for
one CPU, getting migrated, the old CPU continuing execution with another
task (possibly setting recursion) and then the new CPU reading the value
of the old CPU, which is no longer 0.

Similer to:

baffd723e44d ("lockdep: Revert "lockdep: Use raw_cpu_*() for per-cpu variables"")

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/locking/lockdep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index fc206ae..1102849 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -84,7 +84,7 @@ static inline bool lockdep_enabled(void)
if (!debug_locks)
return false;

- if (raw_cpu_read(lockdep_recursion))
+ if (this_cpu_read(lockdep_recursion))
return false;

if (current->lockdep_recursion)

2020-11-02 18:02:44

by Filipe Manana

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")



On 26/10/20 15:22, Peter Zijlstra wrote:
> On Mon, Oct 26, 2020 at 01:55:24PM +0100, Peter Zijlstra wrote:
>> On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
>>>> That smells like the same issue reported here:
>>>>
>>>> https://lkml.kernel.org/r/[email protected]
>>>>
>>>> Make sure you have commit:
>>>>
>>>> f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
>>>>
>>>> (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
>>>
>>> Yes, CONFIG_DEBUG_PREEMPT is enabled.
>>
>> Bummer :/
>>
>>> I'll try with that commit and let you know, however it's gonna take a
>>> few hours to build a kernel and run all fstests (on that test box it
>>> takes over 3 hours) to confirm that fixes the issue.
>>
>> *ouch*, 3 hours is painful. How long to make it sick with the current
>> kernel? quicker I would hope?
>>
>>> Thanks for the quick reply!
>>
>> Anyway, I don't think that commit can actually explain the issue :/
>>
>> The false positive on lockdep_assert_held() happens when the recursion
>> count is !0, however we _should_ be having IRQs disabled when
>> lockdep_recursion > 0, so that should never be observable.
>>
>> My hope was that DEBUG_PREEMPT would trigger on one of the
>> __this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
>> then be a clear violation.
>>
>> And you're seeing this on x86, right?
>>
>> Let me puzzle moar..
>
> So I might have an explanation for the Sparc64 fail, but that can't
> explain x86 :/
>
> I initially thought raw_cpu_read() was OK, since if it is !0 we have
> IRQs disabled and can't get migrated, so if we get migrated both CPUs
> must have 0 and it doesn't matter which 0 we read.
>
> And while that is true; it isn't the whole store, on pretty much all
> architectures (except x86) this can result in computing the address for
> one CPU, getting migrated, the old CPU continuing execution with another
> task (possibly setting recursion) and then the new CPU reading the value
> of the old CPU, which is no longer 0.
>
> I already fixed a bunch of that in:
>
> baffd723e44d ("lockdep: Revert "lockdep: Use raw_cpu_*() for per-cpu variables"")
>
> but clearly this one got crossed.
>
> Still, that leaves me puzzled over you seeing this on x86 :/

Hi Peter,

I still get the same issue with 5.10-rc2.
Is there any non-merged patch I should try, or anything I can help with?

Thanks.

>
> Anatoly, could you try linus+tip/locking/urgent and the below on your
> Sparc, please?
>
> ---
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 3e99dfef8408..a3041463e42d 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -84,7 +84,7 @@ static inline bool lockdep_enabled(void)
> if (!debug_locks)
> return false;
>
> - if (raw_cpu_read(lockdep_recursion))
> + if (this_cpu_read(lockdep_recursion))
> return false;
>
> if (current->lockdep_recursion)
>

2020-11-03 10:20:16

by Jan Kara

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")

On Mon 02-11-20 17:58:54, Filipe Manana wrote:
>
>
> On 26/10/20 15:22, Peter Zijlstra wrote:
> > On Mon, Oct 26, 2020 at 01:55:24PM +0100, Peter Zijlstra wrote:
> >> On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
> >>>> That smells like the same issue reported here:
> >>>>
> >>>> https://lkml.kernel.org/r/[email protected]
> >>>>
> >>>> Make sure you have commit:
> >>>>
> >>>> f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
> >>>>
> >>>> (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
> >>>
> >>> Yes, CONFIG_DEBUG_PREEMPT is enabled.
> >>
> >> Bummer :/
> >>
> >>> I'll try with that commit and let you know, however it's gonna take a
> >>> few hours to build a kernel and run all fstests (on that test box it
> >>> takes over 3 hours) to confirm that fixes the issue.
> >>
> >> *ouch*, 3 hours is painful. How long to make it sick with the current
> >> kernel? quicker I would hope?
> >>
> >>> Thanks for the quick reply!
> >>
> >> Anyway, I don't think that commit can actually explain the issue :/
> >>
> >> The false positive on lockdep_assert_held() happens when the recursion
> >> count is !0, however we _should_ be having IRQs disabled when
> >> lockdep_recursion > 0, so that should never be observable.
> >>
> >> My hope was that DEBUG_PREEMPT would trigger on one of the
> >> __this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
> >> then be a clear violation.
> >>
> >> And you're seeing this on x86, right?
> >>
> >> Let me puzzle moar..
> >
> > So I might have an explanation for the Sparc64 fail, but that can't
> > explain x86 :/
> >
> > I initially thought raw_cpu_read() was OK, since if it is !0 we have
> > IRQs disabled and can't get migrated, so if we get migrated both CPUs
> > must have 0 and it doesn't matter which 0 we read.
> >
> > And while that is true; it isn't the whole store, on pretty much all
> > architectures (except x86) this can result in computing the address for
> > one CPU, getting migrated, the old CPU continuing execution with another
> > task (possibly setting recursion) and then the new CPU reading the value
> > of the old CPU, which is no longer 0.
> >
> > I already fixed a bunch of that in:
> >
> > baffd723e44d ("lockdep: Revert "lockdep: Use raw_cpu_*() for per-cpu variables"")
> >
> > but clearly this one got crossed.
> >
> > Still, that leaves me puzzled over you seeing this on x86 :/
>
> Hi Peter,
>
> I still get the same issue with 5.10-rc2.
> Is there any non-merged patch I should try, or anything I can help with?

BTW, I've just hit the same deadlock issue with ext4 on generic/390 so I
confirm this isn't btrfs specific issue (as we already knew from the
analysis but still it's good to have that confirmed).

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2020-11-03 10:25:51

by Filipe Manana

[permalink] [raw]
Subject: Re: possible lockdep regression introduced by 4d004099a668 ("lockdep: Fix lockdep recursion")



On 03/11/20 10:15, Jan Kara wrote:
> On Mon 02-11-20 17:58:54, Filipe Manana wrote:
>>
>>
>> On 26/10/20 15:22, Peter Zijlstra wrote:
>>> On Mon, Oct 26, 2020 at 01:55:24PM +0100, Peter Zijlstra wrote:
>>>> On Mon, Oct 26, 2020 at 11:56:03AM +0000, Filipe Manana wrote:
>>>>>> That smells like the same issue reported here:
>>>>>>
>>>>>> https://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> Make sure you have commit:
>>>>>>
>>>>>> f8e48a3dca06 ("lockdep: Fix preemption WARN for spurious IRQ-enable")
>>>>>>
>>>>>> (in Linus' tree by now) and do you have CONFIG_DEBUG_PREEMPT enabled?
>>>>>
>>>>> Yes, CONFIG_DEBUG_PREEMPT is enabled.
>>>>
>>>> Bummer :/
>>>>
>>>>> I'll try with that commit and let you know, however it's gonna take a
>>>>> few hours to build a kernel and run all fstests (on that test box it
>>>>> takes over 3 hours) to confirm that fixes the issue.
>>>>
>>>> *ouch*, 3 hours is painful. How long to make it sick with the current
>>>> kernel? quicker I would hope?
>>>>
>>>>> Thanks for the quick reply!
>>>>
>>>> Anyway, I don't think that commit can actually explain the issue :/
>>>>
>>>> The false positive on lockdep_assert_held() happens when the recursion
>>>> count is !0, however we _should_ be having IRQs disabled when
>>>> lockdep_recursion > 0, so that should never be observable.
>>>>
>>>> My hope was that DEBUG_PREEMPT would trigger on one of the
>>>> __this_cpu_{inc,dec}(lockdep_recursion) instance, because that would
>>>> then be a clear violation.
>>>>
>>>> And you're seeing this on x86, right?
>>>>
>>>> Let me puzzle moar..
>>>
>>> So I might have an explanation for the Sparc64 fail, but that can't
>>> explain x86 :/
>>>
>>> I initially thought raw_cpu_read() was OK, since if it is !0 we have
>>> IRQs disabled and can't get migrated, so if we get migrated both CPUs
>>> must have 0 and it doesn't matter which 0 we read.
>>>
>>> And while that is true; it isn't the whole store, on pretty much all
>>> architectures (except x86) this can result in computing the address for
>>> one CPU, getting migrated, the old CPU continuing execution with another
>>> task (possibly setting recursion) and then the new CPU reading the value
>>> of the old CPU, which is no longer 0.
>>>
>>> I already fixed a bunch of that in:
>>>
>>> baffd723e44d ("lockdep: Revert "lockdep: Use raw_cpu_*() for per-cpu variables"")
>>>
>>> but clearly this one got crossed.
>>>
>>> Still, that leaves me puzzled over you seeing this on x86 :/
>>
>> Hi Peter,
>>
>> I still get the same issue with 5.10-rc2.
>> Is there any non-merged patch I should try, or anything I can help with?
>
> BTW, I've just hit the same deadlock issue with ext4 on generic/390 so I
> confirm this isn't btrfs specific issue (as we already knew from the
> analysis but still it's good to have that confirmed).

Indeed, yesterday Darrick was mentioning on IRC that he has run into it
too with fstests on XFS (5.10-rc).

>
> Honza
>