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?