Re: [patch] fix up lock order reversal in writeback

2010-11-23 Thread Nick Piggin
On Mon, Nov 22, 2010 at 07:16:55PM +0100, Jan Kara wrote: On Fri 19-11-10 16:16:19, Nick Piggin wrote: On Fri, Nov 19, 2010 at 01:45:52AM +0100, Jan Kara wrote: On Wed 17-11-10 22:28:34, Andrew Morton wrote: The fact that a call to -write_begin can randomly return with s_umount held,

Re: [patch] fix up lock order reversal in writeback

2010-11-23 Thread Jan Kara
On Tue 23-11-10 19:07:58, Nick Piggin wrote: On Mon, Nov 22, 2010 at 07:16:55PM +0100, Jan Kara wrote: On Fri 19-11-10 16:16:19, Nick Piggin wrote: On Fri, Nov 19, 2010 at 01:45:52AM +0100, Jan Kara wrote: On Wed 17-11-10 22:28:34, Andrew Morton wrote: The fact that a call to

Re: [patch] fix up lock order reversal in writeback

2010-11-22 Thread Jan Kara
On Fri 19-11-10 16:16:19, Nick Piggin wrote: On Fri, Nov 19, 2010 at 01:45:52AM +0100, Jan Kara wrote: On Wed 17-11-10 22:28:34, Andrew Morton wrote: The fact that a call to -write_begin can randomly return with s_umount held, to be randomly released at some random time in the future is a

Re: [patch] fix up lock order reversal in writeback

2010-11-22 Thread Andrew Morton
On Wed, 17 Nov 2010 21:18:13 -0600 Eric Sandeen sand...@redhat.com wrote: On 11/17/10 12:10 AM, Nick Piggin wrote: On Tue, Nov 16, 2010 at 11:05:52PM -0600, Eric Sandeen wrote: On 11/16/10 10:38 PM, Nick Piggin wrote: as for the locking problems ... sorry about that! That's no problem.

Re: [patch] fix up lock order reversal in writeback

