From nobody Fri Oct 06 01:53:04 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 4S1rzw4n42z4wZZK for ; Fri, 6 Oct 2023 01:53:16 +0000 (UTC) (envelope-from rick.macklem@gmail.com) Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4S1rzv5wNzz4JcS; Fri, 6 Oct 2023 01:53:15 +0000 (UTC) (envelope-from rick.macklem@gmail.com) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20230601 header.b=NdsbKMCX; spf=pass (mx1.freebsd.org: domain of rick.macklem@gmail.com designates 2607:f8b0:4864:20::102f as permitted sender) smtp.mailfrom=rick.macklem@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-pj1-x102f.google.com with SMTP id 98e67ed59e1d1-2791747288cso1251558a91.0; Thu, 05 Oct 2023 18:53:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696557194; x=1697161994; darn=freebsd.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=HhyrelIBYf1yCuE3gjve4dLXapXlvQRQ1j/ATxyteH4=; b=NdsbKMCXBN+kr13jjcdmp62YFsQjuAeHXM7dCoNYrUPsRQIZtnu7/o3j9OE+oPfW54 WpvgKrtBAAzqDaqZ+vWxUJfMu6aZZ455fkcvmmK+xngYtY324LVW/u7hqIE97eY3yDyy Sh35ESyjQiThXlXIEBF+izRYvv9yWcNy9CSTLKI5YdeDW0XPXCy1nh7JXXqf41Mzx0tG 3/h2Io7wKA5Vdy2aNtdATvLxw1iLJ9LdZsyDxNB7HVsE1wanUYOXLT7jddmqWahpuG6Y HXmuWox/6jEbypKIG7nrHd8/zgE44ON1Jn7jLwGG9IAGMk33qwRIeS6V5l905RE0rerJ e3Aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696557194; x=1697161994; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HhyrelIBYf1yCuE3gjve4dLXapXlvQRQ1j/ATxyteH4=; b=bC0Z1byFqVeGcShY4Oj2d0Z30PuBHsTnxIFXpGi1OQrQHdzcqbxNSPVqYWNcc4TOeR ZMdUDnY+e9/ui6/KaPupA4XzaBXy4wWe33xDp2zdOZd1mW9ztlIoXtScmLAwQhKgfa0u 5IEcs5GEinRhQnWIzGc28h5B3Btw6mR3NMP7GlvELteJGhJIXm4YUeZgH+/fkb7FjTDA UZ6YYdwUzYwuMYBvh00S2gm8Bqxenx1Yu56MB/059BMAbhXglI6XjZWbYG+aaM/Coh3H ZXtnCdcSp5Z8HHeSWxZJM0dd0RyyHmbQF+i2pV/4YcJ/bSakQ4vA/QKbjTayea1PFKhY R6BQ== X-Gm-Message-State: AOJu0YyGhaUyxwUnV7jFoTAjLi4OJUIaXpMsjpG8QNiGsypVKc85KJJd me07pYj3YDhtXjmKf+keDeITzxFEtCCalDF4hxR2D9s= X-Google-Smtp-Source: AGHT+IFazvNryxmJ0WSJcaD+4zPtZE9BlM5duwR2Df9F+HSPoquEPHbvlyqyXGYqLIsdXebSVnGKeKxEu0+BqV96wAk= X-Received: by 2002:a17:90b:180d:b0:26b:6095:bc3f with SMTP id lw13-20020a17090b180d00b0026b6095bc3fmr6362475pjb.33.1696557193999; Thu, 05 Oct 2023 18:53:13 -0700 (PDT) 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 References: In-Reply-To: From: Rick Macklem Date: Thu, 5 Oct 2023 18:53:04 -0700 Message-ID: Subject: Re: copy_file_range() doesn't update the atime of an empty file To: Alan Somers Cc: Mark Johnston , freebsd-hackers@freebsd.org, rmacklem@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spamd-Bar: --- X-Spamd-Result: default: False [-3.94 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.94)[-0.942]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20230601]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; MIME_GOOD(-0.10)[text/plain]; RCVD_COUNT_ONE(0.00)[1]; MLMMJ_DEST(0.00)[freebsd-hackers@freebsd.org]; FROM_EQ_ENVFROM(0.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::102f:from]; FREEMAIL_ENVFROM(0.00)[gmail.com]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_LAST(0.00)[]; MID_RHS_MATCH_FROMTLD(0.00)[]; TAGGED_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; ARC_NA(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; RCPT_COUNT_THREE(0.00)[4]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; TO_DN_SOME(0.00)[]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-Rspamd-Queue-Id: 4S1rzv5wNzz4JcS On Thu, Oct 5, 2023 at 3:30=E2=80=AFPM Rick Macklem wrote: > > On Thu, Oct 5, 2023 at 8:41=E2=80=AFAM 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 have no problem with Mark's patch being applied for the default > local fs case. NFSv4.2 will not be able to comply with this unless > (as will be the case for the FreeBSD server) the NFSv4.2 server > happens to change atime after Mark's patch is applied to the > FreeBSD NFSv4.2 server (the Linux NFSv4.2 server will not). I have come up with a NFSv4.2 client patch that explicitly sets atime for the input file in the same compound RPC as the Copy. It works for a FreeBSD server without Mark's patch. If a NFSv4.2 server does not do it, we can argue that the server ignores the Setattr of atime. So, with this patch (which I will be testing against assorted servers next week (an ietf bakeathon testing event) and Mark's patch, the only case that may need more work is ZFS? rick ps: I'll admit I still doubt anyone cares about atime being set, but the collective opinion seems to be that it should be set. > > ZFS..I have no idea. Someone else will need to test it (with block clonin= g > enabled). > > rick > > > > On Thu, Oct 5, 2023 at 7:53=E2=80=AFAM Rick Macklem wrote: > > > > > > Note that, although i'd prefer to keep copy_file_range(2) Linux compa= tible, > > > 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 wrote: > > > > > > > > Resent now that I am subscribed to freebsd-hackers@, > > > > > > > > On Wed, Oct 4, 2023 at 4:25=E2=80=AFPM Rick Macklem wrote: > > > > > > > > > > On Wed, Oct 4, 2023 at 8:40=E2=80=AFAM Alan Somers wrote: > > > > > > > > > > > > CAUTION: This email originated from outside of the University o= f Guelph. Do not click links or open attachments unless you recognize the s= ender and know the content is safe. If in doubt, forward suspicious emails = to IThelp@uoguelph.ca. > > > > > > > > > > > > > > > > > > On Wed, Oct 4, 2023 at 8:31=E2=80=AFAM Mark Johnston wrote: > > > > > > > > > > > > > > For a while, Jenkins has been complaining that one of the tmp= fs tests is > > > > > > > failing: > > > > > > > https://ci.freebsd.org/job/FreeBSD-main-amd64-test/23814/test= Report/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 fi= le, waits > > > > > > > for a second, then cat(1)s it and checks that the file's atim= e was > > > > > > > updated. After the aforementioned commit, the atime is not u= pdated. > > > > > > > > > > > > > > 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 ev= en 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 wonde= r if we > > > > > > > could at least change it to call VOP_READ in that scenario, a= s 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 = && interrupted =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 th= e input file. */ > > > > > > > xfer -=3D aresid; > > > > > > > eof =3D true; > > > > > > > > > > > > > > > > > > > From POSIX: "Note that a read() of zero bytes does not modify t= he last > > > > > > data access timestamp. A read() that requests more than zero by= tes, > > > > > > 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 y= ou > > > > > > suggest. > > > > > Well, I'd like to maintain the syscall as "Linux compatible", whi= ch 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 f= iles for > > > > > this reason. (I eventually intend to patch the man page to clari= fy 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 w= hen a > > > > > copy_file_range() returns 0 bytes. I just did a test on Linux (ke= rnel > > > > > 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 t= he 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 ma= ke > > > > > it look close to POSIX compliant). As such, expecting a copy_fil= e_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 compat= ible. > > > > > Does cat(1) really need to exhibit this behaviour or is it just r= ead(2) > > > > > that specifies this? > > > > > > > > > > rick