Skip to content

Commit b5cbe7c

Browse files
committed
Merge tag 'v6.6-rc3.vfs.ctime.revert' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
Pull finegrained timestamp reverts from Christian Brauner: "Earlier this week we sent a few minor fixes for the multi-grained timestamp work in [1]. While we were polishing those up after Linus realized that there might be a nicer way to fix them we received a regression report in [2] that fine grained timestamps break gnulib tests and thus possibly other tools. The kernel will elide fine-grain timestamp updates when no one is actively querying for them to avoid performance impacts. So a sequence like write(f1) stat(f2) write(f2) stat(f2) write(f1) stat(f1) may result in timestamp f1 to be older than the final f2 timestamp even though f1 was last written too but the second write didn't update the timestamp. Such plotholes can lead to subtle bugs when programs compare timestamps. For example, the nap() function in [2] will estimate that it needs to wait one ns on a fine-grain timestamp enabled filesytem between subsequent calls to observe a timestamp change. But in general we don't update timestamps with more than one jiffie if we think that no one is actively querying for fine-grain timestamps to avoid performance impacts. While discussing various fixes the decision was to go back to the drawing board and ultimately to explore a solution that involves only exposing such fine-grained timestamps to nfs internally and never to userspace. As there are multiple solutions discussed the honest thing to do here is not to fix this up or disable it but to cleanly revert. The general infrastructure will probably come back but there is no reason to keep this code in mainline. The general changes to timestamp handling are valid and a good cleanup that will stay. The revert is fully bisectable" Link: https://lore.kernel.org/all/20230918-hirte-neuzugang-4c2324e7bae3@brauner [1] Link: https://lore.kernel.org/all/bf0524debb976627693e12ad23690094e4514303.camel@linuxfromscratch.org [2] * tag 'v6.6-rc3.vfs.ctime.revert' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: Revert "fs: add infrastructure for multigrain timestamps" Revert "btrfs: convert to multigrain timestamps" Revert "ext4: switch to multigrain timestamps" Revert "xfs: switch to multigrain timestamps" Revert "tmpfs: add support for multigrain timestamps"
2 parents 7bdfc1a + 647aa76 commit b5cbe7c

File tree

10 files changed

+38
-178
lines changed

10 files changed

+38
-178
lines changed

fs/btrfs/file.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,25 @@ void btrfs_check_nocow_unlock(struct btrfs_inode *inode)
11061106
btrfs_drew_write_unlock(&inode->root->snapshot_lock);
11071107
}
11081108

1109+
static void update_time_for_write(struct inode *inode)
1110+
{
1111+
struct timespec64 now, ctime;
1112+
1113+
if (IS_NOCMTIME(inode))
1114+
return;
1115+
1116+
now = current_time(inode);
1117+
if (!timespec64_equal(&inode->i_mtime, &now))
1118+
inode->i_mtime = now;
1119+
1120+
ctime = inode_get_ctime(inode);
1121+
if (!timespec64_equal(&ctime, &now))
1122+
inode_set_ctime_to_ts(inode, now);
1123+
1124+
if (IS_I_VERSION(inode))
1125+
inode_inc_iversion(inode);
1126+
}
1127+
11091128
static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
11101129
size_t count)
11111130
{
@@ -1137,10 +1156,7 @@ static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
11371156
* need to start yet another transaction to update the inode as we will
11381157
* update the inode when we finish writing whatever data we write.
11391158
*/
1140-
if (!IS_NOCMTIME(inode)) {
1141-
inode->i_mtime = inode_set_ctime_current(inode);
1142-
inode_inc_iversion(inode);
1143-
}
1159+
update_time_for_write(inode);
11441160

11451161
start_pos = round_down(pos, fs_info->sectorsize);
11461162
oldsize = i_size_read(inode);

fs/btrfs/super.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,16 +2150,15 @@ static struct file_system_type btrfs_fs_type = {
21502150
.name = "btrfs",
21512151
.mount = btrfs_mount,
21522152
.kill_sb = btrfs_kill_super,
2153-
.fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_MGTIME,
2153+
.fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
21542154
};
21552155

21562156
static struct file_system_type btrfs_root_fs_type = {
21572157
.owner = THIS_MODULE,
21582158
.name = "btrfs",
21592159
.mount = btrfs_mount_root,
21602160
.kill_sb = btrfs_kill_super,
2161-
.fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA |
2162-
FS_ALLOW_IDMAP | FS_MGTIME,
2161+
.fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_ALLOW_IDMAP,
21632162
};
21642163

21652164
MODULE_ALIAS_FS("btrfs");

fs/ext4/super.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7314,7 +7314,7 @@ static struct file_system_type ext4_fs_type = {
73147314
.init_fs_context = ext4_init_fs_context,
73157315
.parameters = ext4_param_specs,
73167316
.kill_sb = ext4_kill_sb,
7317-
.fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
7317+
.fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
73187318
};
73197319
MODULE_ALIAS_FS("ext4");
73207320

fs/inode.c

Lines changed: 3 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -2102,52 +2102,10 @@ int file_remove_privs(struct file *file)
21022102
}
21032103
EXPORT_SYMBOL(file_remove_privs);
21042104

