Linux 4.11 compat: statx support #8509

ryao · 2019-03-17T01:01:33Z

Motivation and Context

Support for statx was requested in #8507. This supersedes PR #8508.

Description

Linux 4.11 added a new statx system call that allows us to expose crtime
as btime. We do this by caching crtime in the znode to match how atime,
ctime and mtime are cached in the inode.

statx also introduced a new way of reporting whether the immutable,
append and nodump bits have been set. It adds support for reporting
compression and encryption, but the semantics on other filesystems is
not just to report compression/encryption, but to allow it to be turned
on/off at the file level. We do not support that.

We could implement semantics where we refuse to allow user modification
of the bit, but we would need to do a dnode_hold() in zfs_znode_alloc()
to find out encryption/compression information. That would introduce
locking that will have a minor (although unmeasured) performance cost.
It also would be inferior to zdb, which reports far more detailed
information. We therefore omit reporting of encryption/compression
through statx in favor of recommending that users interested in such
information use zdb.

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.

ryao · 2019-03-17T01:14:51Z

This has not been subject to tests on my local machines. Since I do not actually need this functionality myself, I am going to ask those that want it to test it on their machines to verify that the statx syscall is working properly.

module/zfs/zpl_inode.c

module/zfs/zfs_znode.c

codecov · 2019-03-17T17:17:11Z

Codecov Report

Merging #8509 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #8509      +/-   ##
==========================================
+ Coverage   78.55%   78.62%   +0.07%     
==========================================
  Files         380      380              
  Lines      116324   116337      +13     
==========================================
+ Hits        91375    91469      +94     
+ Misses      24949    24868      -81

Flag	Coverage Δ
#kernel	`79.14% <100%> (+0.07%)`	⬆️
#user	`67.08% <ø> (+0.14%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca6c7a9...b59eb93. Read the comment docs.

ryao · 2019-03-17T22:51:21Z

This should be attribute_mask, not result_mask here and on the other two.

You are right. This is what I get for breaking my rules against coding in the evening and not pushing without doing local tests (although the buildbot was expected to do build tests). A friend had pinged me about this yesterday evening saying that it wasn't possible, so I threw together a patch to show otherwise. Then I noticed that there were some minor performance tweaks to be made and more to statx than just btime, so they got added. The end result was this.

Anyway, I have properly looked at the kernel code this time. It still needs testing to confirm it works as expected, but the code should be right this time (fingers crossed).

Issue resolved

ryao · 2019-03-18T07:31:49Z

The project inherit tests breaking on Fedora might be a real regression because the pre-statx API sets a file attribute for FS_PROJINHERIT_FL, but statx does not allow us to expose that. This could indicate a bug in userspace or a bug in the tests, but I’ll need to set aside some time to look. I am just leaving a note here to indicate that I think there is a possible problem.

include/sys/zfs_znode.h

Linux 4.11 added a new statx system call that allows us to expose crtime as btime. We do this by caching crtime in the znode to match how atime, ctime and mtime are cached in the inode. statx also introduced a new way of reporting whether the immutable, append and nodump bits have been set. It adds support for reporting compression and encryption, but the semantics on other filesystems is not just to report compression/encryption, but to allow it to be turned on/off at the file level. We do not support that. We could implement semantics where we refuse to allow user modification of the bit, but we would need to do a dnode_hold() in zfs_znode_alloc() to find out encryption/compression information. That would introduce locking that will have a minor (although unmeasured) performance cost. It also would be inferior to zdb, which reports far more detailed information. We therefore omit reporting of encryption/compression through statx in favor of recommending that users interested in such information use zdb. Signed-off-by: Richard Yao <[email protected]> Closes openzfs#8507

behlendorf

Thanks, I'm all for adding support for the new interface.

However, before we can include this at a minimum we need to manually run the statx tests added to xfstests and verify they pass. We need to also add some basic tests cases to the ZTS since we are not automatically running xfstests.

As for reporting encryption and compression I think we should include that functionality in this PR. We don't need to allow setting either one of them. And for encryption it was intentionally decided not to allow it to be enabled/disabled at a file level of granularity.

As for the Fedora 29 failures those are unrelated. Currently the version of e2fsprogs in Fedora 29 has a known minor issue which was fixed, we're just waiting for them to update their packages.

ryao · 2019-03-19T15:30:00Z

@behlendorf Thanks for letting me know that the Fedora issue is pre-existing. That saved me some time.

My development setup is somewhat stale. I’ll update it later today, run the new statx XFS test suite shortly afterward and add tests to ZTS in a day or two. I’ll look into adding support for the compression and encryption flags at that time.

rlaager · 2019-04-14T19:37:18Z

I built this on Ubuntu 18.10 and manually built coreutils from its git master.

$ truncate -s 1G tank.img
$ sudo zpool create tank `pwd`/tank.img
$ cd /tank
$ sudo touch test

The normal stat(1) has no support for statx(2), so it cannot show a Birth time:

$ stat test
  File: test
  Size: 0         	Blocks: 1          IO Block: 131072 regular empty file
Device: 32h/50d	Inode: 2           Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-04-14 14:05:00.994796320 -0500
Modify: 2019-04-14 14:05:00.994796320 -0500
Change: 2019-04-14 14:05:00.994796320 -0500
 Birth: -

The stat(1) from coreutils git master supports statx(2) and shows a birth time:

$ ~/coreutils/tmp/usr/local/bin/stat test
  File: test
  Size: 0         	Blocks: 1          IO Block: 131072 regular empty file
Device: 32h/50d	Inode: 2           Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-04-14 14:05:00.994796320 -0500
Modify: 2019-04-14 14:05:00.994796320 -0500
Change: 2019-04-14 14:05:00.994796320 -0500
 Birth: 2019-04-14 14:05:00.994796320 -0500

