msync: massive code cleanup of sys_msync()

mm/msync.c +4 -2

@@ -33,71 +34,60 @@ asmlinkage long sys_msync(unsigned long start, size_t len, int flags)
 	unsigned long end;
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
-	int unmapped_error = 0;
-	int error = -EINVAL;
+	int error = 0, unmapped_error = 0;
+
+	if ((flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC)) ||
+			(start & ~PAGE_MASK) ||

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Signed-off-by Anton Salikhmetov <[email protected]>

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Signed-off-by Anton Salikhmetov <[email protected]>

---

fs/buffer.c +1 -0

index 7249e01..09adf7e 100644
@@ -719,6 +719,7 @@ static int __set_page_dirty(struct page *page,
 	}
 	write_unlock_irq(&mapping->tree_lock);
 	__mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
+	set_bit(AS_MCTIME, &mapping->flags);
 
 	return 1;
 }

fs/inode.c +13 -0

index ed35383..c5b954e 100644
@@ -22,6 +22,7 @@
 #include <linux/bootmem.h>
 #include <linux/inotify.h>
 #include <linux/mount.h>
+#include <linux/file.h>
 
 /*
  * This is needed for the following functions:

@@ -1282,6 +1283,18 @@ void file_update_time(struct file *file)
 
 EXPORT_SYMBOL(file_update_time);
 
+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+	if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+		get_file(file);
+		file_update_time(file);
+		fput(file);
+	}
+}
+
 int inode_needs_sync(struct inode *inode)
 {
 	if (IS_SYNC(inode))

fs/sync.c +2 -0

index 7cd005e..df57507 100644
@@ -87,6 +87,8 @@ long do_fsync(struct file *file, int datasync)
 		goto out;
 	}
 
+	mapped_file_update_time(file);
+
 	ret = filemap_fdatawrite(mapping);
 
 	/*

include/linux/fs.h +1 -0

index b3ec4a4..0b05118 100644
@@ -1978,6 +1978,7 @@ extern int inode_change_ok(struct inode *, struct iattr *);
 extern int __must_check inode_setattr(struct inode *, struct iattr *);
 
 extern void file_update_time(struct file *file);
+extern void mapped_file_update_time(struct file *file);
 
 static inline ino_t parent_ino(struct dentry *dentry)
 {

include/linux/pagemap.h +2 -1

index db8a410..bf0f9e7 100644
@@ -17,8 +17,9 @@
  * Bits in mapping->flags.  The lower __GFP_BITS_SHIFT bits are the page
  * allocation mode flags.
  */
-#define	AS_EIO		(__GFP_BITS_SHIFT + 0)	/* IO error on async write */
+#define AS_EIO		(__GFP_BITS_SHIFT + 0)	/* IO error on async write */
 #define AS_ENOSPC	(__GFP_BITS_SHIFT + 1)	/* ENOSPC on async write */
+#define AS_MCTIME	(__GFP_BITS_SHIFT + 2)	/* mtime and ctime to update */
 
 static inline void mapping_set_error(struct address_space *mapping, int error)
 {

mm/msync.c +16 -8

index e788f7b..9d0a8f9 100644
@@ -5,6 +5,7 @@
  * Copyright (C) 1994-1999  Linus Torvalds
  *
  * Substantial code cleanup.
+ * Updating the ctime and mtime stamps for memory mapped files.
  * Copyright (C) 2008 Anton Salikhmetov <[email protected]>
  */
 

@@ -22,6 +23,10 @@
  * Nor does it mark the relevant pages dirty (it used to up to 2.6.17).
  * Now it doesn't do anything, since dirty pages are properly tracked.
  *
+ * The msync() system call updates the ctime and mtime fields for
+ * the mapped file when called with the MS_SYNC or MS_ASYNC flags
+ * according to the POSIX standard.
+ *
  * The application may now run fsync() to
  * write out the dirty pages and wait on the writeout and check the result.
  * Or the application may run fadvise(FADV_DONTNEED) against the fd to start

@@ -74,14 +79,17 @@ asmlinkage long sys_msync(unsigned long start, size_t len, int flags)
 			break;
 		}
 		file = vma->vm_file;
