Date: Thu, 5 Oct 2023 07:52:55 -0700 From: Rick Macklem <rick.macklem@gmail.com> To: Alan Somers <asomers@freebsd.org> Cc: Mark Johnston <markj@freebsd.org>, freebsd-hackers@freebsd.org, rmacklem@freebsd.org Subject: Re: copy_file_range() doesn't update the atime of an empty file Message-ID: <CAM5tNy4%2BZZTYQ4QuD_sapx3q%2BQ%2Bwz9uNu6CGL17JFsjN13i0Sg@mail.gmail.com> In-Reply-To: <CAM5tNy5nLWf9c%2BnsdxJsU-M9Q3p_VVc%2BnpuY6uwbZPwM6EwhKg@mail.gmail.com> References: <ZR2FUeIhO7DIQIpj@nuc> <CAOtMX2h7QLqLHPm-gUMDJKeR8oyAXssn2vxkJ8xNgBBT6Cc3bw@mail.gmail.com> <CAM5tNy72tPBLHM8mkhqkUu64GuLUiZuKFJ%2B2JFsOzVgA1hm0eA@mail.gmail.com> <CAM5tNy5nLWf9c%2BnsdxJsU-M9Q3p_VVc%2BnpuY6uwbZPwM6EwhKg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Note that, although i'd prefer to keep copy_file_range(2) Linux compatible, I would like to hear others chime in w.r.t. their preference. rick On Wed, Oct 4, 2023 at 4:39=E2=80=AFPM Rick Macklem <rick.macklem@gmail.com= > wrote: > > Resent now that I am subscribed to freebsd-hackers@, > > On Wed, Oct 4, 2023 at 4:25=E2=80=AFPM Rick Macklem <rick.macklem@gmail.c= om> wrote: > > > > On Wed, Oct 4, 2023 at 8:40=E2=80=AFAM Alan Somers <asomers@freebsd.org= > wrote: > > > > > > CAUTION: This email originated from outside of the University of Guel= ph. Do not click links or open attachments unless you recognize the sender = and know the content is safe. If in doubt, forward suspicious emails to ITh= elp@uoguelph.ca. > > > > > > > > > On Wed, Oct 4, 2023 at 8:31=E2=80=AFAM Mark Johnston <markj@freebsd.o= rg> wrote: > > > > > > > > For a while, Jenkins has been complaining that one of the tmpfs tes= ts 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, wa= its > > > > 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) re= sults > > > > 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 a= s 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_WR= ITE > > > > when copylen is 0, so the atime doesn't get updated. I wonder if w= e > > > > could at least change it to call VOP_READ in that scenario, as in t= he > > > > 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 && int= errupted =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 || are= sid > 0)) { > > > > /* Stop the copy at EOF on the inpu= t file. */ > > > > xfer -=3D aresid; > > > > eof =3D true; > > > > > > > > > > From POSIX: "Note that a read() of zero bytes does not modify the las= t > > > 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. > > Well, I'd like to maintain the syscall as "Linux compatible", which was > > my original intent. (I consider Linux as the defacto standard for *nix*= like > > operating systems). > > > > I've been ignoring a recent request for support for non-regular files f= or > > this reason. (I eventually intend to patch the man page to clarify tha= t > > it only works for regular files, which is what Linux does.) > > > > As such, the first step is to figure out if Linux updates atime when a > > copy_file_range() returns 0 bytes. I just did a test on Linux (kernel > > version 6.3) > > using a ext4 fs mounted "relatime" and doing a copy_file_range(2) on it > > (using a trivial file copy program suing copy_file_range(2)) did not up= date > > atime. (I did modify the file via "cat /dev/null > file" so that the at= ime would > > be updated for "relatime". A similar test using "cp" did update the ati= me.) > > > > Also, the above changes the "generic" copy loop, but changes will > > also be required (or at least tested) for ZFS when block cloning is > > enabled and NFSv4.2. The NFSv4.2 RFC does not specify whether > > or not a "Copy" operation that returns 0 bytes updates atime > > (called TimeAccess in NFSv4.2). > > Oh, and the NFS protocol (up to and including NFSv4.2) cannot > > provide a POSIX compliant file system (the NFS client tries to make > > it look close to POSIX compliant). As such, expecting a copy_file_rang= e(2) > > over NFSv4.2 to behave in a POSIX-like way may not make sense? > > > > Personally, I'd rather see copy_file_range(2) remain Linux compatible. > > Does cat(1) really need to exhibit this behaviour or is it just read(2) > > that specifies this? > > > > rick
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy4%2BZZTYQ4QuD_sapx3q%2BQ%2Bwz9uNu6CGL17JFsjN13i0Sg>