2010-11-19 Thread Theodore Tso
On Nov 19, 2010, at 12:10 AM, Nick Piggin wrote: But asynch writeout needs a mutex rather than refcount so the umount has something to block against and not just fail. Or we use a completion handler instead of a mutex for umount? - Ted -- To unsubscribe from this list: send the line

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Nick Piggin
On Wed, Nov 17, 2010 at 10:28:34PM -0800, Andrew Morton wrote: On Thu, 18 Nov 2010 17:00:00 +1100 Nick Piggin npig...@kernel.dk wrote: On Wed, Nov 17, 2010 at 07:29:00PM -0800, Andrew Morton wrote: On Wed, 17 Nov 2010 22:06:13 -0500 Ted Ts'o ty...@mit.edu wrote: On Wed, Nov 17,

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Theodore Tso
On Nov 18, 2010, at 3:18 AM, Nick Piggin wrote: s_count just prevents it from going away, but s_umount is still needed to keep umount, remount,ro, freezing etc activity away. I don't think there is an easy way to do it. Hmm what about encoding the fact that we are in the process of

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Andrew Morton
On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen sand...@redhat.com wrote: Can we just delete writeback_inodes_sb_nr_if_idle() and writeback_inodes_sb_if_idle()? The changelog for 17bd55d037a02 is pretty handwavy - do we know that deleting these things would make a jot of difference?

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Andrew Morton
On Thu, 18 Nov 2010 19:18:22 +1100 Nick Piggin npig...@kernel.dk wrote: On Wed, Nov 17, 2010 at 10:28:34PM -0800, Andrew Morton wrote: Logically I'd expect i_mutex to nest inside s_umount. Because s_umount is a per-superblock thing, and i_mutex is a per-file thing, and files live under

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Eric Sandeen
On 11/18/10 11:10 AM, Andrew Morton wrote: On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen sand...@redhat.com wrote: Can we just delete writeback_inodes_sb_nr_if_idle() and writeback_inodes_sb_if_idle()? The changelog for 17bd55d037a02 is pretty handwavy - do we know that deleting these

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Eric Sandeen
On 11/18/10 12:04 PM, Eric Sandeen wrote: On 11/18/10 11:10 AM, Andrew Morton wrote: On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen sand...@redhat.com wrote: Can we just delete writeback_inodes_sb_nr_if_idle() and writeback_inodes_sb_if_idle()? The changelog for 17bd55d037a02 is pretty

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Chris Mason
Excerpts from Andrew Morton's message of 2010-11-18 01:28:34 -0500: I'm not sure that s_umount versus i_mutex has come up before. Logically I'd expect i_mutex to nest inside s_umount. Because s_umount is a per-superblock thing, and i_mutex is a per-file thing, and files live under

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Chris Mason
Excerpts from Eric Sandeen's message of 2010-11-18 13:24:57 -0500: Um, ok, then, to answer the question directly : No, please don't delete those functions, it will break ENOSPC handling in ext4 as shown by xfstests regression test #204 ... Further - What is going on here is that

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Andrew Morton
On Thu, 18 Nov 2010 12:04:21 -0600 Eric Sandeen sand...@redhat.com wrote: On 11/18/10 11:10 AM, Andrew Morton wrote: On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen sand...@redhat.com wrote: Can we just delete writeback_inodes_sb_nr_if_idle() and writeback_inodes_sb_if_idle()? The

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Chris Mason
Excerpts from Andrew Morton's message of 2010-11-18 13:36:38 -0500: On Thu, 18 Nov 2010 12:04:21 -0600 Eric Sandeen sand...@redhat.com wrote: On 11/18/10 11:10 AM, Andrew Morton wrote: On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen sand...@redhat.com wrote: Can we just delete

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Al Viro
On Wed, Nov 17, 2010 at 10:06:13PM -0500, Ted Ts'o wrote: This makes sense to me as well. Acked-by: Theodore Ts'o ty...@mit.edu So how do we want to send this patch to Linus? It's a writeback change, so through some mm tree? Or it lives in fs/fs-writeback.c (which I always thought was

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Eric Sandeen
On 11/18/10 12:36 PM, Andrew Morton wrote: On Thu, 18 Nov 2010 12:04:21 -0600 Eric Sandeen sand...@redhat.com wrote: On 11/18/10 11:10 AM, Andrew Morton wrote: On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen sand...@redhat.com wrote: Can we just delete writeback_inodes_sb_nr_if_idle() and

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Andrew Morton
On Thu, 18 Nov 2010 13:02:43 -0600 Eric Sandeen sand...@redhat.com wrote: On 11/18/10 12:36 PM, Andrew Morton wrote: On Thu, 18 Nov 2010 12:04:21 -0600 Eric Sandeen sand...@redhat.com wrote: On 11/18/10 11:10 AM, Andrew Morton wrote: On Thu, 18 Nov 2010 08:55:18 -0600 Eric Sandeen

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Andrew Morton
On Thu, 18 Nov 2010 13:51:15 -0500 Chris Mason chris.ma...@oracle.com wrote: If those functions fix a testcase then it was by sheer luck, and the fs's ENOSPC handling is still busted. For a start writeback_inodes_sb_if_idle() is a no-op if the device isn't idle! Secondly, if the

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Chris Mason
Excerpts from Andrew Morton's message of 2010-11-18 15:22:38 -0500: On Thu, 18 Nov 2010 13:51:15 -0500 Chris Mason chris.ma...@oracle.com wrote: If those functions fix a testcase then it was by sheer luck, and the fs's ENOSPC handling is still busted. For a start

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Jan Kara
On Wed 17-11-10 22:28:34, Andrew Morton wrote: I'm not sure that s_umount versus i_mutex has come up before. Logically I'd expect i_mutex to nest inside s_umount. Because s_umount is a per-superblock thing, and i_mutex is a per-file thing, and files live under superblocks. Nesting s_umount

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Nick Piggin
On Thu, Nov 18, 2010 at 09:58:31AM -0800, Andrew Morton wrote: On Thu, 18 Nov 2010 19:18:22 +1100 Nick Piggin npig...@kernel.dk wrote: On Wed, Nov 17, 2010 at 10:28:34PM -0800, Andrew Morton wrote: Logically I'd expect i_mutex to nest inside s_umount. Because s_umount is a

Re: [patch] fix up lock order reversal in writeback

2010-11-18 Thread Nick Piggin
On Fri, Nov 19, 2010 at 01:45:52AM +0100, Jan Kara wrote: On Wed 17-11-10 22:28:34, Andrew Morton wrote: The fact that a call to -write_begin can randomly return with s_umount held, to be randomly released at some random time in the future is a bit ugly, isn't it? write_begin is a pretty

Re: [patch] fix up lock order reversal in writeback

2010-11-17 Thread Ted Ts'o
On Wed, Nov 17, 2010 at 05:10:57PM +1100, Nick Piggin wrote: On Tue, Nov 16, 2010 at 11:05:52PM -0600, Eric Sandeen wrote: On 11/16/10 10:38 PM, Nick Piggin wrote: as for the locking problems ... sorry about that! That's no problem. So is that an ack? :) I'd like to test it