-		if ((flags & MS_SYNC) && file && (vma->vm_flags & VM_SHARED)) {
-			get_file(file);
-			up_read(&mm->mmap_sem);
-			error = do_fsync(file, 0);
-			fput(file);
-			if (error)
-				return error;
-			down_read(&mm->mmap_sem);
+		if (file && (vma->vm_flags & VM_SHARED)) {
+			mapped_file_update_time(file);
+			if (flags & MS_SYNC) {
+				get_file(file);
+				up_read(&mm->mmap_sem);
+				error = do_fsync(file, 0);
+				fput(file);
+				if (error)
+					return error;
+				down_read(&mm->mmap_sem);
+			}
 		}
 
 		start = vma->vm_end;

mm/page-writeback.c +0 -0

index d55cfca..a85df0b 100644

Quoted 9 lines · Anton Salikhmetov

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

What happens if the application does not issue either an msync or an fsync call, but either just munmap's the region or just keeps on manipulating it? It appears to me that the file times will never be updated in these cases.

It seems to me that the file times should be updated eventually, and perhaps even regularly if the file is being constantly updated via the mmap'd region.

    Thanx...

       ps

Quoted 135 lines

Signed-off-by Anton Salikhmetov <[email protected]>

---

fs/buffer.c +1 -0

index 7249e01..09adf7e 100644
@@ -719,6 +719,7 @@ static int __set_page_dirty(struct page *page,
      }
      write_unlock_irq(&mapping->tree_lock);
      __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
+     set_bit(AS_MCTIME, &mapping->flags);
 
      return 1;
 }

fs/inode.c +13 -0

index ed35383..c5b954e 100644
@@ -22,6 +22,7 @@
 #include <linux/bootmem.h>
 #include <linux/inotify.h>
 #include <linux/mount.h>
+#include <linux/file.h>
 
 /*
  * This is needed for the following functions:

@@ -1282,6 +1283,18 @@ void file_update_time(struct file *file)
 
 EXPORT_SYMBOL(file_update_time);
 
+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+     if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+             get_file(file);
+             file_update_time(file);
+             fput(file);
+     }
+}
+
 int inode_needs_sync(struct inode *inode)
 {
      if (IS_SYNC(inode))

fs/sync.c +2 -0

index 7cd005e..df57507 100644
@@ -87,6 +87,8 @@ long do_fsync(struct file *file, int datasync)
              goto out;
      }
 
+     mapped_file_update_time(file);
+
      ret = filemap_fdatawrite(mapping);
 
      /*

include/linux/fs.h +1 -0

index b3ec4a4..0b05118 100644
@@ -1978,6 +1978,7 @@ extern int inode_change_ok(struct inode *, struct iattr *);
 extern int __must_check inode_setattr(struct inode *, struct iattr *);
 
 extern void file_update_time(struct file *file);
+extern void mapped_file_update_time(struct file *file);
 
 static inline ino_t parent_ino(struct dentry *dentry)
 {

include/linux/pagemap.h +2 -1

index db8a410..bf0f9e7 100644
@@ -17,8 +17,9 @@
  * Bits in mapping->flags.  The lower __GFP_BITS_SHIFT bits are the page
  * allocation mode flags.
  */
-#define      AS_EIO          (__GFP_BITS_SHIFT + 0)  /* IO error on async write */
+#define AS_EIO               (__GFP_BITS_SHIFT + 0)  /* IO error on async write */
 #define AS_ENOSPC    (__GFP_BITS_SHIFT + 1)  /* ENOSPC on async write */
