* Possible btrfs deadlock coused by commit 660d3f6c
@ 2012-01-16 8:53 Stanislaw Gruszka
2012-01-16 14:26 ` Josef Bacik
0 siblings, 1 reply; 5+ messages in thread
From: Stanislaw Gruszka @ 2012-01-16 8:53 UTC (permalink / raw)
To: Josef Bacik; +Cc: Patrick, linux-btrfs
Hi Josef,
commit 660d3f6cde552323578b85fc5a09a6742f1fe804
Author: Josef Bacik <josef@redhat.com>
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible btrfs deadlock coused by commit 660d3f6c
2012-01-16 8:53 Possible btrfs deadlock coused by commit 660d3f6c Stanislaw Gruszka
@ 2012-01-16 14:26 ` Josef Bacik
2012-01-16 20:45 ` Al Viro
0 siblings, 1 reply; 5+ messages in thread
From: Josef Bacik @ 2012-01-16 14:26 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Josef Bacik, Patrick, linux-btrfs
On Mon, Jan 16, 2012 at 09:53:23AM +0100, Stanislaw Gruszka wrote:
> Hi Josef,
>
> commit 660d3f6cde552323578b85fc5a09a6742f1fe804
> Author: Josef Bacik <josef@redhat.com>
> 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)
>
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible btrfs deadlock coused by commit 660d3f6c
2012-01-16 14:26 ` Josef Bacik
@ 2012-01-16 20:45 ` Al Viro
2012-01-16 22:01 ` Josef Bacik
0 siblings, 1 reply; 5+ messages in thread
From: Al Viro @ 2012-01-16 20:45 UTC (permalink / raw)
To: Josef Bacik; +Cc: Stanislaw Gruszka, Patrick, linux-btrfs
On Mon, Jan 16, 2012 at 09:26:00AM -0500, Josef Bacik wrote:
> On Mon, Jan 16, 2012 at 09:53:23AM +0100, Stanislaw Gruszka wrote:
> > Hi Josef,
> >
> > commit 660d3f6cde552323578b85fc5a09a6742f1fe804
> > Author: Josef Bacik <josef@redhat.com>
> > 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)
> >
>
> 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...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible btrfs deadlock coused by commit 660d3f6c
2012-01-16 20:45 ` Al Viro
@ 2012-01-16 22:01 ` Josef Bacik
2012-01-16 22:35 ` Chris Samuel
0 siblings, 1 reply; 5+ messages in thread
From: Josef Bacik @ 2012-01-16 22:01 UTC (permalink / raw)
To: Al Viro; +Cc: Josef Bacik, Stanislaw Gruszka, Patrick, linux-btrfs
On Mon, Jan 16, 2012 at 08:45:29PM +0000, Al Viro wrote:
> On Mon, Jan 16, 2012 at 09:26:00AM -0500, Josef Bacik wrote:
> > On Mon, Jan 16, 2012 at 09:53:23AM +0100, Stanislaw Gruszka wrote:
> > > Hi Josef,
> > >
> > > commit 660d3f6cde552323578b85fc5a09a6742f1fe804
> > > Author: Josef Bacik <josef@redhat.com>
> > > 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)
> > >
> >
> > 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...
Good point, either way it's been fixed. Thanks,
Josef
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible btrfs deadlock coused by commit 660d3f6c
2012-01-16 22:01 ` Josef Bacik
@ 2012-01-16 22:35 ` Chris Samuel
0 siblings, 0 replies; 5+ messages in thread
From: Chris Samuel @ 2012-01-16 22:35 UTC (permalink / raw)
To: Josef Bacik; +Cc: Al Viro, Stanislaw Gruszka, Patrick, linux-btrfs
On 17/01/12 09:01, Josef Bacik wrote:
> Good point, either way it's been fixed. Thanks,
Is it worth pushing that fix into the stable 3.2.x releases ?
cheers,
Chris
--
Chris Samuel : http://www.csamuel.org/ : Melbourne, VIC
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-16 22:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-16 8:53 Possible btrfs deadlock coused by commit 660d3f6c Stanislaw Gruszka
2012-01-16 14:26 ` Josef Bacik
2012-01-16 20:45 ` Al Viro
2012-01-16 22:01 ` Josef Bacik
2012-01-16 22:35 ` Chris Samuel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.