Skip to contentSkip to conversation

Possible btrfs deadlock coused by commit 660d3f6c

DISCUSSION Stanislaw Gruszka 4 participants 5 messages Jan 16, 2012 Btrfs file system

lore
Share & tools
lore
#0 lore raw

Hi Josef,

commit 660d3f6cde552323578b85fc5a09a6742f1fe804

Author: Josef Bacik <[email protected]>
Date:   Fri Dec 9 11:18:51 2011 -0500

Btrfs: fix how we do delalloc reservations and how we free reservations on error

introduced possible deadlock. According to comment before btrfs_page_mkwrite(), that function is not allowed to take node->i_mutex .

The problem was detected by Patrick (full lockdep info is provided):
http://bugzilla.kernel.org/attachment.cgi?id=72072
during investigating of some other issue (kernel.org bug 42576)

Stanislaw

#1 lore raw
Quoted 8 lines · Stanislaw Gruszka

Hi Josef,

commit 660d3f6cde552323578b85fc5a09a6742f1fe804

Author: Josef Bacik <[email protected]>
Date:   Fri Dec 9 11:18:51 2011 -0500

Btrfs: fix how we do delalloc reservations and how we free reservations on error

Quoted 8 lines

introduced possible deadlock. According to comment before btrfs_page_mkwrite(), that function is not allowed to take node->i_mutex .

The problem was detected by Patrick (full lockdep info is provided):
http://bugzilla.kernel.org/attachment.cgi?id=72072
during investigating of some other issue (kernel.org bug 42576)

It's not a deadlock since you can't mmap a directory, but I posted a fix for this to linux-btrfs last week, afaik it's headed to -rc1. Thanks,

Joesf

#2 lore raw
Quoted 13 lines · Josef Bacik
Quoted 12 lines · Stanislaw Gruszka

Hi Josef,

commit 660d3f6cde552323578b85fc5a09a6742f1fe804

Author: Josef Bacik <[email protected]>
Date:   Fri Dec 9 11:18:51 2011 -0500

Btrfs: fix how we do delalloc reservations and how we free reservations on error

introduced possible deadlock. According to comment before btrfs_page_mkwrite(), that function is not allowed to take node->i_mutex .

Quoted 7 lines
Quoted 4 lines

The problem was detected by Patrick (full lockdep info is provided):
http://bugzilla.kernel.org/attachment.cgi?id=72072
during investigating of some other issue (kernel.org bug 42576)

It's not a deadlock since you can't mmap a directory, but I posted a fix for this to linux-btrfs last week, afaik it's headed to -rc1. Thanks,

Sigh... You know, I'm getting really annoyed with the "can't mmap a directory" bogosity resurfacing every couple of months or so. No, you can't mmap a directory. You also can't bugger a hedgehog, which is about as relevant.

What you *can* do is write(2) on a regular file. Which will happily grab ->i_mutex (in case of btrfs, that's done in the beginning of btrfs_file_aio_write()) and eventually get the user pages in (in case of btrfs, iov_iter_fault_in_readable() from __btrfs_buffered_write() from btrfs_file_aio_write()). IOW, you do get pagefaults with ->i_mutex held. Which means ->mmap_sem grabbed while holding ->i_mutex. No directories involved...

#3 lore raw
Quoted 26 lines · Al Viro
Quoted 13 lines · Josef Bacik
Quoted 12 lines · Stanislaw Gruszka

Hi Josef,

commit 660d3f6cde552323578b85fc5a09a6742f1fe804

Author: Josef Bacik <[email protected]>
Date:   Fri Dec 9 11:18:51 2011 -0500

Btrfs: fix how we do delalloc reservations and how we free reservations on error

introduced possible deadlock. According to comment before btrfs_page_mkwrite(), that function is not allowed to take node->i_mutex .

Quoted 7 lines
Quoted 4 lines

The problem was detected by Patrick (full lockdep info is provided):
http://bugzilla.kernel.org/attachment.cgi?id=72072
during investigating of some other issue (kernel.org bug 42576)

It's not a deadlock since you can't mmap a directory, but I posted a fix for this to linux-btrfs last week, afaik it's headed to -rc1. Thanks,

Sigh... You know, I'm getting really annoyed with the "can't mmap a directory" bogosity resurfacing every couple of months or so. No, you can't mmap a directory. You also can't bugger a hedgehog, which is about as relevant.

Quoted 7 lines

What you *can* do is write(2) on a regular file. Which will happily grab ->i_mutex (in case of btrfs, that's done in the beginning of btrfs_file_aio_write()) and eventually get the user pages in (in case of btrfs, iov_iter_fault_in_readable() from __btrfs_buffered_write() from btrfs_file_aio_write()). IOW, you do get pagefaults with ->i_mutex held. Which means ->mmap_sem grabbed while holding ->i_mutex. No directories involved...

Good point, either way it's been fixed. Thanks,

Josef

#4 lore raw
Quoted 1 line · Josef Bacik

Good point, either way it's been fixed. Thanks,

Is it worth pushing that fix into the stable 3.2.x releases ?

cheers,
Chris

Signature · 1 line
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC