Re: [PATCH v6 5/9] Documentation: add a new file documenting multigrain timestamps

2024-07-17 Thread Jeff Layton
On Wed, 2024-07-17 at 13:31 +0200, Jan Kara wrote:
> On Mon 15-07-24 08:48:56, Jeff Layton wrote:
> > Add a high-level document that describes how multigrain timestamps work,
> > rationale for them, and some info about implementation and tradeoffs.
> > 
> > Reviewed-by: Josef Bacik 
> > Signed-off-by: Jeff Layton 
> 
> One comment below. With that fixed feel free to add:
> 
> Reviewed-by: Jan Kara 
> 
> > +Implementation Notes
> > +
> > +Multigrain timestamps are intended for use by local filesystems that get
> > +ctime values from the local clock. This is in contrast to network 
> > filesystems
> > +and the like that just mirror timestamp values from a server.
> > +
> > +For most filesystems, it's sufficient to just set the FS_MGTIME flag in the
> > +fstype->fs_flags in order to opt-in, providing the ctime is only ever set 
> > via
> > +inode_set_ctime_current(). If the filesystem has a ->getattr routine that
> > +doesn't call generic_fillattr, then you should have it call fill_mg_cmtime 
> > to
> > +fill those values.
> 
> I think you should explicitely mention that ->setattr() implementation
> needs to use setattr_copy() or otherwise mimic its behavior...
> 
>   Honza

I've added a sentence like you suggest to the patch in my tree. Thanks
for all the reviews!
-- 
Jeff Layton 



Re: [PATCH v6 5/9] Documentation: add a new file documenting multigrain timestamps

2024-07-17 Thread Jan Kara
On Mon 15-07-24 08:48:56, Jeff Layton wrote:
> Add a high-level document that describes how multigrain timestamps work,
> rationale for them, and some info about implementation and tradeoffs.
> 
> Reviewed-by: Josef Bacik 
> Signed-off-by: Jeff Layton 

One comment below. With that fixed feel free to add:

Reviewed-by: Jan Kara 

> +Implementation Notes
> +
> +Multigrain timestamps are intended for use by local filesystems that get
> +ctime values from the local clock. This is in contrast to network filesystems
> +and the like that just mirror timestamp values from a server.
> +
> +For most filesystems, it's sufficient to just set the FS_MGTIME flag in the
> +fstype->fs_flags in order to opt-in, providing the ctime is only ever set via
> +inode_set_ctime_current(). If the filesystem has a ->getattr routine that
> +doesn't call generic_fillattr, then you should have it call fill_mg_cmtime to
> +fill those values.

I think you should explicitely mention that ->setattr() implementation
needs to use setattr_copy() or otherwise mimic its behavior...

Honza
-- 
Jan Kara 
SUSE Labs, CR



Re: [PATCH v6 5/9] Documentation: add a new file documenting multigrain timestamps

2024-07-16 Thread Randy Dunlap



On 7/15/24 5:48 AM, Jeff Layton wrote:
> Add a high-level document that describes how multigrain timestamps work,
> rationale for them, and some info about implementation and tradeoffs.
> 
> Reviewed-by: Josef Bacik 
> Signed-off-by: Jeff Layton 
> ---
>  Documentation/filesystems/multigrain-ts.rst | 120 
> 
>  1 file changed, 120 insertions(+)
> 


Reviewed-by: Randy Dunlap 

Thanks.

-- 
~Randy



Re: [PATCH v6 5/9] Documentation: add a new file documenting multigrain timestamps

2024-07-15 Thread Darrick J. Wong
On Mon, Jul 15, 2024 at 08:48:56AM -0400, Jeff Layton wrote:
> Add a high-level document that describes how multigrain timestamps work,
> rationale for them, and some info about implementation and tradeoffs.
> 
> Reviewed-by: Josef Bacik 
> Signed-off-by: Jeff Layton 

Seems fine to me,
Reviewed-by: Darrick J. Wong 

--D

