[patch] ext3 deadlock fix

[patch] ext3 deadlock fix

[Andrew Morton]

My recent fix for the ext3 data=journal umount data loss problem
has a bug.  The filesystem can deadlock if someone runs `mount -o remount'
while the filesystem is under load.  Everything which writes to that
filesystem gets stuck in `D' state.

This is because:

a) ext3_sync_fs() has to wait until a transaction has finished.

b) a transaction cannot finish when someone else holds lock_super().
   Because lock_super() is used in the block allocator.

The patch ensures that ->sync_fs is never run under lock_super().




 Documentation/filesystems/Locking |    2 ++
 fs/buffer.c                       |    2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

--- 24/Documentation/filesystems/Locking~sync_fs-fix	Sun Dec 15 11:12:48 2002
+++ 24-akpm/Documentation/filesystems/Locking	Sun Dec 15 11:16:15 2002
@@ -93,6 +93,7 @@ prototypes:
 	void (*delete_inode) (struct inode *);
 	void (*put_super) (struct super_block *);
 	void (*write_super) (struct super_block *);
+	int (*sync_fs) (struct super_block *);
 	int (*statfs) (struct super_block *, struct statfs *);
 	int (*remount_fs) (struct super_block *, int *, char *);
 	void (*clear_inode) (struct inode *);
@@ -108,6 +109,7 @@ delete_inode:	no	
 clear_inode:	no	
 put_super:	yes	yes	maybe		(see below)
 write_super:	yes	yes	maybe		(see below)
+write_super:	yes	no	maybe		(see below)
 statfs:		yes	no	no
 remount_fs:	yes	yes	maybe		(see below)
 umount_begin:	yes	no	maybe		(see below)
--- 24/fs/buffer.c~sync_fs-fix	Sun Dec 15 11:12:48 2002
+++ 24-akpm/fs/buffer.c	Sun Dec 15 11:13:13 2002
@@ -327,9 +327,9 @@ int fsync_super(struct super_block *sb)
 	lock_super(sb);
 	if (sb->s_dirt && sb->s_op && sb->s_op->write_super)
 		sb->s_op->write_super(sb);
+	unlock_super(sb);
 	if (sb->s_op && sb->s_op->sync_fs)
 		sb->s_op->sync_fs(sb);
-	unlock_super(sb);
 	unlock_kernel();
 
 	return sync_buffers(dev, 1);

_

Re: [patch] ext3 deadlock fix

[Pierre Lombard]

Hi,

* Andrew Morton <akpm@digeo.com> [2002-12-15 21:27]:

> The patch ensures that ->sync_fs is never run under lock_super().

> --- 24/Documentation/filesystems/Locking~sync_fs-fix	Sun Dec 15 11:12:48 2002
> +++ 24-akpm/Documentation/filesystems/Locking	Sun Dec 15 11:16:15 2002
> @@ -93,6 +93,7 @@ prototypes:
>  	void (*delete_inode) (struct inode *);
>  	void (*put_super) (struct super_block *);
>  	void (*write_super) (struct super_block *);
> +	int (*sync_fs) (struct super_block *);
>  	int (*statfs) (struct super_block *, struct statfs *);
>  	int (*remount_fs) (struct super_block *, int *, char *);
>  	void (*clear_inode) (struct inode *);
> @@ -108,6 +109,7 @@ delete_inode:	no	
>  clear_inode:	no	
>  put_super:	yes	yes	maybe		(see below)
>  write_super:	yes	yes	maybe		(see below)
> +write_super:	yes	no	maybe		(see below)
   ~~~~~~~~~~~
A small typo in the documentation.

>  statfs:		yes	no	no
>  remount_fs:	yes	yes	maybe		(see below)
>  umount_begin:	yes	no	maybe		(see below)

-- 
Best regards,
  Pierre