mount: mark windows files archived and ignore a zero timestamp (#10559)

* mount: mark windows files archived and ignore a zero timestamp

Windows synthesises NORMAL when a file reports no attributes at all, which
is not the same as ARCHIVE and is what create_fileattr_test checks.

Utimens also wrote a zero timestamp through. Windows sends zero for a field
it is not setting, and storing it put 1970 in the atime overlay, which then
overrode the entry's real time — so a file created a moment ago reported an
access time of 1970 whenever the caller asked through an open handle.
Reading the path instead went down a different route and looked right,
which is why a probe of a fresh file showed nothing wrong.

* mount: match the file type by its mask, and only treat the epoch as unset

S_IFDIR is part of the multi-bit type field rather than a flag, so masking
against it alone also matched a symlink, which shares the bit. A regular
file is now identified by the type mask.

Rejecting every timestamp at or below zero also rejected a date genuinely
before 1970. Only the epoch itself is what Windows sends for a field it is
not setting, so that is all that is refused.

create_fileattr goes back on the known-failures list: the archive fix
works and the test simply moves on to ask for READONLY too, which needs
Chflags. Taking it off was premature.

* mount: drop the time overlays when an inode is released

atimeMap and dirMtimeMap are keyed by inode and were only ever trimmed by
a random eviction at capacity. Inodes are derived from the path, so a
delete and recreate hands the same number to a different file, which then
reported the previous file's access time — a file created a moment ago
answering with a time from long before it existed.

Cleared when Forget actually releases the inode, not on every decrement:
a partial forget still has users. Forget now reports that so callers
holding state keyed by the inode know when to drop it.

* ci: keep getfileinfo listed while its access time is unexplained

Two causes have been fixed and neither closed it, so the honest state is
listed-with-a-reason rather than removed in hope.

* mount: drop timestamp overlays while the inode table is locked

Forget released the inode under the table's lock but cleaned up the
atime and dir-mtime overlays after returning from it. Inode numbers are
derived from the path, so a lookup arriving in that window is handed the
same number back and can store a time that the cleanup then deletes.

Run the cleanup at the release point instead, as a callback under the
lock. The directory-cache purge stays deferred until after the unlock,
where it has to be.

Claude-Session: https://claude.ai/code/session_01EgY2QA3iiPtiu6ww3P2EBn
This commit is contained in:
Chris Lu
2026-08-04 16:42:33 -07:00
committed by GitHub
parent 50464702d2
commit b1fecf3b44
7 changed files with 100 additions and 17 deletions
+10 -3
View File
@@ -58,11 +58,18 @@ exec_rename_dir_test
# File information
# ----------------
# Windows attributes (hidden, system, readonly) and creation time are not
# round-tripped: attrToStat leaves Flags unset and reports ctime as birthtime.
# A regular file reports as archived now, but hidden, system and readonly are
# still not round-tripped: create_fileattr gets past the archive check and then
# asks for READONLY as well, which needs Chflags. Creation time is reported as
# ctime because the raw protocol's Attr carries no field for it outside darwin.
create_fileattr_test
create_readonlydir_test
# getfileinfo asserts a file created a moment ago has an access time within ten
# seconds of now, and it does not. Two causes have been fixed and neither was
# it: Utimens no longer stores a zero timestamp, and the in-memory overlays are
# dropped when an inode is released so a recycled number cannot inherit an old
# time. Still unexplained, so it stays listed rather than pretended about.
getfileinfo_test
create_readonlydir_test
setfileinfo_test
# Directory enumeration
+2 -2
View File
@@ -20,7 +20,7 @@ func TestFileHandleFullPathFallsBackAfterForget(t *testing.T) {
}
fh.RememberPath(fullPath)
wfs.inodeToPath.Forget(inode, 1, nil)
wfs.inodeToPath.Forget(inode, 1, nil, nil)
if got := fh.FullPath(); got != fullPath {
t.Fatalf("FullPath() after forget = %q, want %q", got, fullPath)
@@ -44,7 +44,7 @@ func TestFileHandleFullPathUsesSavedRenamePathAfterForget(t *testing.T) {
wfs.inodeToPath.MovePath(oldPath, newPath)
fh.RememberPath(newPath)
wfs.inodeToPath.Forget(inode, 1, nil)
wfs.inodeToPath.Forget(inode, 1, nil, nil)
if got := fh.FullPath(); got != newPath {
t.Fatalf("FullPath() after rename+forget = %q, want %q", got, newPath)
+8 -1
View File
@@ -510,7 +510,11 @@ func (i *InodeToPath) MovePath(sourcePath, targetPath util.FullPath) (sourceInod
return
}
func (i *InodeToPath) Forget(inode, nlookup uint64, onForgetDir func(dir util.FullPath)) {
// Forget drops nlookup references. onRelease, if given, runs at the moment the
// inode is released and while the table is still locked: state keyed by the
// inode number has to be dropped there, because the number is derived from the
// path and a lookup arriving after the unlock would be handed the same one.
func (i *InodeToPath) Forget(inode, nlookup uint64, onRelease func(inode uint64), onForgetDir func(dir util.FullPath)) {
var dirPaths []util.FullPath
callOnForgetDir := false
@@ -525,6 +529,9 @@ func (i *InodeToPath) Forget(inode, nlookup uint64, onForgetDir func(dir util.Fu
}
glog.V(4).Infof("kernel forget: inode %d paths %v nlookup %d", inode, path.paths, path.nlookup)
if path.nlookup == 0 {
if onRelease != nil {
onRelease(inode)
}
if _, isDir := i.dirStates[inode]; isDir && onForgetDir != nil {
dirPaths = append([]util.FullPath(nil), path.paths...)
callOnForgetDir = true
+35 -1
View File
@@ -120,7 +120,7 @@ func TestOnlyDirectoriesGetDirState(t *testing.T) {
}
// forgetting the directory drops its dirState
itp.Forget(dirInode, 1, nil)
itp.Forget(dirInode, 1, nil, nil)
if _, ok := itp.dirStates[dirInode]; ok {
t.Fatal("forgotten directory must be removed from dirStates")
}
@@ -165,3 +165,37 @@ func TestMarkChildrenCachedClearsReadThroughMode(t *testing.T) {
t.Fatal("directory should leave read-through mode after caching")
}
}
// State keyed by an inode number has to be dropped while the table is locked.
// The numbers come from the path, so a lookup racing a forget is handed the
// same one back, and a cleanup running after the unlock would wipe what that
// lookup just stored.
func TestForgetOnReleaseRunsUnderLock(t *testing.T) {
itp := NewInodeToPath(util.FullPath("/"), 60)
file := util.FullPath("/data/f.txt")
inode := itp.Lookup(file, time.Now().Unix(), false, false, 0, true)
itp.Lookup(file, time.Now().Unix(), false, false, 0, true) // nlookup is 2 now
calls := 0
onRelease := func(released uint64) {
calls++
if released != inode {
t.Errorf("released inode = %d, want %d", released, inode)
}
if itp.TryLock() {
itp.Unlock()
t.Error("onRelease ran outside the critical section")
}
}
itp.Forget(inode, 1, onRelease, nil)
if calls != 0 {
t.Fatalf("partial forget must not release: onRelease called %d times", calls)
}
itp.Forget(inode, 1, onRelease, nil)
if calls != 1 {
t.Fatalf("onRelease called %d times, want 1", calls)
}
}
+14
View File
@@ -387,6 +387,20 @@ func (wfs *WFS) setAtime(inode uint64, t time.Time) {
}
// applyInMemoryAtime overlays the in-memory atime onto a fuse.Attr if present.
// forgetInMemoryTimes drops the overlays for an inode. Both maps are keyed by
// inode and inodes are derived from the path, so a delete and recreate can
// hand the same number to a different file — which would then inherit the
// previous one's access or modification time.
func (wfs *WFS) forgetInMemoryTimes(inode uint64) {
wfs.atimeMu.Lock()
delete(wfs.atimeMap, inode)
wfs.atimeMu.Unlock()
wfs.dirMtimeMu.Lock()
delete(wfs.dirMtimeMap, inode)
wfs.dirMtimeMu.Unlock()
}
func (wfs *WFS) applyInMemoryAtime(out *fuse.Attr, inode uint64) {
wfs.atimeMu.Lock()
if t, ok := wfs.atimeMap[inode]; ok {
+10 -6
View File
@@ -65,10 +65,14 @@ func (wfs *WFS) Forget(nodeid, nlookup uint64) {
// lifecycle is driven independently by FUSE Open/Release — touching the
// fhMap here would couple two unrelated refcounts and could tear down a
// still-live handle if Forget ever raced ahead of Release.
wfs.inodeToPath.Forget(nodeid, nlookup, func(dir util.FullPath) {
// Runs after Forget releases its lock; a concurrent lookup+rebuild can
// re-cache the directory in that window, so purge through the apply loop
// rather than wiping the store directly.
wfs.purgeDirectoryCache(dir)
})
wfs.inodeToPath.Forget(nodeid, nlookup,
// Runs at the release, under the table's lock, so a lookup that
// rebuilds the same inode number afterwards keeps the times it sets.
wfs.forgetInMemoryTimes,
func(dir util.FullPath) {
// Runs after Forget releases its lock; a concurrent lookup+rebuild can
// re-cache the directory in that window, so purge through the apply loop
// rather than wiping the store directly.
wfs.purgeDirectoryCache(dir)
})
}
+21 -4
View File
@@ -231,9 +231,16 @@ func (w *WinFS) attrToStat(attr *fuse.Attr, stat *cgofuse.Stat_t) {
stat.Atim = cgofuse.Timespec{Sec: int64(attr.Atime), Nsec: int64(attr.Atimensec)}
stat.Mtim = cgofuse.Timespec{Sec: int64(attr.Mtime), Nsec: int64(attr.Mtimensec)}
stat.Ctim = cgofuse.Timespec{Sec: int64(attr.Ctime), Nsec: int64(attr.Ctimensec)}
// Windows shows a creation time and has nothing to derive it from; ctime
// is the closest the filer tracks.
// The filer records a creation time, but the raw protocol's Attr carries no
// field for it outside darwin, so ctime stands in until there is a way to
// read it through.
stat.Birthtim = stat.Ctim
// Windows expects a regular file to be marked archived; with no flags at
// all it synthesises NORMAL, which is a different thing and what
// create_fileattr_test checks.
if attr.Mode&cgofuse.S_IFMT == cgofuse.S_IFREG {
stat.Flags |= cgofuse.UF_ARCHIVE
}
}
// translateOpenFlags converts cgofuse's open flags, which follow MSVC's
@@ -534,9 +541,19 @@ func (w *WinFS) Utimens(path string, tmsp []cgofuse.Timespec) int {
// applyTimespec reports whether a timestamp should be written. UTIME_OMIT asks
// for the existing value to be kept, and a time at or below Windows' own 1601
// epoch arrives as a large negative second count that would be stored verbatim.
// epoch arrives as a large negative second count. Exactly zero is what Windows
// sends for a field it is not setting; storing it put 1970 in the atime
// overlay, which then read back as the file's access time. A date genuinely
// before 1970 is still allowed through — only the epoch itself is the
// sentinel.
func applyTimespec(ts cgofuse.Timespec) bool {
return ts.Nsec != utimeOmit && ts.Sec > windowsEpochCutoff
if ts.Nsec == utimeOmit {
return false
}
if ts.Sec == 0 && ts.Nsec == 0 {
return false
}
return ts.Sec > windowsEpochCutoff
}
func (w *WinFS) Flush(path string, fh uint64) int {