[PATCH] Ext2/3 noatime and dirsync fixes

Patch from "Theodore Ts'o" <tytso@mit.edu>

I recently noticed a bug in ext2/3; newly created inodes which inherit
the noatime flag from their containing directory do not respect noatime
until the inode is flushed from the inode cache and then re-read later.
This is because the code which checks the ext2 no-atime attribute and
then sets the S_NOATIME in inode->i_flags is present in
ext2_read_inode(), but not in ext2_new_inode().

I fixed this in 2.4, and then found an even worse bug in the 2.5 code;
the DIRSYNC flag is completely ignored *except* in the case where a
directory is newly created using mkdir and its parent directory has the
DIRSYNC flag.  S_DIRSYNC doesn't get set in the ext2_new_inode() or the
ext2_ioctl() paths (which is used by chattr).

This patch centralizes the code which translates the ext2 flags in the
raw ext2 inode to the appropriate flag values in inode->i_flags in a
single location.  This fixes the bug, makes things cleaner, and also
removes 30 lines of code and 128 bytes of compiled x86 text in the
bargain.

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 7850dcbe7bf1ad1cfdea13b6a7cd2721d541b875..610695289845f65b6b29d21bed231f2e8f07b602 100644 (file)
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -112,6 +112,7 @@ extern int ext2_sync_inode (struct inode *);
 extern void ext2_discard_prealloc (struct inode *);
 extern void ext2_truncate (struct inode *);
 extern int ext2_setattr (struct dentry *, struct iattr *);
+extern void ext2_set_inode_flags(struct inode *inode);
 
 /* ioctl.c */
 extern int ext2_ioctl (struct inode *, struct file *, unsigned int,

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index 345e7495176d8ca5323ecaf226e55828bba688e5..87b2d99f4a7c778d74bac5692fcf64faf5be6852 100644 (file)
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -545,10 +545,7 @@ repeat:
        ei->i_prealloc_count = 0;
        ei->i_dir_start_lookup = 0;
        ei->i_state = EXT2_STATE_NEW;
-       if (ei->i_flags & EXT2_SYNC_FL)
-               inode->i_flags |= S_SYNC;
-       if (ei->i_flags & EXT2_DIRSYNC_FL)
-               inode->i_flags |= S_DIRSYNC;
+       ext2_set_inode_flags(inode);
        inode->i_generation = EXT2_SB(sb)->s_next_generation++;
        insert_inode_hash(inode);
 

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index e47f84e305cda33de91d6bf223cb42a92704e09d..c2fbefdd361323705446d24f4979e5174d000726 100644 (file)
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1011,6 +1011,23 @@ Egdp:
        return ERR_PTR(-EIO);
 }
 
+void ext2_set_inode_flags(struct inode *inode)
+{
+       unsigned int flags = EXT2_I(inode)->i_flags;
+
+       inode->i_flags &= ~(S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC);
+       if (flags & EXT2_SYNC_FL)
+               inode->i_flags |= S_SYNC;
+       if (flags & EXT2_APPEND_FL)
+               inode->i_flags |= S_APPEND;
+       if (flags & EXT2_IMMUTABLE_FL)
+               inode->i_flags |= S_IMMUTABLE;
+       if (flags & EXT2_NOATIME_FL)
+               inode->i_flags |= S_NOATIME;
+       if (flags & EXT2_DIRSYNC_FL)
+               inode->i_flags |= S_DIRSYNC;
+}
+
 void ext2_read_inode (struct inode * inode)
 {
        struct ext2_inode_info *ei = EXT2_I(inode);
@@ -1108,14 +1125,7 @@ void ext2_read_inode (struct inode * inode)
                                   le32_to_cpu(raw_inode->i_block[0]));
        }
        brelse (bh);
-       if (ei->i_flags & EXT2_SYNC_FL)
-               inode->i_flags |= S_SYNC;
-       if (ei->i_flags & EXT2_APPEND_FL)
-               inode->i_flags |= S_APPEND;
-       if (ei->i_flags & EXT2_IMMUTABLE_FL)
-               inode->i_flags |= S_IMMUTABLE;
-       if (ei->i_flags & EXT2_NOATIME_FL)
-               inode->i_flags |= S_NOATIME;
+       ext2_set_inode_flags(inode);
        return;
        
 bad_inode:

diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c
index afff8566f3b6f12844fc1bb1f355d73eaad96d38..101055bbf5194b63de5d11934eee7f867495f6f6 100644 (file)
--- a/fs/ext2/ioctl.c
+++ b/fs/ext2/ioctl.c
@@ -58,22 +58,7 @@ int ext2_ioctl (struct inode * inode, struct file * filp, unsigned int cmd,
                flags |= oldflags & ~EXT2_FL_USER_MODIFIABLE;
                ei->i_flags = flags;
 
-               if (flags & EXT2_SYNC_FL)
-                       inode->i_flags |= S_SYNC;
-               else
-                       inode->i_flags &= ~S_SYNC;
-               if (flags & EXT2_APPEND_FL)
-                       inode->i_flags |= S_APPEND;
-               else
-                       inode->i_flags &= ~S_APPEND;
-               if (flags & EXT2_IMMUTABLE_FL)
-                       inode->i_flags |= S_IMMUTABLE;
-               else
-                       inode->i_flags &= ~S_IMMUTABLE;
-               if (flags & EXT2_NOATIME_FL)
-                       inode->i_flags |= S_NOATIME;
-               else
-                       inode->i_flags &= ~S_NOATIME;
+               ext2_set_inode_flags(inode);
                inode->i_ctime = CURRENT_TIME;
                mark_inode_dirty(inode);
                return 0;

diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index 228fa402af196177e1a7edc89dadc38e74825f77..155c19c4ac92a09ec6136dc5e308db271b7c16ab 100644 (file)
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -568,10 +568,7 @@ repeat:
 #endif
        ei->i_block_group = group;
        
-       if (ei->i_flags & EXT3_SYNC_FL)
-               inode->i_flags |= S_SYNC;
-       if (ei->i_flags & EXT3_DIRSYNC_FL)
-               inode->i_flags |= S_DIRSYNC;
+       ext3_set_inode_flags(inode);
        if (IS_DIRSYNC(inode))
                handle->h_sync = 1;
        insert_inode_hash(inode);

diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index b37c23fb3d5a4cff16409216e58389c972efa95d..5012c1719ee4f01a2f0dc5b981a20f9b0df97e4d 100644 (file)
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2209,6 +2209,24 @@ int ext3_get_inode_loc (struct inode *inode, struct ext3_iloc *iloc)
        return -EIO;
 }
 
+void ext3_set_inode_flags(struct inode *inode)
+{
+       unsigned int flags = EXT3_I(inode)->i_flags;
+
+       inode->i_flags &= ~(S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC);
+       if (flags & EXT3_SYNC_FL)
+               inode->i_flags |= S_SYNC;
+       if (flags & EXT3_APPEND_FL)
+               inode->i_flags |= S_APPEND;
+       if (flags & EXT3_IMMUTABLE_FL)
+               inode->i_flags |= S_IMMUTABLE;
+       if (flags & EXT3_NOATIME_FL)
+               inode->i_flags |= S_NOATIME;
+       if (flags & EXT3_DIRSYNC_FL)
+               inode->i_flags |= S_DIRSYNC;
+}
+
+
 void ext3_read_inode(struct inode * inode)
 {
        struct ext3_iloc iloc;
@@ -2320,14 +2338,7 @@ void ext3_read_inode(struct inode * inode)
                init_special_inode(inode, inode->i_mode,
                                   le32_to_cpu(iloc.raw_inode->i_block[0]));
        }
-       if (ei->i_flags & EXT3_SYNC_FL)
-               inode->i_flags |= S_SYNC;
-       if (ei->i_flags & EXT3_APPEND_FL)
-               inode->i_flags |= S_APPEND;
-       if (ei->i_flags & EXT3_IMMUTABLE_FL)
-               inode->i_flags |= S_IMMUTABLE;
-       if (ei->i_flags & EXT3_NOATIME_FL)
-               inode->i_flags |= S_NOATIME;
+       ext3_set_inode_flags(inode);
        return;
        
 bad_inode:

diff --git a/fs/ext3/ioctl.c b/fs/ext3/ioctl.c
index 5d74409cef07e56e9a86db736f50636ab67f7150..cedf91bde8a2a8aeae82dea6a18f4190291c2ceb 100644 (file)
--- a/fs/ext3/ioctl.c
+++ b/fs/ext3/ioctl.c
@@ -85,22 +85,7 @@ int ext3_ioctl (struct inode * inode, struct file * filp, unsigned int cmd,
                flags |= oldflags & ~EXT3_FL_USER_MODIFIABLE;
                ei->i_flags = flags;
 
-               if (flags & EXT3_SYNC_FL)
-                       inode->i_flags |= S_SYNC;
-               else
-                       inode->i_flags &= ~S_SYNC;
-               if (flags & EXT3_APPEND_FL)
-                       inode->i_flags |= S_APPEND;
-               else
-                       inode->i_flags &= ~S_APPEND;
-               if (flags & EXT3_IMMUTABLE_FL)
-                       inode->i_flags |= S_IMMUTABLE;
-               else
-                       inode->i_flags &= ~S_IMMUTABLE;
-               if (flags & EXT3_NOATIME_FL)
-                       inode->i_flags |= S_NOATIME;
-               else
-                       inode->i_flags &= ~S_NOATIME;
+               ext3_set_inode_flags(inode);
                inode->i_ctime = CURRENT_TIME;
 
                err = ext3_mark_iloc_dirty(handle, inode, &iloc);

diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 0612ce8e8274965aa31e86fd1405393d5693ec7d..b3da32479e13fb535c47ae525b9d87598c604499 100644 (file)
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -730,6 +730,7 @@ extern void ext3_discard_prealloc (struct inode *);
 extern void ext3_dirty_inode(struct inode *);
 extern int ext3_change_inode_journal_flag(struct inode *, int);
 extern void ext3_truncate (struct inode *);
+extern void ext3_set_inode_flags(struct inode *);
 
 /* ioctl.c */
 extern int ext3_ioctl (struct inode *, struct file *, unsigned int,