Date: Wed, 4 Oct 2023 08:40:10 -0700 From: Alan Somers <asomers@freebsd.org> To: Mark Johnston <markj@freebsd.org> Cc: freebsd-hackers@freebsd.org, rmacklem@freebsd.org Subject: Re: copy_file_range() doesn't update the atime of an empty file Message-ID: <CAOtMX2h7QLqLHPm-gUMDJKeR8oyAXssn2vxkJ8xNgBBT6Cc3bw@mail.gmail.com> In-Reply-To: <ZR2FUeIhO7DIQIpj@nuc> References: <ZR2FUeIhO7DIQIpj@nuc>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 4, 2023 at 8:31=E2=80=AFAM Mark Johnston <markj@freebsd.org> wr= ote: > > For a while, Jenkins has been complaining that one of the tmpfs tests is > failing: > https://ci.freebsd.org/job/FreeBSD-main-amd64-test/23814/testReport/junit= /sys.fs.tmpfs/times_test/empty/ > > This has been happening since commit > 8113cc827611a88540736c92ced7d3a7020a1723, which converted cat(1) to use > copy_file_range(2). The test in question creates an empty file, waits > for a second, then cat(1)s it and checks that the file's atime was > updated. After the aforementioned commit, the atime is not updated. > > I believe the essential difference is that a zero-length read(2) results > in a call to VOP_READ(), which results in an updated atime even if no > bytes were read. For instance, ffs_read() sets IN_ACCESS so long as the > routine doesn't return an error. (I'm not sure if the mtime is > correspondingly updated upon a zero-length write.) > > copy_file_range() on the other hand elides calls to VOP_READ/VOP_WRITE > when copylen is 0, so the atime doesn't get updated. I wonder if we > could at least change it to call VOP_READ in that scenario, as in the > untested patch below. Any thoughts? > > diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c > index 4e4161ef1a7f..d60608a6d3b9 100644 > --- a/sys/kern/vfs_vnops.c > +++ b/sys/kern/vfs_vnops.c > @@ -3499,7 +3499,7 @@ vn_generic_copy_file_range(struct vnode *invp, off_= t *inoffp, > xfer -=3D (*inoffp % blksize); > } > /* Loop copying the data block. */ > - while (copylen > 0 && error =3D=3D 0 && !eof && interrupt= ed =3D=3D 0) { > + while (error =3D=3D 0 && !eof && interrupted =3D=3D 0) { > if (copylen < xfer) > xfer =3D copylen; > error =3D vn_lock(invp, LK_SHARED); > @@ -3511,7 +3511,7 @@ vn_generic_copy_file_range(struct vnode *invp, off_= t *inoffp, > curthread); > VOP_UNLOCK(invp); > lastblock =3D false; > - if (error =3D=3D 0 && aresid > 0) { > + if (error =3D=3D 0 && (xfer =3D=3D 0 || aresid > = 0)) { > /* Stop the copy at EOF on the input file= . */ > xfer -=3D aresid; > eof =3D true; > >From POSIX: "Note that a read() of zero bytes does not modify the last data access timestamp. A read() that requests more than zero bytes, but returns zero, is required to modify the last data access timestamp." While copy_file_range is not standardized, it ought to comport to POSIX as closely as possible. I think we should change it as you suggest.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2h7QLqLHPm-gUMDJKeR8oyAXssn2vxkJ8xNgBBT6Cc3bw>