+#define AS_MCTIME    (__GFP_BITS_SHIFT + 2)  /* mtime and ctime to update */
 
 static inline void mapping_set_error(struct address_space *mapping, int error)
 {

mm/msync.c +16 -8

index e788f7b..9d0a8f9 100644
@@ -5,6 +5,7 @@
  * Copyright (C) 1994-1999  Linus Torvalds
  *
  * Substantial code cleanup.
+ * Updating the ctime and mtime stamps for memory mapped files.
  * Copyright (C) 2008 Anton Salikhmetov <[email protected]>
  */
 

@@ -22,6 +23,10 @@
  * Nor does it mark the relevant pages dirty (it used to up to 2.6.17).
  * Now it doesn't do anything, since dirty pages are properly tracked.
  *
+ * The msync() system call updates the ctime and mtime fields for
+ * the mapped file when called with the MS_SYNC or MS_ASYNC flags
+ * according to the POSIX standard.
+ *
  * The application may now run fsync() to
  * write out the dirty pages and wait on the writeout and check the result.
  * Or the application may run fadvise(FADV_DONTNEED) against the fd to start

@@ -74,14 +79,17 @@ asmlinkage long sys_msync(unsigned long start, size_t len, int flags)
                      break;
              }
              file = vma->vm_file;
-             if ((flags & MS_SYNC) && file && (vma->vm_flags & VM_SHARED)) {
-                     get_file(file);
-                     up_read(&mm->mmap_sem);
-                     error = do_fsync(file, 0);
-                     fput(file);
-                     if (error)
-                             return error;
-                     down_read(&mm->mmap_sem);
+             if (file && (vma->vm_flags & VM_SHARED)) {
+                     mapped_file_update_time(file);
+                     if (flags & MS_SYNC) {
+                             get_file(file);
+                             up_read(&mm->mmap_sem);
+                             error = do_fsync(file, 0);
+                             fput(file);
+                             if (error)
+                                     return error;
+                             down_read(&mm->mmap_sem);
+                     }
              }
 

Quoted 8 lines

mm/msync.c +0 -0

              start = vma->vm_end;

mm/page-writeback.c +0 -0

index d55cfca..a85df0b 100644

mm/page-writeback.c int __set_page_dirty_nobuffers(struct page *page) · line 1031

Quoted 7 lines

mm/page-writeback.c +1 -0

@@ -1025,6 +1025,7 @@ int __set_page_dirty_nobuffers(struct page *page)
              if (mapping->host) {
                      /* !PageAnon && !swapper_space */
                      __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
+                     set_bit(AS_MCTIME, &mapping->flags);
              }
              return 1;
      }

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Quoted 7 lines · Anton Salikhmetov

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Sorry, one other issue to throw out too -- an mmap'd block device should also have its inode time fields updated. This is a little tricky because the inode referenced via mapping->host isn't the one that needs to have the time fields updated on.

I have attached the patch that I submitted last. It is quite out of date, but does show my attempt to resolve some of these issues.

    Thanx...

       ps

fs/buffer.c renamed from fs/buffer.c.org +7 -2

