Hi Deepa...

I installed this patch series on top of Linux 5.3-rc6 and ran xfstests on orangefs and got a regression... generic/258 failed with: "Timestamp wrapped"...

# cat results/generic/258.out.bad QA output created by 258 Creating file with timestamp of Jan 1, 1960 Testing for negative seconds since epoch Timestamp wrapped: 0 Timestamp wrapped (see /home/hubcap/xfstests-dev/results//generic/258.full for details)

Just to double check, I compiled Linux 5.3-rc6 without the timestamp series, and 258 passed... then added the patch series back and the failure returned...

Also.... when I was using "git am" to apply the patch series, the ceph patch failed, so I "git skipped" it...

I obtained the patch series from: https://github.com/deepa-hub/vfs limits today.

-Mike

On Thu, Aug 29, 2019 at 12:38 AM Deepa Dinamani deepa.kernel@gmail.com wrote:

Adding the others who were on original cc, just in case.

• Deepa

On Wed, Aug 28, 2019 at 9:12 PM Deepa Dinamani deepa.kernel@gmail.com wrote:

...

Hi Al, Arnd,

This is a pull request for filling in min and max timestamps for filesystems. I've added all the acks, and dropped the adfs patch. That will be merged through Russell's tree.

Thanks, Deepa

The following changes since commit 5d18cb62218608a1388858880ad3ec76d6cb0d3b:

Add linux-next specific files for 20190828 (2019-08-28 19:59:14 +1000)

are available in the Git repository at:

https://github.com/deepa-hub/vfs limits

for you to fetch changes up to f0f216afa4c7e4dee9121fde52ccf57f76119188:

isofs: Initialize filesystem timestamp ranges (2019-08-28 19:19:36 -0700)

---

Deepa Dinamani (19): vfs: Add file timestamp range support vfs: Add timestamp_truncate() api timestamp_truncate: Replace users of timespec64_trunc mount: Add mount warning for impending timestamp expiry utimes: Clamp the timestamps before update fs: Fill in max and min timestamps in superblock 9p: Fill min and max timestamps in sb ext4: Initialize timestamps limits fs: nfs: Initialize filesystem timestamp ranges fs: cifs: Initialize filesystem timestamp ranges fs: fat: Initialize filesystem timestamp ranges fs: affs: Initialize filesystem timestamp ranges fs: sysv: Initialize filesystem timestamp ranges fs: ceph: Initialize filesystem timestamp ranges fs: orangefs: Initialize filesystem timestamp ranges fs: hpfs: Initialize filesystem timestamp ranges fs: omfs: Initialize filesystem timestamp ranges pstore: fs superblock limits isofs: Initialize filesystem timestamp ranges

fs/9p/vfs_super.c | 6 +++++- fs/affs/amigaffs.c | 2 +- fs/affs/amigaffs.h | 3 +++ fs/affs/inode.c | 4 ++-- fs/affs/super.c | 4 ++++ fs/attr.c | 21 ++++++++++++--------- fs/befs/linuxvfs.c | 2 ++ fs/bfs/inode.c | 2 ++ fs/ceph/super.c | 2 ++ fs/cifs/cifsfs.c | 22 ++++++++++++++++++++++ fs/cifs/netmisc.c | 14 +++++++------- fs/coda/inode.c | 3 +++ fs/configfs/inode.c | 12 ++++++------ fs/cramfs/inode.c | 2 ++ fs/efs/super.c | 2 ++ fs/ext2/super.c | 2 ++ fs/ext4/ext4.h | 10 +++++++++- fs/ext4/super.c | 17 +++++++++++++++-- fs/f2fs/file.c | 21 ++++++++++++--------- fs/fat/inode.c | 12 ++++++++++++ fs/freevxfs/vxfs_super.c | 2 ++ fs/hpfs/hpfs_fn.h | 6 ++---- fs/hpfs/super.c | 2 ++ fs/inode.c | 33 ++++++++++++++++++++++++++++++++- fs/isofs/inode.c | 7 +++++++ fs/jffs2/fs.c | 3 +++ fs/jfs/super.c | 2 ++ fs/kernfs/inode.c | 7 +++---- fs/minix/inode.c | 2 ++ fs/namespace.c | 33 ++++++++++++++++++++++++++++++++- fs/nfs/super.c | 20 +++++++++++++++++++- fs/ntfs/inode.c | 21 ++++++++++++--------- fs/omfs/inode.c | 4 ++++ fs/orangefs/super.c | 2 ++ fs/pstore/ram.c | 2 ++ fs/qnx4/inode.c | 2 ++ fs/qnx6/inode.c | 2 ++ fs/reiserfs/super.c | 3 +++ fs/romfs/super.c | 2 ++ fs/squashfs/super.c | 2 ++ fs/super.c | 2 ++ fs/sysv/super.c | 5 ++++- fs/ubifs/file.c | 21 ++++++++++++--------- fs/ufs/super.c | 7 +++++++ fs/utimes.c | 6 ++---- fs/xfs/xfs_super.c | 2 ++ include/linux/fs.h | 5 +++++ include/linux/time64.h | 2 ++ 48 files changed, 298 insertions(+), 72 deletions(-)

