Subject: Re: kern/25279: NFS read doesn't update atime
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 06/27/2005 12:48:16
--yrj/dFKFPuw6o+aM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Jun 27, 2005 at 08:02:59AM +0900, YAMAMOTO Takashi wrote:
> > > doesn't it make write(2) and mmap'ed accesses update atime
> > > in somewhat unpredictable manner?
> > 
> > For mmap'ed accesses, probably. But I don't think this is a problem,
> > because if we're faulting in a page via mmap(), we're reading the file,
> > isn't it ?
> 
> i agree that it shouldn't be a problem, because it's
> the pre-ubc behaviour, iirc.

Ha yes, so this should be restored too.

> 
> > [...]
> > Yes, this is the direction I initially took (my idea was to add another vnop,
> > but it's the same idea). The problem is that it can't be pulled up to the
> > netbsd-2 branch, and maybe not the netbsd-3 brnach either (this is a kernel
> > API change).
> 
> changing getpages semantics is also an API change.

Not as much as changing the prototype of setattr, or adding a new vnode
operation.

On Mon, Jun 27, 2005 at 08:33:35AM +0900, YAMAMOTO Takashi wrote:
> > Hum, I just tested. Yes the atime is updated on a write, and testing
> > PGO_OVERWRITE doesn't fix the problem :(
> 
> i think testing access_type works for -current.
> it doesn't work for netbsd-2, though.

Hum, I tried this (see attached patch), and atime is still updated when writing
to an existing file (my test is: echo >> some_file).

Thinking more about it, and given that the getpage vs atime issue affects not
only NFS but also mmap, maybe we should fix this in the other way ?
That is, have getpage always update the atime, exept when it's used for a
read/modify/write operation on a file ?

I don't know how this can be done yet.


-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--yrj/dFKFPuw6o+aM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.getpage"

Index: ext2fs/ext2fs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ext2fs/ext2fs_vnops.c,v
retrieving revision 1.59
diff -u -r1.59 ext2fs_vnops.c
--- ext2fs/ext2fs_vnops.c	26 Feb 2005 22:32:20 -0000	1.59
+++ ext2fs/ext2fs_vnops.c	27 Jun 2005 10:39:33 -0000
@@ -106,6 +106,7 @@
 	__P((struct vnode *, int, struct ucred *, struct proc *));
 static int ext2fs_chown
 	__P((struct vnode *, uid_t, gid_t, struct ucred *, struct proc *));
+static int ext2fs_getpages __P((void *));
 
 union _qcvt {
 	int64_t	qcvt;
@@ -1475,6 +1476,28 @@
 	return (0);
 }
 
+static int
+ext2fs_getpages(void *v)
+{
+	struct vop_getpages_args /* {
+		struct vnode *a_vp;
+		voff_t a_offset;
+		struct vm_page **a_m;
+		int *a_count;
+		int a_centeridx;
+		vm_prot_t a_access_type;
+		int a_advice;
+		int a_flags;
+	} */ *ap = v;
+	struct vnode *vp = ap->a_vp;
+	struct inode *ip = VTOI(vp);
+
+	if (!(vp->v_mount->mnt_flag & MNT_NOATIME) &&
+	    !(ap->a_access_type & VM_PROT_WRITE))
+		ip->i_flag |= IN_ACCESS;
+	return genfs_getpages(v);
+}
+
 /* Global vfs data structures for ext2fs. */
 int (**ext2fs_vnodeop_p) __P((void *));
 const struct vnodeopv_entry_desc ext2fs_vnodeop_entries[] = {
@@ -1523,7 +1546,7 @@
 	{ &vop_truncate_desc, ext2fs_truncate },	/* truncate */
 	{ &vop_update_desc, ext2fs_update },		/* update */
 	{ &vop_bwrite_desc, vn_bwrite },		/* bwrite */
-	{ &vop_getpages_desc, genfs_getpages },		/* getpages */
+	{ &vop_getpages_desc, ext2fs_getpages },	/* getpages */
 	{ &vop_putpages_desc, genfs_putpages },		/* putpages */
 	{ NULL, NULL }
 };
Index: ffs/ffs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vnops.c,v
retrieving revision 1.69
diff -u -r1.69 ffs_vnops.c
--- ffs/ffs_vnops.c	26 Feb 2005 22:32:20 -0000	1.69
+++ ffs/ffs_vnops.c	27 Jun 2005 10:39:33 -0000
@@ -535,6 +535,9 @@
 		}
 		return EINVAL;
 	}
+	if (!(vp->v_mount->mnt_flag & MNT_NOATIME) &&
+	    !(ap->a_access_type & VM_PROT_WRITE))
+		ip->i_flag |= IN_ACCESS;
 	return genfs_getpages(v);
 }
 
Index: lfs/lfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/lfs/lfs_vnops.c,v
retrieving revision 1.152
diff -u -r1.152 lfs_vnops.c
--- lfs/lfs_vnops.c	29 May 2005 21:25:24 -0000	1.152
+++ lfs/lfs_vnops.c	27 Jun 2005 10:39:33 -0000
@@ -1424,6 +1424,9 @@
 	if ((ap->a_access_type & VM_PROT_WRITE) != 0) {
 		LFS_SET_UINO(VTOI(ap->a_vp), IN_MODIFIED);
 	}
+	if (!(ap->a_vp->v_mount->mnt_flag & MNT_NOATIME) &&
+	    !(ap->a_access_type & VM_PROT_WRITE))
+		VTOI(ap->a_vp)->i_flag |= IN_ACCESS;
 
 	/*
 	 * we're relying on the fact that genfs_getpages() always read in

--yrj/dFKFPuw6o+aM--