From nobody Thu Oct 5 17:14:27 2023 X-Original-To: freebsd-hackers@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4S1dTZ30zFz4w69d for ; Thu, 5 Oct 2023 17:14:42 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4S1dTY5yBTz3bVx; Thu, 5 Oct 2023 17:14:41 +0000 (UTC) (envelope-from kostikbel@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.17.1/8.17.1) with ESMTPS id 395HERN9084719 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Thu, 5 Oct 2023 20:14:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 395HERN9084719 Received: (from kostik@localhost) by tom.home (8.17.1/8.17.1/Submit) id 395HERkB084718; Thu, 5 Oct 2023 20:14:27 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 5 Oct 2023 20:14:27 +0300 From: Konstantin Belousov To: Alan Somers Cc: Rick Macklem , Mark Johnston , freebsd-hackers@freebsd.org, rmacklem@freebsd.org Subject: Re: copy_file_range() doesn't update the atime of an empty file Message-ID: References: List-Id: Technical discussions relating to FreeBSD List-Archive: https://lists.freebsd.org/archives/freebsd-hackers List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-hackers@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=4.0.0 X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-14) on tom.home X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; TAGGED_RCPT(0.00)[]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US] X-Rspamd-Queue-Id: 4S1dTY5yBTz3bVx On Thu, Oct 05, 2023 at 08:41:16AM -0700, Alan Somers wrote: > I don't think that Linux is a good model to copy from, where atime is > concerned. It long ago gave up on POSIX-compliance for atime by > default. In this case, I think it's better to stick as closely as we > can to read(2). Preserving the existing behavior of tools like cat, > too, is worthwhile I think. I agree with Alan. > > On Thu, Oct 5, 2023 at 7:53 AM Rick Macklem wrote: > > > > 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 PM Rick Macklem wrote: > > > > > > Resent now that I am subscribed to freebsd-hackers@, > > > > > > On Wed, Oct 4, 2023 at 4:25 PM Rick Macklem wrote: > > > > > > > > On Wed, Oct 4, 2023 at 8:40 AM Alan Somers wrote: > > > > > > > > > > CAUTION: This email originated from outside of the University of Guelph. 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 IThelp@uoguelph.ca. > > > > > > > > > > > > > > > On Wed, Oct 4, 2023 at 8:31 AM Mark Johnston wrote: > > > > > > > > > > > > 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 -= (*inoffp % blksize); > > > > > > } > > > > > > /* Loop copying the data block. */ > > > > > > - while (copylen > 0 && error == 0 && !eof && interrupted == 0) { > > > > > > + while (error == 0 && !eof && interrupted == 0) { > > > > > > if (copylen < xfer) > > > > > > xfer = copylen; > > > > > > error = 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 = false; > > > > > > - if (error == 0 && aresid > 0) { > > > > > > + if (error == 0 && (xfer == 0 || aresid > 0)) { > > > > > > /* Stop the copy at EOF on the input file. */ > > > > > > xfer -= aresid; > > > > > > eof = 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. > > > > 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 for > > > > this reason. (I eventually intend to patch the man page to clarify that > > > > 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 update > > > > atime. (I did modify the file via "cat /dev/null > file" so that the atime would > > > > be updated for "relatime". A similar test using "cp" did update the atime.) > > > > > > > > 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_range(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 >