On Fri, Aug 30, 2019 at 4:02 AM Deepa Dinamani deepa.kernel@gmail.com wrote:

On Thu, Aug 29, 2019 at 6:20 PM Mike Marshall hubcap@omnibond.com wrote:

...

Hi Deepa...

I installed this patch series on top of Linux 5.3-rc6 and ran xfstests on orangefs and got a regression... generic/258 failed with: "Timestamp wrapped"...

# cat results/generic/258.out.bad QA output created by 258 Creating file with timestamp of Jan 1, 1960 Testing for negative seconds since epoch Timestamp wrapped: 0 Timestamp wrapped (see /home/hubcap/xfstests-dev/results//generic/258.full for details)

Note that patch [16/20] https://lkml.org/lkml/2019/8/18/193 assumes that orangefs does not support negative timestamps. And, the reason was pointed out in the commit text:

---

Assume the limits as unsigned according to the below commit 98e8eef557a9 ("changed PVFS_time from int64_t to uint64_t") in https://github.com/waltligon/orangefs

Author: Neill Miller neillm@mcs.anl.gov Date: Thu Sep 2 15:00:38 2004 +0000

---

So the timestamp being wrapped to 0 in this case is correct behavior according to my patchset. The generic/258 assumes that the timestamps can be negative. If this is not true then it should not be run for this fs.

But, if you think the timestamp should support negative timestamps for orangefs, I'd be happy to change it.

I think it's unclear from the orangefs source code what the intention is, as there is a mixed of signed and unsigned types used for the inode stamps:

#define encode_PVFS_time encode_int64_t #define encode_int64_t(pptr,x) do { \ *(int64_t*) *(pptr) = cpu_to_le64(*(x)); \ *(pptr) += 8; \ } while (0) #define decode_PVFS_time decode_int64_t #define decode_int64_t(pptr,x) do { \ *(x) = le64_to_cpu(*(int64_t*) *(pptr)); \ *(pptr) += 8; \ } while (0)

This suggests that making it unsigned may have been an accident.

Then again, it's clearly and consistently printed as unsigned in user space:

gossip_debug( GOSSIP_GETATTR_DEBUG, " VERSION is %llu, mtime is %llu\n", llu(s_op->attr.mtime), llu(resp_attr->mtime));

A related issue I noticed is this:

PVFS_time PINT_util_mktime_version(PVFS_time time) { struct timeval t = {0,0}; PVFS_time version = (time << 32);

gettimeofday(&t, NULL); version |= (PVFS_time)t.tv_usec; return version; } PVFS_time PINT_util_mkversion_time(PVFS_time version) { return (PVFS_time)(version >> 32); } static PINT_sm_action getattr_verify_attribs( struct PINT_smcb *smcb, job_status_s *js_p) { ... resp_attr->mtime = PINT_util_mkversion_time(s_op->attr.mtime); ... }

which suggests that at least for some purposes, the mtime field is only an unsigned 32-bit number (1970..2106). From my readiing, this affects the on-disk format, but not the protocol implemented by the kernel.

atime and ctime are apparently 64-bit, but mtime is only 32-bit seconds, plus a 32-bit 'version'. I suppose the server could be fixed to allow a larger range, but probably would take it out of the 'version' bits, not the upper half.

