On Wed 16-02-11 22:13:53, Andreas Dilger wrote:

On 2011-02-16, at 20:50, Toshiyuki Okajima wrote:

quoted

(2011/02/16 23:56), Jan Kara wrote:

quoted

quoted

I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel without a fixed patch. After an hour, I confirmed that this deadlock happened.

However, on the kernel with a fixed patch, this deadlock doesn't still happen
after 12 hours passed.

The patch for linux-2.6.38-rc4 is as follows:
---
 fs/fs-writeback.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 59c6e49..1c9a05e 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c

@@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
        spin_unlock(&sb_lock);

        if (down_read_trylock(&sb->s_umount)) {
-               if (sb->s_root)
+               if (sb->s_frozen == SB_UNFROZEN && sb->s_root)
                        return true;
                up_read(&sb->s_umount);

This seems like a very low-risk fix.

quoted

quoted

  So this is something along the lines I thought but it actually won't work
for example if sync(1) is run while the filesystem is frozen (that takes
s_umount semaphore in a different place). And generally, I'm not convinced
there are not other places that try to do IO while holding s_umount
semaphore...

OK. I understand.

This code only fixes the case for the following path:
writeback_inodes_wb
-> ext4_da_writepages
  -> ext4_journal_start_sb
     -> vfs_check_frozen
But, the code doesn't fix the other cases.

We must modify the local filesystem part in order to fix all cases...?

It seems worthwhile to implement the low-risk fix that covers the common
case, and if/when someone hits the rare 3-process case and/or submits a
patch for it then that one will be fixed also.

  Yes, the fix is simple enough that I won't oppose it getting in as a
band aid and if we add this band aid to fs/sync.c:sync_one_sb(), it would
even be a reasonably reliable band aid. But that doesn't change the fact
that the locking is simply broken ;).

								Honza

-- 
Jan Kara [off-list ref]
SUSE Labs, CR