kernel - Fix mtime for deferred writes from mmap R+W
authorMatthew Dillon <dillon@apollo.backplane.com>
Mon, 13 Nov 2017 22:03:20 +0000 (14:03 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Mon, 13 Nov 2017 22:03:20 +0000 (14:03 -0800)
* When a program modifies a file via a shared R+W mmap and does not
  msync() it, the pages do not get synchronized to the filesystem until
  potentially up to 30 seconds later.  Our filesystems then record a
  mtime as-of when that synchronization occurs rather than when the
  modification was made to the pages.

* We cannot easily determine when the modification was made, but we do
  track close()'s on file pointers that were opened writable.  So what
  we do is record the timestamp as of the last close() of a file pointer
  that was opened for writing.  Then later on we pages are synchronizded,
  we use this timestamp for any mtime update.  It isn't perfect, but its
  better than it was before.

* This is not a perfect fix.  The mtime really needs to be updated at the
  time of the close() in this situation as well, but it will fix 'make'
  related issues for files that are modified with mmap rather than
  write() (aka such as ld.gold appears to do).

Reported-by: zrj
sys/kern/vfs_default.c
sys/sys/vnode.h
sys/vfs/hammer/hammer_vnops.c
sys/vfs/hammer2/hammer2_vnops.c
sys/vfs/tmpfs/tmpfs_vnops.c
sys/vfs/ufs/inode.h
sys/vfs/ufs/ufs_readwrite.c
sys/vfs/ufs/ufs_vnops.c

index c84da84..776af1c 100644 (file)
@@ -1190,6 +1190,10 @@ vop_stdopen(struct vop_open_args *ap)
  * (struct vnode *a_vp, int a_fflag)
  *
  * a_fflag: note, 'F' modes, e.g. FREAD, FWRITE.  same as a_mode in stdopen?
+ *
+ * v_lastwrite_ts is used to record the timestamp that should be used to
+ * set the file mtime for any asynchronously flushed pages modified via
+ * mmap(), which can occur after the last close().
  */
 int
 vop_stdclose(struct vop_close_args *ap)
@@ -1203,6 +1207,7 @@ vop_stdclose(struct vop_close_args *ap)
                KASSERT(vp->v_writecount > 0,
                        ("VOP_STDCLOSE: BAD WRITECOUNT %p %d",
                        vp, vp->v_writecount));
+               vfs_timestamp(&vp->v_lastwrite_ts);
                atomic_add_int(&vp->v_writecount, -1);
        }
        atomic_add_int(&vp->v_opencount, -1);
index cd7797b..b4d22f7 100644 (file)
@@ -112,6 +112,14 @@ struct mountctl_opt {
  * deal with clustered cache coherency issues and, more immediately, to
  * protect operations associated with the kernel-managed journaling module.
  *
+ * Generally speaking modification timestamps for inodes get set as-of the
+ * time the write occurs.  However, files mmap()'d R+W may not see the
+ * underlying pages get flushed until well after close().  In this situation,
+ * v_lastwrite_ts is set to the timestamp that the filesystem should use
+ * for such updates.  VFS's should be aware that symlink data sometimes uses
+ * the file write mechanism to set the symlink content and v_writecount
+ * will probably be 0.  v_lastwrite_ts should not be used in that situation.
+ *
  * NOTE: Certain fields within the vnode structure requires v_token to be
  *      held.  The vnode's normal lock need not be held when accessing
  *      these fields as long as the vnode is deterministically referenced
@@ -186,6 +194,7 @@ struct vnode {
        } v_pollinfo;
        struct vmresident *v_resident;          /* optional vmresident */
        struct mount *v_pfsmp;                  /* real mp for pfs/nullfs mt */
+       struct timespec v_lastwrite_ts;         /* async mmap flush ts */
 };
 #define        v_socket        v_un.vu_socket
 #define v_umajor       v_un.vu_cdev.vu_umajor
index d91f132..6497f03 100644 (file)
@@ -496,6 +496,7 @@ hammer_vop_write(struct vop_write_args *ap)
        hammer_inode_t ip;
        hammer_mount_t hmp;
        thread_t td;
+       struct vnode *vp;
        struct uio *uio;
        int offset;
        off_t base_offset;
@@ -508,7 +509,8 @@ hammer_vop_write(struct vop_write_args *ap)
        int seqcount;
        int bigwrite;
 
-       if (ap->a_vp->v_type != VREG)
+       vp = ap->a_vp;
+       if (vp->v_type != VREG)
                return (EINVAL);
        ip = VTOI(ap->a_vp);
        hmp = ip->hmp;
@@ -525,6 +527,16 @@ hammer_vop_write(struct vop_write_args *ap)
        hammer_start_transaction(&trans, hmp);
        uio = ap->a_uio;
 
