Re: How to debug stuck read?

[Matthew Wilcox <willy@infradead.org>]

On Mon, Feb 07, 2022 at 01:21:53AM +0200, Dāvis Mosāns wrote:
> svētd., 2022. g. 6. febr., plkst. 13:01 — lietotājs FMDF
> (<fmdefrancesco@gmail.com>) rakstīja:
> [...]
> > There is no special recipe for debugging "this properly" :)
> >
> > You wrote that "by pure luck" you found a memcpy() that wrote beyond the
> > limit of allocated memory. I suppose that you found that faulty memcpy()
> > somewhere in one of the function listed in the stack trace.
> >
> > That's the right approach! You read the calls chain and find out where something
> > looks wrong and then fix it. This is why stack traces are so helpful.
> >
> > It was not "pure luck". I think that you did what developers usually do after
> > decoding a stack trace. If not, how did you find that faulty memcpy() buried
> > somewhere in 40 millions lines of code?
> >
> > it seems that you've found the right way to figure out the problems in code
> > that (probably) you had not ever worked on or read before you hit that bug.
> >
> 
> I think there should be a way to see which locks (and by who/where)
> have been taken for a long time.

Well ... we do, but the problem is that the page lock is a single bit.
We just don't have the space in struct page for a pointer to a stack
trace.  So the page lock isn't like a spinlock or a mutex where we can
use the LOCKDEP infrastructure to tell us this kind of thing.

Also, in this case, we know exactly where the lock was taken and by whom
-- and it would be no more information than you had from the stack trace.
Something I slightly regret is that you used to get a "task blocked for
more than 120 seconds" message from check_hung_task().  But I made
that not show up in this path because I made the sleep killable and
that's only called for UNINTERRUPTIBLE tasks.

Maybe that should be changed.  Perhaps we should emit those messages
for TASK_KILLABLE too.

> Yep, fully sure and tested :P I'm able to reproduce stuck process with
> 100% reliability. After applying my patch it returns EIO as expected
> and doesn't get stuck.
> If you look at copy_compressed_segment (in fs/btrfs/lzo.c) you'll see
>   kaddr = kmap(cur_page);
>   memcpy(dest + *cur_in - orig_in,
>                  kaddr + offset_in_page(*cur_in), copy_len);
>   kunmap(cur_page);
> 
> My guess is that kmap(cur_page) locks that page, then memcpy crashes
> so that page never gets unmapped causing anyone that tries to map it
> to wait forever. Hence this can be reproduced only once per boot. If I
> knew how to find kernel thread that is running this we could check
> it's stack and it should be stuck here on kmap.

kmap() doesn't lock the page; it's already locked at this point.
But if the memcpy() does crash then you're right, the page will never
be unlocked because it was this thread's job to unlock it when the page
was uptodate.  The thread will be dead, so there's no way to find it.
Do we not dump the thread's stack on its death?