2002-12-15 20:17:39

by Andrew Morton

Subject: [patch] ext3 deadlock fix

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);

_

2002-12-18 00:09:00

by Pierre Lombard

Subject: Re: [patch] ext3 deadlock fix

Hi,

* Andrew Morton <[email protected]> [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