Page MenuHomeFreeBSD

stat(2): expand list of syscalls that modify st_mtim
ClosedPublic

Authored by asomers on Sun, Dec 2, 11:35 PM.

Details

Summary

stat(2): expand list of syscalls that modify st_mtim

creat, link, rename, rmdir, symlink, truncate, and unlink all modify st_mtim
as well as st_ctim. Verified on UFS and ZFS.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

asomers created this revision.Sun, Dec 2, 11:35 PM
kib added a comment.Mon, Dec 3, 11:55 AM

I do not understand this change. Are you claiming that link(2) modifies mtim ? If yes, for which vnode ?

In D18410#391986, @kib wrote:

I do not understand this change. Are you claiming that link(2) modifies mtim ? If yes, for which vnode ?

Yes. link(2) and every other syscall that creates a new file, like mkdir(2), mkfifo(2), creat(2), etc modify the ctime and mtime of the parent directory. Also, I should remove mknod(2) from the list of syscalls that modify atime.

asomers updated this revision to Diff 51531.Mon, Dec 3, 2:24 PM

Also note that mknod does not update atime

kib added a comment.EditedMon, Dec 3, 2:46 PM
In D18410#391986, @kib wrote:

I do not understand this change. Are you claiming that link(2) modifies mtim ? If yes, for which vnode ?

Yes. link(2) and every other syscall that creates a new file, like mkdir(2), mkfifo(2), creat(2), etc modify the ctime and mtime of the parent directory. Also, I should remove mknod(2) from the list of syscalls that modify atime.

Ok then, in the current form the patch is not correct. For link(2), we have two vnodes, the one which gets additional name, and the directory where the name is placed. The patch makes the confusion between them.

One option to make the patch correct is to add a new list with enumeration of syscalls which change mtime of the directories participating in the operation (so e.g. creat(2) would be listed twice, one time for the created vnode, and another for the directory where the link to the created vnode is placed).

Another option is to just note that all syscalls that modify directories update its mtime. This seems much more maintainable, because we do add new syscalls, so the list becomes stale fast. Also, I think you would need to list from 1/4 to 1/3 of all syscalls to make the list complete to start from.

Also, for ordinary file accesses, the page fails to mention modifications of the writeable memory mappings of the files.

asomers updated this revision to Diff 51532.Mon, Dec 3, 4:06 PM

Leave syscall list open-ended.

Also, for ordinary file accesses, the page fails to mention modifications of the writeable memory mappings of the files.

What wording would you like for that?

kib added inline comments.Mon, Dec 3, 4:20 PM
lib/libc/sys/stat.2
211 ↗(On Diff #51532)

This is too loose and really unusal formulation. You might say something like

Any syscall which modifies directory content changes st_mtim for the affected directory.
For instance,
.Xr creat 2 ,
.Xr mknod 2 ,
...
218 ↗(On Diff #51532)

Not 'parent'.

223 ↗(On Diff #51532)

ctim is only updated when metadata is changed, not data. So for instance, write(2) does not cause update to ctime if file size and file mode are unchanged.

asomers marked an inline comment as done.Mon, Dec 3, 4:33 PM
asomers added inline comments.
lib/libc/sys/stat.2
223 ↗(On Diff #51532)

Actually, it does. I verified that both write and pwrite do update ctime even if file size is unchanged, on both UFS and ZFS.

asomers updated this revision to Diff 51534.Mon, Dec 3, 4:40 PM

Use kib's preferred wording for directory-updating syscalls.

kib added inline comments.Mon, Dec 3, 5:55 PM
lib/libc/sys/stat.2
223 ↗(On Diff #51532)

In fact yes, I checked UFS and tmpfs. I can see a reasoning there, mtime needs to be updated, but this changes inode content, which implies ctime change.

So might be mention this and then there is no need to say 'affects file data or metadata'.

asomers updated this revision to Diff 51609.Wed, Dec 5, 4:26 PM

Clarify why write(2) updates st_ctim

kib accepted this revision.Wed, Dec 5, 5:20 PM
This revision is now accepted and ready to land.Wed, Dec 5, 5:20 PM
This revision was automatically updated to reflect the committed changes.