2105-
/**
2106-
* current_mgtime - Return FS time (possibly fine-grained)
2107-
* @inode: inode.
2108-
*
2109-
* Return the current time truncated to the time granularity supported by
2110-
* the fs, as suitable for a ctime/mtime change. If the ctime is flagged
2111-
* as having been QUERIED, get a fine-grained timestamp.
2112-
*/
2113-
struct timespec64 current_mgtime(struct inode *inode)
2114-
{
2115-
struct timespec64 now, ctime;
2116-
atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
2117-
long nsec = atomic_long_read(pnsec);
2118-
2119-
if (nsec & I_CTIME_QUERIED) {
2120-
ktime_get_real_ts64(&now);
2121-
return timestamp_truncate(now, inode);
2122-
}
2123-
2124-
ktime_get_coarse_real_ts64(&now);
2125-
now = timestamp_truncate(now, inode);
2126-
2127-
/*
2128-
* If we've recently fetched a fine-grained timestamp
2129-
* then the coarse-grained one may still be earlier than the
2130-
* existing ctime. Just keep the existing value if so.
2131-
*/
2132-
ctime = inode_get_ctime(inode);
2133-
if (timespec64_compare(&ctime, &now) > 0)
2134-
now = ctime;
2135-
2136-
return now;
2137-
}
2138-
EXPORT_SYMBOL(current_mgtime);
2139-
2140-
static struct timespec64 current_ctime(struct inode *inode)
2141-
{
2142-
if (is_mgtime(inode))
2143-
return current_mgtime(inode);
2144-
return current_time(inode);
2145-
}
2146-
21472105
static int inode_needs_update_time(struct inode *inode)
21482106
{
21492107
int sync_it = 0;
2150-
struct timespec64 now = current_ctime(inode);
2108+
struct timespec64 now = current_time(inode);
21512109
struct timespec64 ctime;
21522110

21532111
/* First try to exhaust all avenues to not sync */
@@ -2578,43 +2536,9 @@ EXPORT_SYMBOL(current_time);
25782536
*/
25792537
struct timespec64 inode_set_ctime_current(struct inode *inode)
25802538
{
2581-
struct timespec64 now;
2582-
struct timespec64 ctime;
2583-
2584-
ctime.tv_nsec = READ_ONCE(inode->__i_ctime.tv_nsec);
2585-
if (!(ctime.tv_nsec & I_CTIME_QUERIED)) {
2586-
now = current_time(inode);
2539+
struct timespec64 now = current_time(inode);
25872540

2588-
/* Just copy it into place if it's not multigrain */
2589-
if (!is_mgtime(inode)) {
2590-
inode_set_ctime_to_ts(inode, now);
2591-
return now;
2592-
}
2593-
2594-
/*
2595-
* If we've recently updated with a fine-grained timestamp,
2596-
* then the coarse-grained one may still be earlier than the
2597-
* existing ctime. Just keep the existing value if so.
2598-
*/
2599-
ctime.tv_sec = inode->__i_ctime.tv_sec;
2600-
if (timespec64_compare(&ctime, &now) > 0)
2601-
return ctime;
2602-
2603-
/*
2604-
* Ctime updates are usually protected by the inode_lock, but
2605-
* we can still race with someone setting the QUERIED flag.
2606-
* Try to swap the new nsec value into place. If it's changed
2607-
* in the interim, then just go with a fine-grained timestamp.
2608-
*/
2609-
if (cmpxchg(&inode->__i_ctime.tv_nsec, ctime.tv_nsec,
2610-
now.tv_nsec) != ctime.tv_nsec)
2611-
goto fine_grained;
2612-
inode->__i_ctime.tv_sec = now.tv_sec;
2613-
return now;
2614-
}
2615-
fine_grained:
2616-
ktime_get_real_ts64(&now);
2617-
inode_set_ctime_to_ts(inode, timestamp_truncate(now, inode));
2541+
inode_set_ctime(inode, now.tv_sec, now.tv_nsec);
26182542
return now;
26192543
}
26202544
EXPORT_SYMBOL(inode_set_ctime_current);

fs/stat.c

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -26,37 +26,6 @@
2626
#include "internal.h"
2727
#include "mount.h"
2828

29-
/**
30-
* fill_mg_cmtime - Fill in the mtime and ctime and flag ctime as QUERIED
31-
* @stat: where to store the resulting values
32-
* @request_mask: STATX_* values requested
33-
* @inode: inode from which to grab the c/mtime
34-
*
35-
* Given @inode, grab the ctime and mtime out if it and store the result
36-
* in @stat. When fetching the value, flag it as queried so the next write
37-
* will use a fine-grained timestamp.
38-
*/
39-
void fill_mg_cmtime(struct kstat *stat, u32 request_mask, struct inode *inode)
40-
{
41-
atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
42-
43-
/* If neither time was requested, then don't report them */
44-
if (!(request_mask & (STATX_CTIME|STATX_MTIME))) {
45-
stat->result_mask &= ~(STATX_CTIME|STATX_MTIME);
46-
return;
47-
}
48-
49-
stat->mtime = inode->i_mtime;
50-
stat->ctime.tv_sec = inode->__i_ctime.tv_sec;
51-
/*
52-
* Atomically set the QUERIED flag and fetch the new value with
53-
* the flag masked off.
54-
*/
55-
stat->ctime.tv_nsec = atomic_long_fetch_or(I_CTIME_QUERIED, pnsec) &
56-
~I_CTIME_QUERIED;
57-
}
58-
EXPORT_SYMBOL(fill_mg_cmtime);
59-
6029
/**
6130
* generic_fillattr - Fill in the basic attributes from the inode struct
6231
* @idmap: idmap of the mount the inode was found from
@@ -89,14 +58,8 @@ void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
8958
stat->rdev = inode->i_rdev;
9059
stat->size = i_size_read(inode);
9160
stat->atime = inode->i_atime;
92-
93-
if (is_mgtime(inode)) {
94-
fill_mg_cmtime(stat, request_mask, inode);
95-
} else {
96-
stat->mtime = inode->i_mtime;
97-
stat->ctime = inode_get_ctime(inode);
98-
}
99-
61+
stat->mtime = inode->i_mtime;
62+
stat->ctime = inode_get_ctime(inode);
10063
stat->blksize = i_blocksize(inode);
10164
stat->blocks = inode->i_blocks;
10265

fs/xfs/libxfs/xfs_trans_inode.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,12 @@ xfs_trans_ichgtime(
6262
ASSERT(tp);
6363
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
6464

65-
/* If the mtime changes, then ctime must also change */
66-
ASSERT(flags & XFS_ICHGTIME_CHG);
65+
tv = current_time(inode);
6766

68-
tv = inode_set_ctime_current(inode);
6967
if (flags & XFS_ICHGTIME_MOD)
7068
inode->i_mtime = tv;
69+
if (flags & XFS_ICHGTIME_CHG)
70+
inode_set_ctime_to_ts(inode, tv);
7171
if (flags & XFS_ICHGTIME_CREATE)
7272
ip->i_crtime = tv;
7373
}