To be on the safe side, I suppose the kernel can only assume an unsigned 32-bit range to be available. If the server gets extended beyond that, it would have to pass a feature flag.

Arnd

[resending, rebased onto linux v5.3-rc6, and dropped orangefs patch from the series]

Hi Al, Arnd,

This is a pull request for filling in min and max timestamps for filesystems. I've added all the acks, and dropped the adfs patch. That will be merged through Russell's tree.

Dropped orangefs until the maintainers decide what its limits should be.

The following changes since commit a55aa89aab90fae7c815b0551b07be37db359d76:

Linux 5.3-rc6 (2019-08-25 12:01:23 -0700)

are available in the Git repository at:

https://github.com/deepa-hub/vfs limits

for you to fetch changes up to 5ad32b3acded06183f40806f76b030c3143017bb:

isofs: Initialize filesystem timestamp ranges (2019-08-30 08:11:25 -0700)

----------------------------------------------------------------

- Deepa

Deepa Dinamani (18): vfs: Add file timestamp range support vfs: Add timestamp_truncate() api timestamp_truncate: Replace users of timespec64_trunc mount: Add mount warning for impending timestamp expiry utimes: Clamp the timestamps before update fs: Fill in max and min timestamps in superblock 9p: Fill min and max timestamps in sb ext4: Initialize timestamps limits fs: nfs: Initialize filesystem timestamp ranges fs: cifs: Initialize filesystem timestamp ranges fs: fat: Initialize filesystem timestamp ranges fs: affs: Initialize filesystem timestamp ranges fs: sysv: Initialize filesystem timestamp ranges fs: ceph: Initialize filesystem timestamp ranges fs: hpfs: Initialize filesystem timestamp ranges fs: omfs: Initialize filesystem timestamp ranges pstore: fs superblock limits isofs: Initialize filesystem timestamp ranges

fs/9p/vfs_super.c | 6 +++++- fs/affs/amigaffs.c | 2 +- fs/affs/amigaffs.h | 3 +++ fs/affs/inode.c | 4 ++-- fs/affs/super.c | 4 ++++ fs/attr.c | 21 ++++++++++++--------- fs/befs/linuxvfs.c | 2 ++ fs/bfs/inode.c | 2 ++ fs/ceph/super.c | 2 ++ fs/cifs/cifsfs.c | 22 ++++++++++++++++++++++ fs/cifs/netmisc.c | 14 +++++++------- fs/coda/inode.c | 3 +++ fs/configfs/inode.c | 12 ++++++------ fs/cramfs/inode.c | 2 ++ fs/efs/super.c | 2 ++ fs/ext2/super.c | 2 ++ fs/ext4/ext4.h | 10 +++++++++- fs/ext4/super.c | 17 +++++++++++++++-- fs/f2fs/file.c | 21 ++++++++++++--------- fs/fat/inode.c | 12 ++++++++++++ fs/freevxfs/vxfs_super.c | 2 ++ fs/hpfs/hpfs_fn.h | 6 ++---- fs/hpfs/super.c | 2 ++ fs/inode.c | 33 ++++++++++++++++++++++++++++++++- fs/isofs/inode.c | 7 +++++++ fs/jffs2/fs.c | 3 +++ fs/jfs/super.c | 2 ++ fs/kernfs/inode.c | 7 +++---- fs/minix/inode.c | 2 ++ fs/namespace.c | 33 ++++++++++++++++++++++++++++++++- fs/nfs/super.c | 20 +++++++++++++++++++- fs/ntfs/inode.c | 21 ++++++++++++--------- fs/omfs/inode.c | 4 ++++ fs/pstore/ram.c | 2 ++ fs/qnx4/inode.c | 2 ++ fs/qnx6/inode.c | 2 ++ fs/reiserfs/super.c | 3 +++ fs/romfs/super.c | 2 ++ fs/squashfs/super.c | 2 ++ fs/super.c | 2 ++ fs/sysv/super.c | 5 ++++- fs/ubifs/file.c | 21 ++++++++++++--------- fs/ufs/super.c | 7 +++++++ fs/utimes.c | 6 ++---- fs/xfs/xfs_super.c | 2 ++ include/linux/fs.h | 5 +++++ include/linux/time64.h | 2 ++ 47 files changed, 296 insertions(+), 72 deletions(-)