Touching the file shows the mtime is updated, but the btime/crtime is the same:

$ sudo touch test
$ ~/coreutils/tmp/usr/local/bin/stat test
  File: test
  Size: 0         	Blocks: 1          IO Block: 131072 regular empty file
Device: 32h/50d	Inode: 2           Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-04-14 14:05:19.222901873 -0500
Modify: 2019-04-14 14:05:19.222901873 -0500
Change: 2019-04-14 14:05:19.222901873 -0500
 Birth: 2019-04-14 14:05:00.994796320 -0500

I am unable to xfstests (either from upstream or zfsonlinux/xfstests, branch zfs-upstream) to work at all. The latter fails to build.

behlendorf · 2019-04-16T18:38:27Z

@rlaager thanks for manually testing this. If we added some basic tests, like you've done above, to the ZFS Test Suite I think we'd be able to move forward with this. Running xfstests would be nice, but I understand it's currently challenging to run with ZFS, so we probably don't need to get hung up on that.

That leaves the only missing functionality I think being reporting encryption and compression. @ryao what are your plans for this PR. Would you mind if someone else picked up this work?

ryao · 2019-09-06T21:43:49Z

@behlendorf It has been on my todo list for a while, but it is fairly far down at the moment. I do not mind if some else does it.

ahrens · 2021-06-04T23:32:35Z

I noticed that this PR hasn’t been updated in some time. We would like to see this feature added, but it seems that it isn’t quite complete, so we’re going to close it for now. When you have time to revisit this work, feel free to reopen this PR or open a new one. Thanks for your contribution to OpenZFS and please continue to open PR’s to address issues that you encounter!

sunflsks · 2021-06-23T02:43:44Z

From what I've read so far, the only things that need to be added are reporting compression and encryption, and adding the tests to the ZTS, right? If so, do y'all mind if I (try to) pick up on this?

ahrens · 2021-06-23T03:00:05Z

@sunflsks Sounds great! Open a new PR when you're ready.

This was referenced Mar 17, 2019

Add support for birthtime #8508

Closed

ZFS should support statx(), at least for btime #8507

Closed

ryao force-pushed the statx branch from b703eb8 to c98dabf Compare March 17, 2019 01:12

ryao force-pushed the statx branch 3 times, most recently from 09fd73f to dd93db6 Compare March 17, 2019 03:33

rlaager suggested changes Mar 17, 2019

View reviewed changes

module/zfs/zpl_inode.c Show resolved Hide resolved

rlaager previously requested changes Mar 17, 2019

View reviewed changes

module/zfs/zfs_znode.c Show resolved Hide resolved

ryao force-pushed the statx branch 5 times, most recently from dce2f47 to e09a4ef Compare March 17, 2019 09:14

ryao requested a review from rlaager March 17, 2019 10:59

ryao force-pushed the statx branch from e09a4ef to 7d781de Compare March 17, 2019 22:46

ryao force-pushed the statx branch from 7d781de to 947bce3 Compare March 18, 2019 00:10

don-brady reviewed Mar 18, 2019

View reviewed changes

include/sys/zfs_znode.h Show resolved Hide resolved

ryao force-pushed the statx branch from bd25f05 to c0e0d20 Compare March 18, 2019 16:59

ryao force-pushed the statx branch from c0e0d20 to b59eb93 Compare March 18, 2019 17:38

behlendorf requested changes Mar 18, 2019

View reviewed changes

behlendorf added the Status: Revision Needed Changes are required for the PR to be accepted label Mar 19, 2019

rlaager approved these changes Aug 27, 2019

View reviewed changes

ahrens added the Status: Inactive Not being actively updated label Nov 15, 2019

ahrens closed this Jun 4, 2021

fryfrog mentioned this pull request Jun 6, 2021

Failure to read creation time on ZFS pkolaczk/fclones#55

Closed

adamdmoss mentioned this pull request Jul 26, 2021

Linux 4.11 compat: statx support #12432

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Linux 4.11 compat: statx support #8509

Linux 4.11 compat: statx support #8509

Uh oh!

ryao commented Mar 17, 2019

Uh oh!

ryao commented Mar 17, 2019

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 17, 2019 •

edited

Loading

Uh oh!

ryao commented Mar 17, 2019

Uh oh!

ryao commented Mar 18, 2019

Uh oh!

Uh oh!

behlendorf left a comment

Uh oh!

ryao commented Mar 19, 2019 •

edited

Loading

Uh oh!

rlaager commented Apr 14, 2019

Uh oh!

behlendorf commented Apr 16, 2019

Uh oh!

ryao commented Sep 6, 2019

Uh oh!

ahrens commented Jun 4, 2021

Uh oh!

sunflsks commented Jun 23, 2021

Uh oh!

ahrens commented Jun 23, 2021

Uh oh!

Uh oh!

Linux 4.11 compat: statx support #8509

Linux 4.11 compat: statx support #8509

Uh oh!

Conversation

ryao commented Mar 17, 2019

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

ryao commented Mar 17, 2019

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ryao commented Mar 17, 2019

Uh oh!

ryao commented Mar 18, 2019

Uh oh!

Uh oh!

behlendorf left a comment

Choose a reason for hiding this comment

Uh oh!

ryao commented Mar 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rlaager commented Apr 14, 2019

Uh oh!

behlendorf commented Apr 16, 2019

Uh oh!

ryao commented Sep 6, 2019

Uh oh!

ahrens commented Jun 4, 2021

Uh oh!

sunflsks commented Jun 23, 2021

Uh oh!

ahrens commented Jun 23, 2021

Uh oh!

Uh oh!

codecov bot commented Mar 17, 2019 •

edited

Loading

ryao commented Mar 19, 2019 •

edited

Loading