fs/xfs/xfs_iops.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -573,10 +573,10 @@ xfs_vn_getattr(
573573
stat->gid = vfsgid_into_kgid(vfsgid);
574574
stat->ino = ip->i_ino;
575575
stat->atime = inode->i_atime;
576+
stat->mtime = inode->i_mtime;
577+
stat->ctime = inode_get_ctime(inode);
576578
stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks);
577579

578-
fill_mg_cmtime(stat, request_mask, inode);
579-
580580
if (xfs_has_v3inodes(mp)) {
581581
if (request_mask & STATX_BTIME) {
582582
stat->result_mask |= STATX_BTIME;
@@ -917,7 +917,7 @@ xfs_setattr_size(
917917
if (newsize != oldsize &&
918918
!(iattr->ia_valid & (ATTR_CTIME | ATTR_MTIME))) {
919919
iattr->ia_ctime = iattr->ia_mtime =
920-
current_mgtime(inode);
920+
current_time(inode);
921921
iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME;
922922
}
923923

fs/xfs/xfs_super.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2065,7 +2065,7 @@ static struct file_system_type xfs_fs_type = {
20652065
.init_fs_context = xfs_init_fs_context,
20662066
.parameters = xfs_fs_parameters,
20672067
.kill_sb = xfs_kill_sb,
2068-
.fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME,
2068+
.fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
20692069
};
20702070
MODULE_ALIAS_FS("xfs");
20712071

include/linux/fs.h

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,47 +1508,18 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
15081508
kgid_has_mapping(fs_userns, kgid);
15091509
}
15101510

1511-
struct timespec64 current_mgtime(struct inode *inode);
15121511
struct timespec64 current_time(struct inode *inode);
15131512
struct timespec64 inode_set_ctime_current(struct inode *inode);
15141513

1515-
/*
1516-
* Multigrain timestamps
1517-
*
1518-
* Conditionally use fine-grained ctime and mtime timestamps when there
1519-
* are users actively observing them via getattr. The primary use-case
1520-
* for this is NFS clients that use the ctime to distinguish between
1521-
* different states of the file, and that are often fooled by multiple
1522-
* operations that occur in the same coarse-grained timer tick.
1523-
*
1524-
* The kernel always keeps normalized struct timespec64 values in the ctime,
1525-
* which means that only the first 30 bits of the value are used. Use the
1526-
* 31st bit of the ctime's tv_nsec field as a flag to indicate that the value
1527-
* has been queried since it was last updated.
1528-
*/
1529-
#define I_CTIME_QUERIED (1L<<30)
1530-
15311514
/**
15321515
* inode_get_ctime - fetch the current ctime from the inode
15331516
* @inode: inode from which to fetch ctime
15341517
*
1535-
* Grab the current ctime tv_nsec field from the inode, mask off the
1536-
* I_CTIME_QUERIED flag and return it. This is mostly intended for use by
1537-
* internal consumers of the ctime that aren't concerned with ensuring a
1538-
* fine-grained update on the next change (e.g. when preparing to store
1539-
* the value in the backing store for later retrieval).
1540-
*
1541-
* This is safe to call regardless of whether the underlying filesystem
1542-
* is using multigrain timestamps.
1518+
* Grab the current ctime from the inode and return it.
15431519
*/
15441520
static inline struct timespec64 inode_get_ctime(const struct inode *inode)
15451521
{
1546-
struct timespec64 ctime;
1547-
1548-
ctime.tv_sec = inode->__i_ctime.tv_sec;
1549-
ctime.tv_nsec = inode->__i_ctime.tv_nsec & ~I_CTIME_QUERIED;
1550-
1551-
return ctime;
1522+
return inode->__i_ctime;
15521523
}
15531524

15541525
/**
@@ -2334,7 +2305,6 @@ struct file_system_type {
23342305
#define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */
23352306
#define FS_DISALLOW_NOTIFY_PERM 16 /* Disable fanotify permission events */
23362307
#define FS_ALLOW_IDMAP 32 /* FS has been updated to handle vfs idmappings. */
2337-
#define FS_MGTIME 64 /* FS uses multigrain timestamps */
23382308
#define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */
23392309
int (*init_fs_context)(struct fs_context *);
23402310
const struct fs_parameter_spec *parameters;
@@ -2358,17 +2328,6 @@ struct file_system_type {
23582328

23592329
#define MODULE_ALIAS_FS(NAME) MODULE_ALIAS("fs-" NAME)
23602330

2361-
/**
2362-
* is_mgtime: is this inode using multigrain timestamps
2363-
* @inode: inode to test for multigrain timestamps
2364-
*
2365-
* Return true if the inode uses multigrain timestamps, false otherwise.
2366-
*/
2367-
static inline bool is_mgtime(const struct inode *inode)
2368-
{
2369-
return inode->i_sb->s_type->fs_flags & FS_MGTIME;
2370-
}
2371-
23722331
extern struct dentry *mount_bdev(struct file_system_type *fs_type,
23732332
int flags, const char *dev_name, void *data,
23742333
int (*fill_super)(struct super_block *, void *, int));
@@ -3054,7 +3013,6 @@ extern void page_put_link(void *);
30543013
extern int page_symlink(struct inode *inode, const char *symname, int len);
30553014
extern const struct inode_operations page_symlink_inode_operations;
30563015
extern void kfree_link(void *);
3057-
void fill_mg_cmtime(struct kstat *stat, u32 request_mask, struct inode *inode);
30583016
void generic_fillattr(struct mnt_idmap *, u32, struct inode *, struct kstat *);
30593017
void generic_fill_statx_attr(struct inode *inode, struct kstat *stat);
30603018
extern int vfs_getattr_nosec(const struct path *, struct kstat *, u32, unsigned int);

mm/shmem.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4586,7 +4586,7 @@ static struct file_system_type shmem_fs_type = {
45864586
#endif
45874587
.kill_sb = kill_litter_super,
45884588
#ifdef CONFIG_SHMEM
4589-
.fs_flags = FS_USERNS_MOUNT | FS_ALLOW_IDMAP | FS_MGTIME,
4589+
.fs_flags = FS_USERNS_MOUNT | FS_ALLOW_IDMAP,
45904590
#else
45914591
.fs_flags = FS_USERNS_MOUNT,
45924592
#endif

0 commit comments

Comments
 (0)