Re: [patch] fix up lock order reversal in writeback

2010-11-17 Thread Eric Sandeen
On 11/17/10 12:10 AM, Nick Piggin wrote: On Tue, Nov 16, 2010 at 11:05:52PM -0600, Eric Sandeen wrote: On 11/16/10 10:38 PM, Nick Piggin wrote: as for the locking problems ... sorry about that! That's no problem. So is that an ack? :) I'd like to test it with the original case it was

Re: [patch] fix up lock order reversal in writeback

2010-11-17 Thread Andrew Morton
On Wed, 17 Nov 2010 22:06:13 -0500 Ted Ts'o ty...@mit.edu wrote: On Wed, Nov 17, 2010 at 05:10:57PM +1100, Nick Piggin wrote: On Tue, Nov 16, 2010 at 11:05:52PM -0600, Eric Sandeen wrote: On 11/16/10 10:38 PM, Nick Piggin wrote: as for the locking problems ... sorry about that!

Re: [patch] fix up lock order reversal in writeback

2010-11-17 Thread Nick Piggin
On Wed, Nov 17, 2010 at 07:29:00PM -0800, Andrew Morton wrote: On Wed, 17 Nov 2010 22:06:13 -0500 Ted Ts'o ty...@mit.edu wrote: On Wed, Nov 17, 2010 at 05:10:57PM +1100, Nick Piggin wrote: On Tue, Nov 16, 2010 at 11:05:52PM -0600, Eric Sandeen wrote: On 11/16/10 10:38 PM, Nick Piggin

[patch] fix up lock order reversal in writeback

2010-11-16 Thread Nick Piggin
I saw a lock order warning on ext4 trigger. This should solve it. Raciness shouldn't matter much, because writeback can stop just after we make the test and return anyway (so the API is racy anyway). Signed-off-by: Nick Piggin npig...@kernel.dk Index: linux-2.6/fs/fs-writeback.c

Re: [patch] fix up lock order reversal in writeback

2010-11-16 Thread Jan Kara
On Tue 16-11-10 22:00:58, Nick Piggin wrote: I saw a lock order warning on ext4 trigger. This should solve it. Raciness shouldn't matter much, because writeback can stop just after we make the test and return anyway (so the API is racy anyway). Hmm, for now the fix is OK. Ultimately, we

Re: [patch] fix up lock order reversal in writeback

2010-11-16 Thread Nick Piggin
On Tue, Nov 16, 2010 at 12:32:52PM -0800, Andrew Morton wrote: On Tue, 16 Nov 2010 22:00:58 +1100 Nick Piggin npig...@kernel.dk wrote: I saw a lock order warning on ext4 trigger. This should solve it. Send us the trace, please. I lost it, sorry. The code comment implies that someone

Re: [patch] fix up lock order reversal in writeback

2010-11-16 Thread Eric Sandeen
On 11/16/10 7:01 AM, Jan Kara wrote: On Tue 16-11-10 22:00:58, Nick Piggin wrote: I saw a lock order warning on ext4 trigger. This should solve it. Raciness shouldn't matter much, because writeback can stop just after we make the test and return anyway (so the API is racy anyway). Hmm, for

Re: [patch] fix up lock order reversal in writeback

2010-11-16 Thread Nick Piggin
On Tue, Nov 16, 2010 at 10:30:37PM -0600, Eric Sandeen wrote: On 11/16/10 7:01 AM, Jan Kara wrote: On Tue 16-11-10 22:00:58, Nick Piggin wrote: I saw a lock order warning on ext4 trigger. This should solve it. Raciness shouldn't matter much, because writeback can stop just after we make

Re: [patch] fix up lock order reversal in writeback

2010-11-16 Thread Eric Sandeen
On 11/16/10 10:38 PM, Nick Piggin wrote: On Tue, Nov 16, 2010 at 10:30:37PM -0600, Eric Sandeen wrote: On 11/16/10 7:01 AM, Jan Kara wrote: On Tue 16-11-10 22:00:58, Nick Piggin wrote: I saw a lock order warning on ext4 trigger. This should solve it. Raciness shouldn't matter much, because

Re: [patch] fix up lock order reversal in writeback

2010-11-16 Thread Nick Piggin
On Tue, Nov 16, 2010 at 11:05:52PM -0600, Eric Sandeen wrote: On 11/16/10 10:38 PM, Nick Piggin wrote: as for the locking problems ... sorry about that! That's no problem. So is that an ack? :) I'd like to test it with the original case it was supposed to solve; will do that