Re: [RFC PATCH] mm: silence soft lockups from unlock_page

[Hugh Dickins <hughd@google.com>]

On Sun, 26 Jul 2020, Linus Torvalds wrote:
> On Sun, Jul 26, 2020 at 1:30 PM Hugh Dickins <hughd@google.com> wrote:
> >
> > I've deduced nothing useful from the logs, will have to leave that
> > to others here with more experience of them.  But my assumption now
> > is that you have successfully removed one bottleneck, so the tests
> > get somewhat further and now stick in the next bottleneck, whatever
> > that may be.  Which shows up as "failure", where the unlock_page()
> > wake_up_page_bit() bottleneck had allowed the tests to proceed in
> > a more serially sedate way.
> 
> Well, that's the very optimistic reading.
> 
> As the optimistic and happy person I am (hah!) I'm going to agree with
> you, and plan on just merging that patch early in the next merge
> window. It may fix a real bug in the current trere, but it's much too
> late to apply right now, particularly with your somewhat ambiguous
> results.

Absolutely: it should be good to see it in v5.9,
but much too late for a patch like this in v5.8.

> 
> Oleg's theoretical race has probably never been seen, and while the
> watchdog triggering is clearly a real bug, it's also extreme enough
> not to really be a strong argument for merging this out-of-window..
> 
> > The xhci handle_cmd_completion list_del bugs (on an older version
> > of the driver): weird, nothing to do with page wakeups, I'll just
> > have to assume that it's some driver bug exposed by the greater
> > stress allowed down, and let driver people investigate (if it
> > still manifests) when we take in your improvements.
> 
> Do you have the bug-report, just to google against anybody else
> reporting something simialr>

Okay, just on that basis, with some reluctance an edited extract:
certainly not asking you or anyone on the list to investigate further.

[35196.140502] kernel BUG at lib/list_debug.c:53!
[35196.141448] RIP: 0010:__list_del_entry_valid+0x8e/0xb0
[35196.141534] Call Trace:
[35196.141538]  <IRQ>
[35196.141557]  [<ffffffffc01bc8b4>] handle_cmd_completion+0x7d4/0x14f0 [xhci_hcd]
[35196.141578]  [<ffffffffc01bda22>] xhci_irq+0x242/0x1ea0 [xhci_hcd]
[35196.141608]  [<ffffffffc01bf691>] xhci_msi_irq+0x11/0x20 [xhci_hcd]
[35196.141622]  [<ffffffffb9ff27f8>] __handle_irq_event_percpu+0x48/0x2c0
[35196.141636]  [<ffffffffb9ff2aa2>] handle_irq_event_percpu+0x32/0x80
[35196.141651]  [<ffffffffb9ff2b3a>] handle_irq_event+0x4a/0x80
[35196.141680]  [<ffffffffb9ff6b08>] handle_edge_irq+0xd8/0x1b0
[35196.141697]  [<ffffffffb9ec22ab>] handle_irq+0x2b/0x50
[35196.141712]  [<ffffffffbaa02766>] do_IRQ+0xb6/0x1c0
[35196.141725]  [<ffffffffbaa00990>] common_interrupt+0x90/0x90
[35196.141732]  </IRQ>
> 
> > One nice thing from the comparison runs without your patches:
> > watchdog panic did crash one of those with exactly the unlock_page()
> > wake_up_page_bit() softlockup symptom we've been fighting, that did
> > not appear with your patches.  So although the sample size is much
> > too small to justify a conclusion, it does tend towards confirming
> > your changes.
> 
> You win some, you lose some. But yes, I'll take that as a tentative
> success and that the approach is valid.

Great, yes, tentative success: and we have three months in which to
change our minds if any real trouble surfaces; and I wouldn't call
anything I've seen (since that very first version) *real* trouble.

> 
> Thanks,
> 
>                Linus