> ---
>  Documentation/filesystems/multigrain-ts.rst | 120 
> 
>  1 file changed, 120 insertions(+)
> 
> diff --git a/Documentation/filesystems/multigrain-ts.rst 
> b/Documentation/filesystems/multigrain-ts.rst
> new file mode 100644
> index ..5cefc204ecec
> --- /dev/null
> +++ b/Documentation/filesystems/multigrain-ts.rst
> @@ -0,0 +1,120 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=
> +Multigrain Timestamps
> +=
> +
> +Introduction
> +
> +Historically, the kernel has always used coarse time values to stamp
> +inodes. This value is updated on every jiffy, so any change that happens
> +within that jiffy will end up with the same timestamp.
> +
> +When the kernel goes to stamp an inode (due to a read or write), it first 
> gets
> +the current time and then compares it to the existing timestamp(s) to see
> +whether anything will change. If nothing changed, then it can avoid updating
> +the inode's metadata.
> +
> +Coarse timestamps are therefore good from a performance standpoint, since 
> they
> +reduce the need for metadata updates, but bad from the standpoint of
> +determining whether anything has changed, since a lot of things can happen 
> in a
> +jiffy.
> +
> +They are particularly troublesome with NFSv3, where unchanging timestamps can
> +make it difficult to tell whether to invalidate caches. NFSv4 provides a
> +dedicated change attribute that should always show a visible change, but not
> +all filesystems implement this properly, causing the NFS server to substitute
> +the ctime in many cases.
> +
> +Multigrain timestamps aim to remedy this by selectively using fine-grained
> +timestamps when a file has had its timestamps queried recently, and the 
> current
> +coarse-grained time does not cause a change.
> +
> +Inode Timestamps
> +
> +There are currently 3 timestamps in the inode that are updated to the current
> +wallclock time on different activity:
> +
> +ctime:
> +  The inode change time. This is stamped with the current time whenever
> +  the inode's metadata is changed. Note that this value is not settable
> +  from userland.
> +
> +mtime:
> +  The inode modification time. This is stamped with the current time
> +  any time a file's contents change.
> +
> +atime:
> +  The inode access time. This is stamped whenever an inode's contents are
> +  read. Widely considered to be a terrible mistake. Usually avoided with
> +  options like noatime or relatime.
> +
> +Updating the mtime always implies a change to the ctime, but updating the
> +atime due to a read request does not.
> +
> +Multigrain timestamps are only tracked for the ctime and the mtime. atimes 
> are
> +not affected and always use the coarse-grained value (subject to the floor).
> +
> +Inode Timestamp Ordering
> +
> +
> +In addition to just providing info about changes to individual files, file
> +timestamps also serve an important purpose in applications like "make". These
> +programs measure timestamps in order to determine whether source files might 
> be
> +newer than cached objects.
> +
> +Userland applications like make can only determine ordering based on
> +operational boundaries. For a syscall those are the syscall entry and exit
> +points. For io_uring or nfsd operations, that's the request submission and
> +response. In the case of concurrent operations, userland can make no
> +determination about the order in which things will occur.
> +
> +For instance, if a single thread modifies one file, and then another file in
> +sequence, the second file must show an equal or later mtime than the first. 
> The
> +same is true if two threads are issuing similar operations that do not 
> overlap
> +in time.
> +
> +If however, two threads have racing syscalls that overlap in time, then there
> +is no such guarantee, and the second file may appear to have been modified
> +before, after or at the same time as the first, regardless of which one was
> +submitted first.
> +
> +Multigrain Timestamps
> +=
> +Multigrain timestamps are aimed at ensuring that changes to a single file are
> +always recognizable, without violating the ordering guarantees when multiple
> +different files are modified. This affects the mtime and the ctime, but the
> +atime will always use coarse-grained timestamps.
> +
> +It uses an unused bit in the i_ctime_nsec field to indicate whether the mtime
> +or ctime has been queried. If either or both have, then the kernel takes
> +special care to ensure the next timestamp update will display a visible 
> change.
> +This ensures tight cache