+       /*
+        * Use v_lastwrite_ts if file not open for writing
+        * (i.e. a late msync)
+        */
+       if (vp->v_writecount == 0) {
+               trans.time = vp->v_lastwrite_ts.tv_sec * 1000000 +
+                            vp->v_lastwrite_ts.tv_nsec / 1000;
+       }
+
+
        /*
         * Check append mode
         */
index c092e8c..5bed6a7 100644 (file)
@@ -1089,7 +1089,15 @@ hammer2_write_file(hammer2_inode_t *ip, struct uio *uio,
        } else if (modified) {
                hammer2_mtx_ex(&ip->lock);
                hammer2_inode_modify(ip);
-               hammer2_update_time(&ip->meta.mtime);
+               if (ip->vp && ip->vp->v_writecount == 0 &&
+                   ip->vp->v_type == VREG) {
+                       ip->meta.mtime =
+                               (unsigned long)ip->vp->v_lastwrite_ts.tv_sec *
+                                1000000 +
+                               ip->vp->v_lastwrite_ts.tv_nsec / 1000;
+               } else {
+                       hammer2_update_time(&ip->meta.mtime);
+               }
                if (ip->flags & HAMMER2_INODE_MODIFIED)
                        hammer2_inode_chain_sync(ip);
                hammer2_mtx_unlock(&ip->lock);
index 407c8cd..34c7f51 100644 (file)
@@ -742,8 +742,13 @@ tmpfs_write(struct vop_write_args *ap)
         * order to be able to dispose of the buffer cache buffer without
         * flushing it.
         */
-       if (uio->uio_segflg != UIO_NOCOPY)
+       if (vp->v_writecount) {
                node->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_MODIFIED;
+       } else {
+               node->tn_mtime = vp->v_lastwrite_ts.tv_sec;
+               node->tn_mtimensec = vp->v_lastwrite_ts.tv_nsec;
+       }
+
        if (extended)
                node->tn_status |= TMPFS_NODE_CHANGED;
 
index 3b6636d..f04f6ff 100644 (file)
@@ -137,6 +137,7 @@ struct inode {
 #define        IN_EXLOCK       0x0040          /* File has exclusive lock. */
 #define        IN_HASHED       0x0080          /* Inode is on hash list */
 #define        IN_LAZYMOD      0x0100          /* Modified, but don't write yet. */
+#define IN_WRITING     0x0200          /* Write in progress */
 
 #if defined(_KERNEL) || defined(_KERNEL_STRUCTURES)
 
index b67e06c..a986f7d 100644 (file)
@@ -282,6 +282,8 @@ ffs_write(struct vop_write_args *ap)
        if ((ioflag & IO_SYNC) && !DOINGASYNC(vp))
                flags |= B_SYNC;
 
+       ip->i_flag |= IN_WRITING;
+
        for (error = 0; uio->uio_resid > 0;) {
                lbn = lblkno(fs, uio->uio_offset);
                blkoffset = blkoff(fs, uio->uio_offset);
@@ -405,7 +407,10 @@ ffs_write(struct vop_write_args *ap)
                }
        } else if (resid > uio->uio_resid && (ioflag & IO_SYNC)) {
                error = ffs_update(vp, 1);
+       } else {
+               ufs_itimes(vp);
        }
+       ip->i_flag &= ~IN_WRITING;
 
        return (error);
 }
index 1086651..003f9a2 100644 (file)
@@ -156,21 +156,27 @@ ufs_itimes(struct vnode *vp)
                ip->i_flag |= IN_LAZYMOD;
        else
                ip->i_flag |= IN_MODIFIED;
+
        if ((vp->v_mount->mnt_flag & MNT_RDONLY) == 0) {
                vfs_timestamp(&ts);
                if (ip->i_flag & IN_ACCESS) {
                        ip->i_atime = ts.tv_sec;
                        ip->i_atimensec = ts.tv_nsec;
                }
+               if (ip->i_flag & IN_CHANGE) {
+                       ip->i_ctime = ts.tv_sec;
+                       ip->i_ctimensec = ts.tv_nsec;
+               }
                if (ip->i_flag & IN_UPDATE) {
+                       if (vp->v_writecount == 0 &&
+                           (ip->i_flag & IN_WRITING) &&
+                           vp->v_type == VREG) {
+                               ts = vp->v_lastwrite_ts;
+                       }
                        ip->i_mtime = ts.tv_sec;
                        ip->i_mtimensec = ts.tv_nsec;
                        ip->i_modrev++;
                }
-               if (ip->i_flag & IN_CHANGE) {
-                       ip->i_ctime = ts.tv_sec;
-                       ip->i_ctimensec = ts.tv_nsec;
-               }
        }
        ip->i_flag &= ~(IN_ACCESS | IN_CHANGE | IN_UPDATE);
 }