From nobody Wed Oct 04 15:40:10 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 4S0zRD6kmBz4w7qw for ; Wed, 4 Oct 2023 15:40:24 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) (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 4S0zRD4rYhz4VjZ; Wed, 4 Oct 2023 15:40:24 +0000 (UTC) (envelope-from asomers@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-vs1-f50.google.com with SMTP id ada2fe7eead31-4526b9404b0so736688137.0; Wed, 04 Oct 2023 08:40:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696434023; x=1697038823; 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=X/0C217lo/sbqpIBXknhgdd9COBTytocMcZTPvNedNg=; b=eNjmclH+SLHWd+QpLSAoHRdvYR3pfqKCLqRls8ZjlYBwWLzC6IXJ83unj0QS7PAXI3 qlCHso+MiAxLmymZu6muA4kS7lyedaer7kLa1cI2z5IiauSOQ6aNeveyqQkD0g65pVfB g55ywpi8KJI704MyDJQCjDy4u+V+MJEje2A/x1n/XxnNuXNyQjLZrQ3WienAZOFERiQC E+veVssy76/iAYpJ4woJaPCIs8EGn35C6Tt8G1vLWHOzj6TPCYihsMKHb2LRlmtX9phe Swiy5xErPgV5b0YQTj1OuoK/mrAaUZsDAzJ2LMXr2ZFoAVkOFBBhvXg+7uiXWkTYOdWf ZPig== X-Gm-Message-State: AOJu0YwY5Iu5yADb0FJZ3FiNE5h45vRv2PD8LXNzOc3Ztho6FvPLx5c5 R0+qW9xdXuSXToT4tdiy2Aa5SzGbB+6Y1WOacNuw9e+C5NM= X-Google-Smtp-Source: AGHT+IFSHYEmQ4QbmzPOpRq3MHk21kR96b6mL0eoTCxoQXwx6xCEfC5qAk5uUhu5nXFgxjpE7P9WpqIf5BYhzGSHuNI= X-Received: by 2002:a67:ee49:0:b0:452:d5cb:a211 with SMTP id g9-20020a67ee49000000b00452d5cba211mr848vsp.15.1696434022336; Wed, 04 Oct 2023 08:40:22 -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: Alan Somers Date: Wed, 4 Oct 2023 08:40:10 -0700 Message-ID: Subject: Re: copy_file_range() doesn't update the atime of an empty file To: Mark Johnston Cc: 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:209.85.128.0/17, country:US] X-Rspamd-Queue-Id: 4S0zRD4rYhz4VjZ On Wed, Oct 4, 2023 at 8:31=E2=80=AFAM Mark Johnston 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.