@@ -710,6 +710,7 @@ EXPORT_SYMBOL(mark_buffer_dirty_inode);
 int __set_page_dirty_buffers(struct page *page)
 {
        struct address_space * const mapping = page_mapping(page);
+       int ret = 0;
 
        if (unlikely(!mapping))
                return !TestSetPageDirty(page);

@@ -727,7 +728,7 @@ int __set_page_dirty_buffers(struct page
        spin_unlock(&mapping->private_lock);
 
        if (TestSetPageDirty(page))
-               return 0;
+               goto out;
 
        write_lock_irq(&mapping->tree_lock);
        if (page->mapping) {    /* Race with truncate? */

@@ -740,7 +741,11 @@ int __set_page_dirty_buffers(struct page
        }
        write_unlock_irq(&mapping->tree_lock);
        __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
-       return 1;
+       ret = 1;
+out:
+       if (page_mapped(page))
+               set_bit(AS_MCTIME, &mapping->flags);
+       return ret;
 }
 EXPORT_SYMBOL(__set_page_dirty_buffers);
 

fs/fs-writeback.c renamed from fs/fs-writeback.c.org +7 -0

@@ -167,6 +167,13 @@ __sync_single_inode(struct inode *inode,
 
        spin_unlock(&inode_lock);
 
+       if (test_and_clear_bit(AS_MCTIME, &mapping->flags)) {
+               if (S_ISBLK(inode->i_mode))
+                       bd_inode_update_time(inode);
+               else
+                       inode_update_time(inode);
+       }
+
        ret = do_writepages(mapping, wbc);
 
        /* Don't write the inode if only I_DIRTY_PAGES was set */

fs/inode.c renamed from fs/inode.c.org +4 -5

@@ -1201,8 +1201,8 @@ void touch_atime(struct vfsmount *mnt, s
 EXPORT_SYMBOL(touch_atime);
 
 /**
- *     file_update_time        -       update mtime and ctime time
- *     @file: file accessed
+ *     inode_update_time       -       update mtime and ctime time
+ *     @inode: file accessed
  *
  *     Update the mtime and ctime members of an inode and mark the inode
  *     for writeback.  Note that this function is meant exclusively for

@@ -1212,9 +1212,8 @@ EXPORT_SYMBOL(touch_atime);
  *     timestamps are handled by the server.
  */
 
-void file_update_time(struct file *file)
+void inode_update_time(struct inode *inode)
 {
-       struct inode *inode = file->f_path.dentry->d_inode;
        struct timespec now;
        int sync_it = 0;
 

@@ -1238,7 +1237,7 @@ void file_update_time(struct file *file)
                mark_inode_dirty_sync(inode);
 }
 
-EXPORT_SYMBOL(file_update_time);
+EXPORT_SYMBOL(inode_update_time);
 
 int inode_needs_sync(struct inode *inode)
 {

fs/block_dev.c renamed from fs/block_dev.c.org +16 -0

@@ -608,6 +608,22 @@ void bdput(struct block_device *bdev)
 
 EXPORT_SYMBOL(bdput);
 
+void bd_inode_update_time(struct inode *inode)
+{
+       struct block_device *bdev = inode->i_bdev;
+       struct list_head *p;
+
+       if (bdev == NULL)
+               return;
+
+       spin_lock(&bdev_lock);
+       list_for_each(p, &bdev->bd_inodes) {
+               inode = list_entry(p, struct inode, i_devices);
+               inode_update_time(inode);
+       }
+       spin_unlock(&bdev_lock);
+}
+
 static struct block_device *bd_acquire(struct inode *inode)
 {
        struct block_device *bdev;

include/linux/fs.h renamed from include/linux/fs.h.org +6 -1

@@ -1488,6 +1488,7 @@ extern struct block_device *bdget(dev_t)
 extern void bd_set_size(struct block_device *, loff_t size);
 extern void bd_forget(struct inode *inode);
 extern void bdput(struct block_device *);
+extern void bd_inode_update_time(struct inode *);
 extern struct block_device *open_by_devnum(dev_t, unsigned);
 extern const struct address_space_operations def_blk_aops;
 #else

@@ -1892,7 +1893,11 @@ extern int buffer_migrate_page(struct ad
 extern int inode_change_ok(struct inode *, struct iattr *);
 extern int __must_check inode_setattr(struct inode *, struct iattr *);
 
-extern void file_update_time(struct file *file);
+extern void inode_update_time(struct inode *inode);
+static inline void file_update_time(struct file *file)
+{
+       inode_update_time(file->f_path.dentry->d_inode);
+}
 
 static inline ino_t parent_ino(struct dentry *dentry)
 {

include/linux/pagemap.h renamed from include/linux/pagemap.h.org +2 -1

@@ -16,8 +16,9 @@
  * Bits in mapping->flags.  The lower __GFP_BITS_SHIFT bits are the page
  * allocation mode flags.
  */
-#define        AS_EIO          (__GFP_BITS_SHIFT + 0)  /* IO error on async write */
+#define AS_EIO         (__GFP_BITS_SHIFT + 0)  /* IO error on async write */
 #define AS_ENOSPC      (__GFP_BITS_SHIFT + 1)  /* ENOSPC on async write */
+#define AS_MCTIME      (__GFP_BITS_SHIFT + 2)  /* need m/ctime change */
 
 static inline gfp_t mapping_gfp_mask(struct address_space * mapping)
 {

mm/page-writeback.c renamed from mm/page-writeback.c.org +7 -3

@@ -760,8 +760,10 @@ int __set_page_dirty_no_writeback(struct
  */
 int __set_page_dirty_nobuffers(struct page *page)
 {
+       struct address_space *mapping = page_mapping(page);
+       int ret = 0;
+
        if (!TestSetPageDirty(page)) {
-               struct address_space *mapping = page_mapping(page);
                struct address_space *mapping2;
 
                if (!mapping)

@@ -783,9 +785,11 @@ int __set_page_dirty_nobuffers(struct pa
                        /* !PageAnon && !swapper_space */
                        __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
                }
-               return 1;
+               ret = 1;
        }
-       return 0;
+       if (page_mapped(page))
+               set_bit(AS_MCTIME, &mapping->flags);
+       return ret;
 }
 EXPORT_SYMBOL(__set_page_dirty_nobuffers);
 

mm/msync.c renamed from mm/msync.c.org +3 -2

@@ -12,6 +12,7 @@
 #include <linux/mman.h>
 #include <linux/file.h>
 #include <linux/syscalls.h>
+#include <linux/pagemap.h>
 
 /*
  * MS_SYNC syncs the entire file - including mappings.

@@ -77,11 +78,16 @@ asmlinkage long sys_msync(unsigned long
                }
                file = vma->vm_file;
                start = vma->vm_end;
-               if ((flags & MS_SYNC) && file &&
-                               (vma->vm_flags & VM_SHARED)) {
+               if ((flags & (MS_SYNC|MS_ASYNC)) &&
+                   file && (vma->vm_flags & VM_SHARED)) {
                        get_file(file);
                        up_read(&mm->mmap_sem);

2008/1/11, Peter Staubach <[email protected]>:

Quoted 10 lines

Quoted 8 lines · Anton Salikhmetov

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Quoted 8 lines

Sorry, one other issue to throw out too -- an mmap'd block device should also have its inode time fields updated. This is a little tricky because the inode referenced via mapping->host isn't the one that needs to have the time fields updated on.

I have attached the patch that I submitted last. It is quite out of date, but does show my attempt to resolve some of these issues.

Thanks for your feedback!

Now I'm looking at your solution and thinking about which parts of it I could adapt to the infrastructure I'm trying to develop.

However, I would like to address the block device case within
a separate project. But for now, I want the msync() and fsync() system calls to update ctime and mtime at least for memory-mapped regular files properly. I feel that even this little improvement could address many customer's troubles such as the one Jacob Oestergaard reported in the bug #2645.

Quoted 26 lines · Anton Salikhmetov

2008/1/11, Peter Staubach <[email protected]>:

Quoted 10 lines

Quoted 8 lines · Anton Salikhmetov

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Quoted 8 lines

Sorry, one other issue to throw out too -- an mmap'd block device should also have its inode time fields updated. This is a little tricky because the inode referenced via mapping->host isn't the one that needs to have the time fields updated on.

I have attached the patch that I submitted last. It is quite out of date, but does show my attempt to resolve some of these issues.

Thanks for your feedback!

Now I'm looking at your solution and thinking about which parts of it I could adapt to the infrastructure I'm trying to develop.

Quoted 6 lines

However, I would like to address the block device case within
a separate project. But for now, I want the msync() and fsync() system calls to update ctime and mtime at least for memory-mapped regular files properly. I feel that even this little improvement could address many customer's troubles such as the one Jacob Oestergaard reported in the bug #2645.

Not that I disagree and I also have customers who would really like to see this situation addressed so that I can then fix it in RHEL, but the block device issue was raised by Andrew Morton during my first attempt to get a patch integrated.

Just so that you are aware of who has raised which issues... :-)

    Thanx...

       ps

Quoted 7 lines · Anton Salikhmetov

From: Anton Salikhmetov <[email protected]>

The patch contains changes for updating the ctime and mtime fields for memory mapped files:

1) adding a new flag triggering update of the inode data;
2) implementing a helper function for checking that flag and updating ctime and mtime;
3) updating time stamps for mapped files in sys_msync() and do_fsync().

Sorry, one other issue to throw out too -- an mmap'd block device should also have its inode time fields updated. This is a little tricky because the inode referenced via mapping->host isn't the one that needs to have the time fields updated on.

I have attached the patch that I submitted last. It is quite out of date, but does show my attempt to resolve some of these issues.

    Thanx...

       ps

fs/buffer.c renamed from fs/buffer.c.org +7 -2

@@ -710,6 +710,7 @@ EXPORT_SYMBOL(mark_buffer_dirty_inode);
 int __set_page_dirty_buffers(struct page *page)
 {
        struct address_space * const mapping = page_mapping(page);
+       int ret = 0;
 
        if (unlikely(!mapping))
                return !TestSetPageDirty(page);

@@ -727,7 +728,7 @@ int __set_page_dirty_buffers(struct page
        spin_unlock(&mapping->private_lock);
 
        if (TestSetPageDirty(page))
-               return 0;
+               goto out;
 
        write_lock_irq(&mapping->tree_lock);
        if (page->mapping) {    /* Race with truncate? */

@@ -740,7 +741,11 @@ int __set_page_dirty_buffers(struct page
        }
        write_unlock_irq(&mapping->tree_lock);
        __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
-       return 1;
+       ret = 1;
+out:
+       if (page_mapped(page))
+               set_bit(AS_MCTIME, &mapping->flags);
+       return ret;
 }
 EXPORT_SYMBOL(__set_page_dirty_buffers);
 

fs/fs-writeback.c renamed from fs/fs-writeback.c.org +0 -0

        ret = do_writepages(mapping, wbc);
 
        /* Don't write the inode if only I_DIRTY_PAGES was set */

fs/inode.c renamed from fs/inode.c.org +4 -5

@@ -1201,8 +1201,8 @@ void touch_atime(struct vfsmount *mnt, s
 EXPORT_SYMBOL(touch_atime);
 
 /**
- *     file_update_time        -       update mtime and ctime time
- *     @file: file accessed
+ *     inode_update_time       -       update mtime and ctime time
+ *     @inode: file accessed
  *
  *     Update the mtime and ctime members of an inode and mark the inode
  *     for writeback.  Note that this function is meant exclusively for

@@ -1212,9 +1212,8 @@ EXPORT_SYMBOL(touch_atime);
  *     timestamps are handled by the server.
  */
 
-void file_update_time(struct file *file)
+void inode_update_time(struct inode *inode)
 {
-       struct inode *inode = file->f_path.dentry->d_inode;
        struct timespec now;
        int sync_it = 0;
 

@@ -1238,7 +1237,7 @@ void file_update_time(struct file *file)
                mark_inode_dirty_sync(inode);
 }
 
-EXPORT_SYMBOL(file_update_time);
+EXPORT_SYMBOL(inode_update_time);
 
 int inode_needs_sync(struct inode *inode)
 {

fs/block_dev.c renamed from fs/block_dev.c.org +16 -0

@@ -608,6 +608,22 @@ void bdput(struct block_device *bdev)
 
 EXPORT_SYMBOL(bdput);
 
+void bd_inode_update_time(struct inode *inode)
+{
+       struct block_device *bdev = inode->i_bdev;
+       struct list_head *p;
+
+       if (bdev == NULL)
+               return;
+
+       spin_lock(&bdev_lock);
+       list_for_each(p, &bdev->bd_inodes) {
+               inode = list_entry(p, struct inode, i_devices);
+               inode_update_time(inode);
+       }
+       spin_unlock(&bdev_lock);
+}
+
 static struct block_device *bd_acquire(struct inode *inode)
 {
        struct block_device *bdev;

include/linux/fs.h renamed from include/linux/fs.h.org +6 -1

@@ -1488,6 +1488,7 @@ extern struct block_device *bdget(dev_t)
 extern void bd_set_size(struct block_device *, loff_t size);
 extern void bd_forget(struct inode *inode);
 extern void bdput(struct block_device *);
+extern void bd_inode_update_time(struct inode *);
 extern struct block_device *open_by_devnum(dev_t, unsigned);
 extern const struct address_space_operations def_blk_aops;
 #else

@@ -1892,7 +1893,11 @@ extern int buffer_migrate_page(struct ad
 extern int inode_change_ok(struct inode *, struct iattr *);
 extern int __must_check inode_setattr(struct inode *, struct iattr *);
 
-extern void file_update_time(struct file *file);
+extern void inode_update_time(struct inode *inode);
+static inline void file_update_time(struct file *file)
+{
+       inode_update_time(file->f_path.dentry->d_inode);
+}
 
 static inline ino_t parent_ino(struct dentry *dentry)
 {

include/linux/pagemap.h renamed from include/linux/pagemap.h.org +2 -1

@@ -16,8 +16,9 @@
  * Bits in mapping->flags.  The lower __GFP_BITS_SHIFT bits are the page
  * allocation mode flags.
  */
-#define        AS_EIO          (__GFP_BITS_SHIFT + 0)  /* IO error on async write */
+#define AS_EIO         (__GFP_BITS_SHIFT + 0)  /* IO error on async write */
 #define AS_ENOSPC      (__GFP_BITS_SHIFT + 1)  /* ENOSPC on async write */
+#define AS_MCTIME      (__GFP_BITS_SHIFT + 2)  /* need m/ctime change */
 
 static inline gfp_t mapping_gfp_mask(struct address_space * mapping)
 {

mm/page-writeback.c renamed from mm/page-writeback.c.org +7 -3

@@ -760,8 +760,10 @@ int __set_page_dirty_no_writeback(struct
  */
 int __set_page_dirty_nobuffers(struct page *page)
 {
+       struct address_space *mapping = page_mapping(page);
+       int ret = 0;
+
        if (!TestSetPageDirty(page)) {
-               struct address_space *mapping = page_mapping(page);
                struct address_space *mapping2;
 
                if (!mapping)

@@ -783,9 +785,11 @@ int __set_page_dirty_nobuffers(struct pa
                        /* !PageAnon && !swapper_space */
                        __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
                }
-               return 1;
+               ret = 1;
        }
-       return 0;
+       if (page_mapped(page))
+               set_bit(AS_MCTIME, &mapping->flags);
+       return ret;
 }
 EXPORT_SYMBOL(__set_page_dirty_nobuffers);
 

mm/msync.c renamed from mm/msync.c.org +2 -2

@@ -12,6 +12,7 @@
 #include <linux/mman.h>
 #include <linux/file.h>
 #include <linux/syscalls.h>
+#include <linux/pagemap.h>
 
 /*
  * MS_SYNC syncs the entire file - including mappings.

@@ -77,11 +78,16 @@ asmlinkage long sys_msync(unsigned long
                }
                file = vma->vm_file;
                start = vma->vm_end;
-               if ((flags & MS_SYNC) && file &&
-                               (vma->vm_flags & VM_SHARED)) {
+               if ((flags & (MS_SYNC|MS_ASYNC)) &&

mm/msync.c +4 -8

@@ -74,14 +79,17 @@ asmlinkage long sys_msync(unsigned long start, size_t len, int flags)
 			break;
 		}
 		file = vma->vm_file;
-		if ((flags & MS_SYNC) && file && (vma->vm_flags & VM_SHARED)) {
-			get_file(file);
-			up_read(&mm->mmap_sem);
-			error = do_fsync(file, 0);
-			fput(file);
-			if (error)
-				return error;
-			down_read(&mm->mmap_sem);
+		if (file && (vma->vm_flags & VM_SHARED)) {
+			mapped_file_update_time(file);
+			if (flags & MS_SYNC) {
+				get_file(file);

fs/inode.c void file_update_time(struct file *file) · line 1297

Quoted 12 lines · Anton Salikhmetov

fs/inode.c +12 -0

+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+	if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+		get_file(file);
+		file_update_time(file);
+		fput(file);
+	}
+}
+

I don't think you need the get/put file stuff here, because

Quoted 16 lines · Anton Salikhmetov

fs/inode.c void file_update_time(struct file *file) · line 1297

Quoted 13 lines

fs/inode.c +12 -0

+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+   if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+           get_file(file);
+           file_update_time(file);
+           fput(file);
+   }
+}
+

I don't think you need the get/put file stuff here, because

BTW, the reason for me noticing this is that if it would be needed there is a race condition right there, who is to say that the file pointer you're deref'ing in your test condition isn't a dead one already.

2008/1/12, Peter Zijlstra <[email protected]>:

Quoted 19 lines · Peter Zijlstra

Quoted 16 lines · Anton Salikhmetov

fs/inode.c void file_update_time(struct file *file) · line 1297

Quoted 13 lines

fs/inode.c +12 -0

+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+   if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+           get_file(file);
+           file_update_time(file);
+           fput(file);
+   }
+}
+

I don't think you need the get/put file stuff here, because

Quoted 3 lines

BTW, the reason for me noticing this is that if it would be needed there is a race condition right there, who is to say that the file pointer you're deref'ing in your test condition isn't a dead one already.

So, in your opinion, is it at all needed here to play with the file reference counter? May I drop the get_file() and fput() calls from the sys_msync() function?

Quoted 20 lines

2008/1/12, Peter Zijlstra <[email protected]>:

Quoted 19 lines · Peter Zijlstra

Quoted 16 lines · Anton Salikhmetov

fs/inode.c void file_update_time(struct file *file) · line 1297

Quoted 13 lines

fs/inode.c +12 -0

+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+   if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+           get_file(file);
+           file_update_time(file);
+           fput(file);
+   }
+}
+

I don't think you need the get/put file stuff here, because

Quoted 7 lines

Quoted 3 lines

BTW, the reason for me noticing this is that if it would be needed there is a race condition right there, who is to say that the file pointer you're deref'ing in your test condition isn't a dead one already.

So, in your opinion, is it at all needed here to play with the file reference counter? May I drop the get_file() and fput() calls from the sys_msync() function?

No, the ones in sys_msync() around calling do_fsync() are most definately needed because we release mmap_sem there.

What I'm saying is that you can remove the get_file()/fput() calls from your new mapped_file_update_time() function.

fs/inode.c void file_update_time(struct file *file) · line 1297

Quoted 13 lines

fs/inode.c +12 -0

+/*
+ * Update the ctime and mtime stamps after checking if they are to be updated.
+ */
+void mapped_file_update_time(struct file *file)
+{
+     if (test_and_clear_bit(AS_MCTIME, &file->f_mapping->flags)) {
+             get_file(file);
+             file_update_time(file);
+             fput(file);
+     }
+}
+

I don't think you need the get/put file stuff here, because

fs/sync.c long do_fsync(struct file *file, int datasync) · line 94

Quoted 9 lines

fs/sync.c +2 -0

@@ -87,6 +87,8 @@ long do_fsync(struct file *file, int datasync)
              goto out;
      }
 
+     mapped_file_update_time(file);
+
      ret = filemap_fdatawrite(mapping);
 
      /*

at this call-site we already hold an extra reference on the file, and

Quoted 25 lines

mm/msync.c +11 -8

@@ -74,14 +79,17 @@ asmlinkage long sys_msync(unsigned long start, size_t len, int flags)
                      break;
              }
              file = vma->vm_file;
-             if ((flags & MS_SYNC) && file && (vma->vm_flags & VM_SHARED)) {
-                     get_file(file);
-                     up_read(&mm->mmap_sem);
-                     error = do_fsync(file, 0);
-                     fput(file);
-                     if (error)
-                             return error;
-                     down_read(&mm->mmap_sem);
+             if (file && (vma->vm_flags & VM_SHARED)) {
+                     mapped_file_update_time(file);
+                     if (flags & MS_SYNC) {
+                             get_file(file);
+                             up_read(&mm->mmap_sem);
+                             error = do_fsync(file, 0);
+                             fput(file);
+                             if (error)
+                                     return error;
+                             down_read(&mm->mmap_sem);
+                     }
              }
 

mm/msync.c asmlinkage long sys_msync(unsigned long start, size_t len, int flags) · line 95

Quoted 1 line

mm/msync.c +0 -0

              start = vma->vm_end;

here we hold the mmap_sem so the vma reference on the file can't go away.