mbox series

[v5,0/9] fs: multigrain timestamp redux

Message ID 20240711-mgtime-v5-0-37bb5b465feb@kernel.org
Headers show
Series fs: multigrain timestamp redux | expand

Message

Jeff Layton July 11, 2024, 11:08 a.m. UTC
tl;dr for those who have been following along:

There are several changes in this version. The conversion of ctime to
be a ktime_t value has been dropped, and we now use an unused bit in
the nsec field as the QUERIED flag (like the earlier patchset did).

The floor value is now tracked as a monotonic clock value, and is
converted to a realtime value on an as-needed basis. This eliminates the
problem of trying to detect when the realtime clock jumps backward.

Longer patch description for those just joining in:

At LSF/MM this year, we had a discussion about the inode change
attribute. At the time I mentioned that I thought I could salvage the
multigrain timestamp work that had to be reverted last year [1].

That version had to be reverted because it was possible for a file to
get a coarse grained timestamp that appeared to be earlier than another
file that had recently gotten a fine-grained stamp.

This version corrects the problem by establishing a per-time_namespace
ctime_floor value that should prevent this from occurring. In the above
situation, the two files might end up with the same timestamp value, but
they won't appear to have been modified in the wrong order.

That problem was discovered by the test-stat-time gnulib test. Note that
that test still fails on multigrain timestamps, but that's because its
method of determining the minimum delay that will show a timestamp
change will no longer work with multigrain timestamps. I have a patch to
change the testcase to use a different method that is in the process of
being merged.

The testing I've done seems to show performance parity with multigrain
timestamps enabled vs. disabled, but it's hard to rule this out
regressing some workload.

This set is based on top of Christian's vfs.misc branch (which has the
earlier change to track inode timestamps as discrete integers). If there
are no major objections, I'd like to have this considered for v6.12,
after a nice long full-cycle soak in linux-next.

PS: I took a stab at a conversion for bcachefs too, but it's not
trivial. bcachefs handles timestamps backward from the way most
block-based filesystems do. Instead of updating them in struct inode and
eventually copying them to a disk-based representation, it does the
reverse and updates the timestamps in its in-core image of the on-disk
inode, and then copies that into struct inode. Either that will need to
be changed, or we'll need to come up with a different way to do this for
bcachefs.

[1]: https://lore.kernel.org/linux-fsdevel/20230807-mgctime-v7-0-d1dec143a704@kernel.org/

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v5:
- refetch coarse time in coarse_ctime if not returning floor
- timestamp_truncate before swapping new ctime value into place
- track floor value as atomic64_t
- cleanups to Documentation file
- Link to v4: https://lore.kernel.org/r/20240708-mgtime-v4-0-a0f3c6fb57f3@kernel.org

Changes in v4:
- reordered tracepoint fields for better packing
- rework percpu counters again to also count fine grained timestamps
- switch to try_cmpxchg for better efficiency
- Link to v3: https://lore.kernel.org/r/20240705-mgtime-v3-0-85b2daa9b335@kernel.org

Changes in v3:
- Drop the conversion of i_ctime fields to ktime_t, and use an unused bit
  of the i_ctime_nsec field as QUERIED flag.
- Better tracepoints for tracking floor and ctime updates
- Reworked percpu counters to be more useful
- Track floor as monotonic value, which eliminates clock-jump problem

Changes in v2:
- Added Documentation file
- Link to v1: https://lore.kernel.org/r/20240626-mgtime-v1-0-a189352d0f8f@kernel.org

---
Jeff Layton (9):
      fs: add infrastructure for multigrain timestamps
      fs: tracepoints around multigrain timestamp events
      fs: add percpu counters for significant multigrain timestamp events
      fs: have setattr_copy handle multigrain timestamps appropriately
      Documentation: add a new file documenting multigrain timestamps
      xfs: switch to multigrain timestamps
      ext4: switch to multigrain timestamps
      btrfs: convert to multigrain timestamps
      tmpfs: add support for multigrain timestamps

 Documentation/filesystems/multigrain-ts.rst | 120 ++++++++++++++
 fs/attr.c                                   |  52 ++++++-
 fs/btrfs/file.c                             |  25 +--
 fs/btrfs/super.c                            |   3 +-
 fs/ext4/super.c                             |   2 +-
 fs/inode.c                                  | 234 ++++++++++++++++++++++++----
 fs/stat.c                                   |  39 ++++-
 fs/xfs/libxfs/xfs_trans_inode.c             |   6 +-
 fs/xfs/xfs_iops.c                           |  10 +-
 fs/xfs/xfs_super.c                          |   2 +-
 include/linux/fs.h                          |  34 +++-
 include/trace/events/timestamp.h            | 109 +++++++++++++
 mm/shmem.c                                  |   2 +-
 13 files changed, 560 insertions(+), 78 deletions(-)
---
base-commit: 7507ae6c41bb8990d3ae98ad0f5b0c15ca4156fe
change-id: 20240626-mgtime-5cd80b18d810

Best regards,

Comments

Josef Bacik July 11, 2024, 2:44 p.m. UTC | #1
On Thu, Jul 11, 2024 at 07:08:04AM -0400, Jeff Layton wrote:
> tl;dr for those who have been following along:
> 
> There are several changes in this version. The conversion of ctime to
> be a ktime_t value has been dropped, and we now use an unused bit in
> the nsec field as the QUERIED flag (like the earlier patchset did).
> 
> The floor value is now tracked as a monotonic clock value, and is
> converted to a realtime value on an as-needed basis. This eliminates the
> problem of trying to detect when the realtime clock jumps backward.
> 
> Longer patch description for those just joining in:
> 
> At LSF/MM this year, we had a discussion about the inode change
> attribute. At the time I mentioned that I thought I could salvage the
> multigrain timestamp work that had to be reverted last year [1].
> 
> That version had to be reverted because it was possible for a file to
> get a coarse grained timestamp that appeared to be earlier than another
> file that had recently gotten a fine-grained stamp.
> 
> This version corrects the problem by establishing a per-time_namespace
> ctime_floor value that should prevent this from occurring. In the above
> situation, the two files might end up with the same timestamp value, but
> they won't appear to have been modified in the wrong order.
> 
> That problem was discovered by the test-stat-time gnulib test. Note that
> that test still fails on multigrain timestamps, but that's because its
> method of determining the minimum delay that will show a timestamp
> change will no longer work with multigrain timestamps. I have a patch to
> change the testcase to use a different method that is in the process of
> being merged.
> 
> The testing I've done seems to show performance parity with multigrain
> timestamps enabled vs. disabled, but it's hard to rule this out
> regressing some workload.
> 
> This set is based on top of Christian's vfs.misc branch (which has the
> earlier change to track inode timestamps as discrete integers). If there
> are no major objections, I'd like to have this considered for v6.12,
> after a nice long full-cycle soak in linux-next.
> 
> PS: I took a stab at a conversion for bcachefs too, but it's not
> trivial. bcachefs handles timestamps backward from the way most
> block-based filesystems do. Instead of updating them in struct inode and
> eventually copying them to a disk-based representation, it does the
> reverse and updates the timestamps in its in-core image of the on-disk
> inode, and then copies that into struct inode. Either that will need to
> be changed, or we'll need to come up with a different way to do this for
> bcachefs.
> 
> [1]: https://lore.kernel.org/linux-fsdevel/20230807-mgctime-v7-0-d1dec143a704@kernel.org/
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef