From nobody Thu Oct 05 22:30:25 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 4S1mV61PCMz4w1sq for ; Thu, 5 Oct 2023 22:30:38 +0000 (UTC) (envelope-from rick.macklem@gmail.com) Received: from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com [IPv6:2607:f8b0:4864:20::42f]) (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 4S1mV56p08z3Kbd; Thu, 5 Oct 2023 22:30:37 +0000 (UTC) (envelope-from rick.macklem@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pf1-x42f.google.com with SMTP id d2e1a72fcca58-692c70bc440so1280828b3a.3; Thu, 05 Oct 2023 15:30:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696545035; x=1697149835; 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=UZxWY3mgfDOK8vfHiH1gsEMMGZFZusn70W0thTM4YQc=; b=gzVnZkgg5Wx+2FH/o03Pbuh4Jvxcd6YJbabAX0B+PWcyAvhw/PkMRJMu2QoFyE1pxC BI4cHeU9USHKgGAY7JC2i9v0vI/09O/U3XUqtmDvEcuWFlyK7LkcMfWS6BgolDtQnEL1 EU7M22oZ9enAuy95S8orkraLapysZr9SfXaDXB6QZS0uhIgm7QnDGVgIDYIeFvDKvK0G 4x1IlUa3oE4mc3Q7U2ln7JwqnGRtVHKwnahTDtdxizgKYO2itfvBf1gBvSItsw5yfrCv 89Src17FFEqleRoiR19LdaHis5+rtFy4dP9NNt/gAHtGN4SmcJYP6PtwZjxdy22s1CCW AeZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696545035; x=1697149835; 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=UZxWY3mgfDOK8vfHiH1gsEMMGZFZusn70W0thTM4YQc=; b=vn7RCu0LdWWMx2orcQYjJgr0ioVFwhi/d9tcu3T4DfJSVDpDJkW8wBUOgdoHXniPYr n92dBq3nz2pcMyCBtQnxoupRBIAl6FGfJDqL8/Xtrj5N0Ax/0vD9ofOaxjGO9/CIANJP UzsnsP9Mu5ygbIP+VsFdW2+bSsQMb5xxpDuxLTL0anlC1W72UO2C07o8H8m3vhe9OTmB VqjkQp5Z+L7+yXwBnEHIxiINFcyJM+U3csEXiFQchKbldq3q5Nfuod5rkwg98ncdVpqU FfT3jS8q6FMShViJWU06fuaN0SzsYQKtGVpLEPoVnfjRiNK5u0YbtCEom/3Bn3fuyfvJ n8Dw== X-Gm-Message-State: AOJu0YzUHybNZvY1QNOaVyCtOs/xhudwat61xQ+yKK9VZ8SuM9xS2ic0 biRqwNp1Fq7QWK7azig9OTU1nPN3FfNUyf8DDjaz6hs= X-Google-Smtp-Source: AGHT+IHIGBIJopzLRL3584z1OTMfAdTJw6WuK2QdvEBP4/mpzhzOqPtd6MTbD48RSK02lJFg2MCDn0WQwxpliXedw0w= X-Received: by 2002:a17:90b:38cb:b0:274:729a:bcd9 with SMTP id nn11-20020a17090b38cb00b00274729abcd9mr6511560pjb.43.1696545035186; Thu, 05 Oct 2023 15:30:35 -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 15:30:25 -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-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)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; TAGGED_FROM(0.00)[] X-Rspamd-Queue-Id: 4S1mV56p08z3Kbd On Thu, Oct 5, 2023 at 8:41=E2=80=AFAM Alan Somers wr= ote: > > 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). ZFS..I have no idea. Someone else will need to test it (with block cloning 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 compati= ble, > > 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 of = Guelph. Do not click links or open attachments unless you recognize the sen= der 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 tmpfs= tests is > > > > > > failing: > > > > > > https://ci.freebsd.org/job/FreeBSD-main-amd64-test/23814/testRe= port/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 upd= ated. > > > > > > > > > > > > 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 lo= ng 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/VO= P_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 &&= 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 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 byte= s, > > > > > 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 fil= es 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 whe= n a > > > > copy_file_range() returns 0 bytes. I just did a test on Linux (kern= el > > > > version 6.3) > > > > using a ext4 fs mounted "relatime" and doing a copy_file_range(2) o= n it > > > > (using a trivial file copy program suing copy_file_range(2)) did no= t update > > > > atime. (I did modify the file via "cat /dev/null > file" so that th= e 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 compatib= le. > > > > Does cat(1) really need to exhibit this behaviour or is it just rea= d(2) > > > > that specifies this? > > > > > > > > rick