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