| Message ID | 381e5327-618b-13ab-ebe5-175f99abf7db@sandeen.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
2020-04-16 13:11 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: Hi Eric, > exfat atimes are restricted to only 2s granularity so after > we set an atime, round it down to the nearest 2s and set the > sub-second component of the timestamp to 0. Is there any reason why only atime is truncated with 2s granularity ? Thanks. > > Signed-off-by: Eric Sandeen <sandeen@sandeen.net> > --- > > V2: don't just zero tv_nsec, must also round down tv_sec I think. > > diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h > index 67d4e46fb810..d67fb8a6f770 100644 > --- a/fs/exfat/exfat_fs.h > +++ b/fs/exfat/exfat_fs.h > @@ -507,6 +507,7 @@ void exfat_msg(struct super_block *sb, const char *lv, > const char *fmt, ...) > __printf(3, 4) __cold; > void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 > *ts, > u8 tz, __le16 time, __le16 date, u8 time_ms); > +void exfat_truncate_atime(struct timespec64 *ts); > void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 > *ts, > u8 *tz, __le16 *time, __le16 *date, u8 *time_ms); > unsigned short exfat_calc_chksum_2byte(void *data, int len, > diff --git a/fs/exfat/file.c b/fs/exfat/file.c > index 483f683757aa..4f76764165cf 100644 > --- a/fs/exfat/file.c > +++ b/fs/exfat/file.c > @@ -273,6 +273,7 @@ int exfat_getattr(const struct path *path, struct kstat > *stat, > struct exfat_inode_info *ei = EXFAT_I(inode); > > generic_fillattr(inode, stat); > + exfat_truncate_atime(&stat->atime); > stat->result_mask |= STATX_BTIME; > stat->btime.tv_sec = ei->i_crtime.tv_sec; > stat->btime.tv_nsec = ei->i_crtime.tv_nsec; > @@ -339,6 +340,7 @@ int exfat_setattr(struct dentry *dentry, struct iattr > *attr) > } > > setattr_copy(inode, attr); > + exfat_truncate_atime(&inode->i_atime); > mark_inode_dirty(inode); > > out: > diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c > index 14a3300848f6..c8b33278d474 100644 > --- a/fs/exfat/misc.c > +++ b/fs/exfat/misc.c > @@ -88,7 +88,8 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, > struct timespec64 *ts, > if (time_ms) { > ts->tv_sec += time_ms / 100; > ts->tv_nsec = (time_ms % 100) * 10 * NSEC_PER_MSEC; > - } > + } else > + exfat_truncate_atime(ts); > > if (tz & EXFAT_TZ_VALID) > /* Adjust timezone to UTC0. */ > @@ -124,6 +125,13 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi, > struct timespec64 *ts, > *tz = EXFAT_TZ_VALID; > } > > +/* atime has only a 2-second resolution */ > +void exfat_truncate_atime(struct timespec64 *ts) > +{ > + ts->tv_sec = round_down(ts->tv_sec, 2); > + ts->tv_nsec = 0; > +} > + > unsigned short exfat_calc_chksum_2byte(void *data, int len, > unsigned short chksum, int type) > { > diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c > index a8681d91f569..b72d782568b8 100644 > --- a/fs/exfat/namei.c > +++ b/fs/exfat/namei.c > @@ -595,6 +595,7 @@ static int exfat_create(struct inode *dir, struct dentry > *dentry, umode_t mode, > inode_inc_iversion(inode); > inode->i_mtime = inode->i_atime = inode->i_ctime = > EXFAT_I(inode)->i_crtime = current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > /* timestamp is already written, so mark_inode_dirty() is unneeded. */ > > d_instantiate(dentry, inode); > @@ -854,6 +855,7 @@ static int exfat_unlink(struct inode *dir, struct dentry > *dentry) > > inode_inc_iversion(dir); > dir->i_mtime = dir->i_atime = current_time(dir); > + exfat_truncate_atime(&dir->i_atime); > if (IS_DIRSYNC(dir)) > exfat_sync_inode(dir); > else > @@ -861,6 +863,7 @@ static int exfat_unlink(struct inode *dir, struct dentry > *dentry) > > clear_nlink(inode); > inode->i_mtime = inode->i_atime = current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > exfat_unhash_inode(inode); > exfat_d_version_set(dentry, inode_query_iversion(dir)); > unlock: > @@ -903,6 +906,7 @@ static int exfat_mkdir(struct inode *dir, struct dentry > *dentry, umode_t mode) > inode_inc_iversion(inode); > inode->i_mtime = inode->i_atime = inode->i_ctime = > EXFAT_I(inode)->i_crtime = current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > /* timestamp is already written, so mark_inode_dirty() is unneeded. */ > > d_instantiate(dentry, inode); > @@ -1019,6 +1023,7 @@ static int exfat_rmdir(struct inode *dir, struct > dentry *dentry) > > inode_inc_iversion(dir); > dir->i_mtime = dir->i_atime = current_time(dir); > + exfat_truncate_atime(&dir->i_atime); > if (IS_DIRSYNC(dir)) > exfat_sync_inode(dir); > else > @@ -1027,6 +1032,7 @@ static int exfat_rmdir(struct inode *dir, struct > dentry *dentry) > > clear_nlink(inode); > inode->i_mtime = inode->i_atime = current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > exfat_unhash_inode(inode); > exfat_d_version_set(dentry, inode_query_iversion(dir)); > unlock: > @@ -1387,6 +1393,7 @@ static int exfat_rename(struct inode *old_dir, struct > dentry *old_dentry, > inode_inc_iversion(new_dir); > new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime = > EXFAT_I(new_dir)->i_crtime = current_time(new_dir); > + exfat_truncate_atime(&new_dir->i_atime); > if (IS_DIRSYNC(new_dir)) > exfat_sync_inode(new_dir); > else > diff --git a/fs/exfat/super.c b/fs/exfat/super.c > index 16ed202ef527..aed62cafb0da 100644 > --- a/fs/exfat/super.c > +++ b/fs/exfat/super.c > @@ -351,6 +351,7 @@ static int exfat_read_root(struct inode *inode) > exfat_save_attr(inode, ATTR_SUBDIR); > inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime = > current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > exfat_cache_init_inode(inode); > return 0; > } > >
2020-04-18 22:03 GMT+09:00, Namjae Jeon <linkinjeon@kernel.org>: > 2020-04-16 13:11 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: > Hi Eric, > >> exfat atimes are restricted to only 2s granularity so after >> we set an atime, round it down to the nearest 2s and set the >> sub-second component of the timestamp to 0. > Is there any reason why only atime is truncated with 2s granularity ? I will directly update description more. Sorry to bother you! > > Thanks. >> >> Signed-off-by: Eric Sandeen <sandeen@sandeen.net> >> --- >> >> V2: don't just zero tv_nsec, must also round down tv_sec I think. >> >> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h >> index 67d4e46fb810..d67fb8a6f770 100644 >> --- a/fs/exfat/exfat_fs.h >> +++ b/fs/exfat/exfat_fs.h >> @@ -507,6 +507,7 @@ void exfat_msg(struct super_block *sb, const char >> *lv, >> const char *fmt, ...) >> __printf(3, 4) __cold; >> void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 >> *ts, >> u8 tz, __le16 time, __le16 date, u8 time_ms); >> +void exfat_truncate_atime(struct timespec64 *ts); >> void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 >> *ts, >> u8 *tz, __le16 *time, __le16 *date, u8 *time_ms); >> unsigned short exfat_calc_chksum_2byte(void *data, int len, >> diff --git a/fs/exfat/file.c b/fs/exfat/file.c >> index 483f683757aa..4f76764165cf 100644 >> --- a/fs/exfat/file.c >> +++ b/fs/exfat/file.c >> @@ -273,6 +273,7 @@ int exfat_getattr(const struct path *path, struct >> kstat >> *stat, >> struct exfat_inode_info *ei = EXFAT_I(inode); >> >> generic_fillattr(inode, stat); >> + exfat_truncate_atime(&stat->atime); >> stat->result_mask |= STATX_BTIME; >> stat->btime.tv_sec = ei->i_crtime.tv_sec; >> stat->btime.tv_nsec = ei->i_crtime.tv_nsec; >> @@ -339,6 +340,7 @@ int exfat_setattr(struct dentry *dentry, struct iattr >> *attr) >> } >> >> setattr_copy(inode, attr); >> + exfat_truncate_atime(&inode->i_atime); >> mark_inode_dirty(inode); >> >> out: >> diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c >> index 14a3300848f6..c8b33278d474 100644 >> --- a/fs/exfat/misc.c >> +++ b/fs/exfat/misc.c >> @@ -88,7 +88,8 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, >> struct timespec64 *ts, >> if (time_ms) { >> ts->tv_sec += time_ms / 100; >> ts->tv_nsec = (time_ms % 100) * 10 * NSEC_PER_MSEC; >> - } >> + } else >> + exfat_truncate_atime(ts); >> >> if (tz & EXFAT_TZ_VALID) >> /* Adjust timezone to UTC0. */ >> @@ -124,6 +125,13 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi, >> struct timespec64 *ts, >> *tz = EXFAT_TZ_VALID; >> } >> >> +/* atime has only a 2-second resolution */ >> +void exfat_truncate_atime(struct timespec64 *ts) >> +{ >> + ts->tv_sec = round_down(ts->tv_sec, 2); >> + ts->tv_nsec = 0; >> +} >> + >> unsigned short exfat_calc_chksum_2byte(void *data, int len, >> unsigned short chksum, int type) >> { >> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c >> index a8681d91f569..b72d782568b8 100644 >> --- a/fs/exfat/namei.c >> +++ b/fs/exfat/namei.c >> @@ -595,6 +595,7 @@ static int exfat_create(struct inode *dir, struct >> dentry >> *dentry, umode_t mode, >> inode_inc_iversion(inode); >> inode->i_mtime = inode->i_atime = inode->i_ctime = >> EXFAT_I(inode)->i_crtime = current_time(inode); >> + exfat_truncate_atime(&inode->i_atime); >> /* timestamp is already written, so mark_inode_dirty() is unneeded. */ >> >> d_instantiate(dentry, inode); >> @@ -854,6 +855,7 @@ static int exfat_unlink(struct inode *dir, struct >> dentry >> *dentry) >> >> inode_inc_iversion(dir); >> dir->i_mtime = dir->i_atime = current_time(dir); >> + exfat_truncate_atime(&dir->i_atime); >> if (IS_DIRSYNC(dir)) >> exfat_sync_inode(dir); >> else >> @@ -861,6 +863,7 @@ static int exfat_unlink(struct inode *dir, struct >> dentry >> *dentry) >> >> clear_nlink(inode); >> inode->i_mtime = inode->i_atime = current_time(inode); >> + exfat_truncate_atime(&inode->i_atime); >> exfat_unhash_inode(inode); >> exfat_d_version_set(dentry, inode_query_iversion(dir)); >> unlock: >> @@ -903,6 +906,7 @@ static int exfat_mkdir(struct inode *dir, struct >> dentry >> *dentry, umode_t mode) >> inode_inc_iversion(inode); >> inode->i_mtime = inode->i_atime = inode->i_ctime = >> EXFAT_I(inode)->i_crtime = current_time(inode); >> + exfat_truncate_atime(&inode->i_atime); >> /* timestamp is already written, so mark_inode_dirty() is unneeded. */ >> >> d_instantiate(dentry, inode); >> @@ -1019,6 +1023,7 @@ static int exfat_rmdir(struct inode *dir, struct >> dentry *dentry) >> >> inode_inc_iversion(dir); >> dir->i_mtime = dir->i_atime = current_time(dir); >> + exfat_truncate_atime(&dir->i_atime); >> if (IS_DIRSYNC(dir)) >> exfat_sync_inode(dir); >> else >> @@ -1027,6 +1032,7 @@ static int exfat_rmdir(struct inode *dir, struct >> dentry *dentry) >> >> clear_nlink(inode); >> inode->i_mtime = inode->i_atime = current_time(inode); >> + exfat_truncate_atime(&inode->i_atime); >> exfat_unhash_inode(inode); >> exfat_d_version_set(dentry, inode_query_iversion(dir)); >> unlock: >> @@ -1387,6 +1393,7 @@ static int exfat_rename(struct inode *old_dir, >> struct >> dentry *old_dentry, >> inode_inc_iversion(new_dir); >> new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime = >> EXFAT_I(new_dir)->i_crtime = current_time(new_dir); >> + exfat_truncate_atime(&new_dir->i_atime); >> if (IS_DIRSYNC(new_dir)) >> exfat_sync_inode(new_dir); >> else >> diff --git a/fs/exfat/super.c b/fs/exfat/super.c >> index 16ed202ef527..aed62cafb0da 100644 >> --- a/fs/exfat/super.c >> +++ b/fs/exfat/super.c >> @@ -351,6 +351,7 @@ static int exfat_read_root(struct inode *inode) >> exfat_save_attr(inode, ATTR_SUBDIR); >> inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime = >> current_time(inode); >> + exfat_truncate_atime(&inode->i_atime); >> exfat_cache_init_inode(inode); >> return 0; >> } >> >> >
On 4/18/20 8:03 AM, Namjae Jeon wrote: > 2020-04-16 13:11 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: > Hi Eric, > >> exfat atimes are restricted to only 2s granularity so after >> we set an atime, round it down to the nearest 2s and set the >> sub-second component of the timestamp to 0. > Is there any reason why only atime is truncated with 2s granularity ? You're the expert, so I might be wrong! :) My reading of the spec & code is that every timestamp has 2s granularity, in the main time field, and some timestamps also have another field with 10ms granularity, which can range from 0 to 199, i.e. 0 to 1990 ms, i.e. 0 to 1.99 seconds in the _ms field. i.e. the _ms fields can add more than 1 second to the timestamp, right? struct { __u8 num_ext; __le16 checksum; __le16 attr; __le16 reserved1; __le16 create_time; __le16 create_date; __le16 modify_time; __le16 modify_date; __le16 access_time; __le16 access_date; __u8 create_time_ms; __u8 modify_time_ms; __u8 create_tz; __u8 modify_tz; __u8 access_tz; __u8 reserved2[7]; } __packed file; /* file directory entry */ and per the publiished spec, 7.4.8.1 DoubleSeconds Field The DoubleSeconds field shall describe the seconds portion of the Timestamp field, in two-second multiples. The valid range of values for this field shall be: 0, which represents 0 seconds 29, which represents 58 seconds 7.4.9 10msIncrement Fields 10msIncrement fields shall provide additional time resolution to their corresponding Timestamp fields in ten-millisecond multiples. The valid range of values for these fields shall be: At least 0, which represents 0 milliseconds At most 199, which represents 1990 milliseconds since access_time has no corresponding 10msIncrement field, my understanding was that it could only have a 2s granularity. Happy to have the patch dropped or corrected if I read this wrong, though. -Eric
On 4/18/20 11:04 AM, Eric Sandeen wrote:
> since access_time has no corresponding 10msIncrement field, my understanding was that it could only have a 2s granularity.
Maybe your concern is whether the other _time fields should also be
truncated to 2s even though they have the _ms field? I don't think so; the
s_time_gran already limits in-core timestamp resolution to 10ms, which will
be properly translated when the inode is written to disk.
atime has a different granularity though, so s_time_gran doens't help and we
must manually change it to 2s whenever we call something like current_time(), which
only enforces the 10ms granularity.
So for cases like this:
generic_fillattr(inode, stat);
+ exfat_truncate_atime(&stat->atime);
or this:
inode->i_mtime = inode->i_atime = inode->i_ctime =
EXFAT_I(inode)->i_crtime = current_time(inode);
+ exfat_truncate_atime(&inode->i_atime);
I think it's clearly the right thing to do; anything finer than 2s will be thrown
away when the vfs inode atime is translated to the disk format, so we should never
hold finer granularity in the in-memory vfs inode.
However, in exfat_get_entry_time() maybe all we need to do is set ts->tv_nsec to 0;
that might be clearer.
-Eric
On 4/18/20 11:40 AM, Eric Sandeen wrote: > On 4/18/20 11:04 AM, Eric Sandeen wrote: >> since access_time has no corresponding 10msIncrement field, my understanding was that it could only have a 2s granularity. > > Maybe your concern is whether the other _time fields should also be > truncated to 2s even though they have the _ms field? I don't think so; the > s_time_gran already limits in-core timestamp resolution to 10ms, which will > be properly translated when the inode is written to disk. > > atime has a different granularity though, so s_time_gran doens't help and we > must manually change it to 2s whenever we call something like current_time(), which > only enforces the 10ms granularity. > > So for cases like this: > > generic_fillattr(inode, stat); > + exfat_truncate_atime(&stat->atime); > > or this: > > inode->i_mtime = inode->i_atime = inode->i_ctime = > EXFAT_I(inode)->i_crtime = current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > > I think it's clearly the right thing to do; anything finer than 2s will be thrown > away when the vfs inode atime is translated to the disk format, so we should never > hold finer granularity in the in-memory vfs inode. > > However, in exfat_get_entry_time() maybe all we need to do is set ts->tv_nsec to 0; > that might be clearer. so maybe this is better - diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c index c8b33278d474..2c5629b4e7e6 100644 --- a/fs/exfat/misc.c +++ b/fs/exfat/misc.c @@ -89,7 +89,7 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts, ts->tv_sec += time_ms / 100; ts->tv_nsec = (time_ms % 100) * 10 * NSEC_PER_MSEC; } else - exfat_truncate_atime(ts); + ts->tv_nsec = 0; if (tz & EXFAT_TZ_VALID) /* Adjust timezone to UTC0. */ because the conversion should already limit tv_sec to a 2s granularity. -Eric
2020-04-19 1:04 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: > On 4/18/20 8:03 AM, Namjae Jeon wrote: >> 2020-04-16 13:11 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: >> Hi Eric, >> >>> exfat atimes are restricted to only 2s granularity so after >>> we set an atime, round it down to the nearest 2s and set the >>> sub-second component of the timestamp to 0. >> Is there any reason why only atime is truncated with 2s granularity ? > > You're the expert, so I might be wrong! :) > > My reading of the spec & code is that every timestamp has 2s granularity, in > the > main time field, and some timestamps also have another field with 10ms > granularity, > which can range from 0 to 199, i.e. 0 to 1990 ms, i.e. 0 to 1.99 seconds in > the _ms > field. i.e. the _ms fields can add more than 1 second to the timestamp, > right? Right. > > struct { > __u8 num_ext; > __le16 checksum; > __le16 attr; > __le16 reserved1; > __le16 create_time; > __le16 create_date; > __le16 modify_time; > __le16 modify_date; > __le16 access_time; > __le16 access_date; > __u8 create_time_ms; > __u8 modify_time_ms; > __u8 create_tz; > __u8 modify_tz; > __u8 access_tz; > __u8 reserved2[7]; > } __packed file; /* file directory entry */ > > and per the publiished spec, > > 7.4.8.1 DoubleSeconds Field > > The DoubleSeconds field shall describe the seconds portion of the Timestamp > field, in two-second multiples. > > The valid range of values for this field shall be: > > 0, which represents 0 seconds > > 29, which represents 58 seconds > > 7.4.9 10msIncrement Fields > > 10msIncrement fields shall provide additional time resolution to their > corresponding Timestamp fields in ten-millisecond multiples. > > The valid range of values for these fields shall be: > > At least 0, which represents 0 milliseconds > > At most 199, which represents 1990 milliseconds > > > since access_time has no corresponding 10msIncrement field, my understanding > was that it could only have a 2s granularity. Right. > > Happy to have the patch dropped or corrected if I read this wrong, though. Thanks for your detail explanation:) I just wanted to update the patch's descirption and function's comments to make it easier for others to understand this patch and code. > > -Eric >
2020-04-19 1:40 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: > On 4/18/20 11:04 AM, Eric Sandeen wrote: >> since access_time has no corresponding 10msIncrement field, my >> understanding was that it could only have a 2s granularity. > > Maybe your concern is whether the other _time fields should also be > truncated to 2s even though they have the _ms field? I don't think so; the > s_time_gran already limits in-core timestamp resolution to 10ms, which will > be properly translated when the inode is written to disk. > > atime has a different granularity though, so s_time_gran doens't help and > we > must manually change it to 2s whenever we call something like > current_time(), which > only enforces the 10ms granularity. > > So for cases like this: > > generic_fillattr(inode, stat); > + exfat_truncate_atime(&stat->atime); > > or this: > > inode->i_mtime = inode->i_atime = inode->i_ctime = > EXFAT_I(inode)->i_crtime = current_time(inode); > + exfat_truncate_atime(&inode->i_atime); > > I think it's clearly the right thing to do; anything finer than 2s will be > thrown > away when the vfs inode atime is translated to the disk format, so we should > never > hold finer granularity in the in-memory vfs inode. > > However, in exfat_get_entry_time() maybe all we need to do is set > ts->tv_nsec to 0; > that might be clearer. Right. Thanks! > > -Eric >
2020-04-19 2:06 GMT+09:00, Eric Sandeen <sandeen@sandeen.net>: > > > On 4/18/20 11:40 AM, Eric Sandeen wrote: >> On 4/18/20 11:04 AM, Eric Sandeen wrote: >>> since access_time has no corresponding 10msIncrement field, my >>> understanding was that it could only have a 2s granularity. >> >> Maybe your concern is whether the other _time fields should also be >> truncated to 2s even though they have the _ms field? I don't think so; >> the >> s_time_gran already limits in-core timestamp resolution to 10ms, which >> will >> be properly translated when the inode is written to disk. >> >> atime has a different granularity though, so s_time_gran doens't help and >> we >> must manually change it to 2s whenever we call something like >> current_time(), which >> only enforces the 10ms granularity. >> >> So for cases like this: >> >> generic_fillattr(inode, stat); >> + exfat_truncate_atime(&stat->atime); >> >> or this: >> >> inode->i_mtime = inode->i_atime = inode->i_ctime = >> EXFAT_I(inode)->i_crtime = current_time(inode); >> + exfat_truncate_atime(&inode->i_atime); >> >> I think it's clearly the right thing to do; anything finer than 2s will be >> thrown >> away when the vfs inode atime is translated to the disk format, so we >> should never >> hold finer granularity in the in-memory vfs inode. >> >> However, in exfat_get_entry_time() maybe all we need to do is set >> ts->tv_nsec to 0; >> that might be clearer. > > so maybe this is better - > > diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c > index c8b33278d474..2c5629b4e7e6 100644 > --- a/fs/exfat/misc.c > +++ b/fs/exfat/misc.c > @@ -89,7 +89,7 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, > struct timespec64 *ts, > ts->tv_sec += time_ms / 100; > ts->tv_nsec = (time_ms % 100) * 10 * NSEC_PER_MSEC; > } else > - exfat_truncate_atime(ts); > + ts->tv_nsec = 0; > > if (tz & EXFAT_TZ_VALID) > /* Adjust timezone to UTC0. */ > > > because the conversion should already limit tv_sec to a 2s granularity. Right. I will update it and replace it with old one. Thanks! > > -Eric >
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index 67d4e46fb810..d67fb8a6f770 100644 --- a/fs/exfat/exfat_fs.h +++ b/fs/exfat/exfat_fs.h @@ -507,6 +507,7 @@ void exfat_msg(struct super_block *sb, const char *lv, const char *fmt, ...) __printf(3, 4) __cold; void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts, u8 tz, __le16 time, __le16 date, u8 time_ms); +void exfat_truncate_atime(struct timespec64 *ts); void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts, u8 *tz, __le16 *time, __le16 *date, u8 *time_ms); unsigned short exfat_calc_chksum_2byte(void *data, int len, diff --git a/fs/exfat/file.c b/fs/exfat/file.c index 483f683757aa..4f76764165cf 100644 --- a/fs/exfat/file.c +++ b/fs/exfat/file.c @@ -273,6 +273,7 @@ int exfat_getattr(const struct path *path, struct kstat *stat, struct exfat_inode_info *ei = EXFAT_I(inode); generic_fillattr(inode, stat); + exfat_truncate_atime(&stat->atime); stat->result_mask |= STATX_BTIME; stat->btime.tv_sec = ei->i_crtime.tv_sec; stat->btime.tv_nsec = ei->i_crtime.tv_nsec; @@ -339,6 +340,7 @@ int exfat_setattr(struct dentry *dentry, struct iattr *attr) } setattr_copy(inode, attr); + exfat_truncate_atime(&inode->i_atime); mark_inode_dirty(inode); out: diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c index 14a3300848f6..c8b33278d474 100644 --- a/fs/exfat/misc.c +++ b/fs/exfat/misc.c @@ -88,7 +88,8 @@ void exfat_get_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts, if (time_ms) { ts->tv_sec += time_ms / 100; ts->tv_nsec = (time_ms % 100) * 10 * NSEC_PER_MSEC; - } + } else + exfat_truncate_atime(ts); if (tz & EXFAT_TZ_VALID) /* Adjust timezone to UTC0. */ @@ -124,6 +125,13 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts, *tz = EXFAT_TZ_VALID; } +/* atime has only a 2-second resolution */ +void exfat_truncate_atime(struct timespec64 *ts) +{ + ts->tv_sec = round_down(ts->tv_sec, 2); + ts->tv_nsec = 0; +} + unsigned short exfat_calc_chksum_2byte(void *data, int len, unsigned short chksum, int type) { diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index a8681d91f569..b72d782568b8 100644 --- a/fs/exfat/namei.c +++ b/fs/exfat/namei.c @@ -595,6 +595,7 @@ static int exfat_create(struct inode *dir, struct dentry *dentry, umode_t mode, inode_inc_iversion(inode); inode->i_mtime = inode->i_atime = inode->i_ctime = EXFAT_I(inode)->i_crtime = current_time(inode); + exfat_truncate_atime(&inode->i_atime); /* timestamp is already written, so mark_inode_dirty() is unneeded. */ d_instantiate(dentry, inode); @@ -854,6 +855,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry) inode_inc_iversion(dir); dir->i_mtime = dir->i_atime = current_time(dir); + exfat_truncate_atime(&dir->i_atime); if (IS_DIRSYNC(dir)) exfat_sync_inode(dir); else @@ -861,6 +863,7 @@ static int exfat_unlink(struct inode *dir, struct dentry *dentry) clear_nlink(inode); inode->i_mtime = inode->i_atime = current_time(inode); + exfat_truncate_atime(&inode->i_atime); exfat_unhash_inode(inode); exfat_d_version_set(dentry, inode_query_iversion(dir)); unlock: @@ -903,6 +906,7 @@ static int exfat_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) inode_inc_iversion(inode); inode->i_mtime = inode->i_atime = inode->i_ctime = EXFAT_I(inode)->i_crtime = current_time(inode); + exfat_truncate_atime(&inode->i_atime); /* timestamp is already written, so mark_inode_dirty() is unneeded. */ d_instantiate(dentry, inode); @@ -1019,6 +1023,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry) inode_inc_iversion(dir); dir->i_mtime = dir->i_atime = current_time(dir); + exfat_truncate_atime(&dir->i_atime); if (IS_DIRSYNC(dir)) exfat_sync_inode(dir); else @@ -1027,6 +1032,7 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry) clear_nlink(inode); inode->i_mtime = inode->i_atime = current_time(inode); + exfat_truncate_atime(&inode->i_atime); exfat_unhash_inode(inode); exfat_d_version_set(dentry, inode_query_iversion(dir)); unlock: @@ -1387,6 +1393,7 @@ static int exfat_rename(struct inode *old_dir, struct dentry *old_dentry, inode_inc_iversion(new_dir); new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime = EXFAT_I(new_dir)->i_crtime = current_time(new_dir); + exfat_truncate_atime(&new_dir->i_atime); if (IS_DIRSYNC(new_dir)) exfat_sync_inode(new_dir); else diff --git a/fs/exfat/super.c b/fs/exfat/super.c index 16ed202ef527..aed62cafb0da 100644 --- a/fs/exfat/super.c +++ b/fs/exfat/super.c @@ -351,6 +351,7 @@ static int exfat_read_root(struct inode *inode) exfat_save_attr(inode, ATTR_SUBDIR); inode->i_mtime = inode->i_atime = inode->i_ctime = ei->i_crtime = current_time(inode); + exfat_truncate_atime(&inode->i_atime); exfat_cache_init_inode(inode); return 0; }
exfat atimes are restricted to only 2s granularity so after we set an atime, round it down to the nearest 2s and set the sub-second component of the timestamp to 0. Signed-off-by: Eric Sandeen <sandeen@sandeen.net> --- V2: don't just zero tv_nsec, must also round down tv_sec I think.