From 221ed0373a2df26de8bd14f829b47c5790de8d41 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Thu, 29 Jun 2023 21:00:09 +0800 Subject: [PATCH] fs: stops pre-fetching the inode of dot-dot ("..") (#1544) Signed-off-by: Adrian Cole --- RATIONALE.md | 27 +++++++++++++++++++++++ imports/wasi_snapshot_preview1/fs_test.go | 12 +++++----- internal/sys/fs.go | 12 ++-------- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/RATIONALE.md b/RATIONALE.md index dedcc3c8..27cc39f4 100644 --- a/RATIONALE.md +++ b/RATIONALE.md @@ -745,6 +745,33 @@ yet these are returned for `wasi_snapshot_preview1.fd_readdir`. This was a change specifically made to pass wasi-testsuite, and has problems known since late 2019. +### Why don't we pre-populate an inode for the dot-dot ("..") entry? + +We only populate an inode for dot (".") because wasi-testsuite requires it, and +we likely already have it (because we cache it). We could attempt to populate +one for dot-dot (".."), but chose not to. + +Firstly, wasi-testsuite does not require the inode of dot-dot, possibly because +the wasip2 adapter doesn't populate it (but we don't really know why). + +The only other reason to populate it would be to avoid wasi-libc's stat fanout +when it is missing. However, the inode for dot-dot is not cached, and is also +likely not possible to get on a pseudo file. Even if we could get it, we would +have to use stat. In other words, pre-populating this would have the same cost +as waiting for something to call stat instead. + +Fetching dot-dot's inode despite the above not only doesn't help wasi-libc, but +it also hurts languages that don't use it, such as Go. These languages would +pay a stat syscall penalty even if they don't need the inode. In fact, Go +discards both dot entries! + +In summary, there are no significant upsides in attempting to pre-fetch +dot-dot's inode, and there are downsides to doing it anyway. + +See https://github.com/WebAssembly/wasi-libc/pull/345 +See https://github.com/WebAssembly/wasi-testsuite/blob/main/tests/rust/src/bin/fd_readdir.rs#L108 +See https://github.com/bytecodealliance/preview2-prototyping/blob/e4c04bcfbd11c42c27c28984948d501a3e168121/crates/wasi-preview1-component-adapter/src/lib.rs#L1037 + ## Why does "wasi_snapshot_preview1" require dot entries when POSIX does not? In October 2019, WASI project knew requiring dot entries ("." and "..") was not diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go index 13d44d65..c9d7c548 100644 --- a/imports/wasi_snapshot_preview1/fs_test.go +++ b/imports/wasi_snapshot_preview1/fs_test.go @@ -4919,8 +4919,8 @@ func requireOpenFile(t *testing.T, tmpDir string, pathName string, data []byte, return mod, fd, log, r } -// Test_fdReaddir_dotEntriesHaveRealInodes because wasi-testsuite requires it. -func Test_fdReaddir_dotEntriesHaveRealInodes(t *testing.T) { +// Test_fdReaddir_dotEntryHasARealInode because wasi-testsuite requires it. +func Test_fdReaddir_dotEntryHasARealInode(t *testing.T) { if runtime.GOOS == "windows" && !platform.IsGo120 { t.Skip("windows before go 1.20 has trouble reading the inode information on directories.") } @@ -4954,11 +4954,10 @@ func Test_fdReaddir_dotEntriesHaveRealInodes(t *testing.T) { dirents = append(dirents, 3, 0, 0, 0) // d_type = directory dirents = append(dirents, '.') // name - // get the real inode of the parent directory - st, errno = preopen.Stat(".") require.EqualErrno(t, 0, errno) dirents = append(dirents, 2, 0, 0, 0, 0, 0, 0, 0) // d_next = 2 - dirents = append(dirents, u64.LeBytes(st.Ino)...) // d_ino + // See /RATIONALE.md for why we don't attempt to get an inode for ".." + dirents = append(dirents, 0, 0, 0, 0, 0, 0, 0, 0) // d_ino dirents = append(dirents, 2, 0, 0, 0) // d_namlen = 2 characters dirents = append(dirents, 3, 0, 0, 0) // d_type = directory dirents = append(dirents, '.', '.') // name @@ -5021,7 +5020,8 @@ func Test_fdReaddir_opened_file_written(t *testing.T) { st, errno = preopen.Stat(".") require.EqualErrno(t, 0, errno) dirents = append(dirents, 2, 0, 0, 0, 0, 0, 0, 0) // d_next = 2 - dirents = append(dirents, u64.LeBytes(st.Ino)...) // d_ino + // See /RATIONALE.md for why we don't attempt to get an inode for ".." + dirents = append(dirents, 0, 0, 0, 0, 0, 0, 0, 0) // d_ino dirents = append(dirents, 2, 0, 0, 0) // d_namlen = 2 characters dirents = append(dirents, 3, 0, 0, 0) // d_type = directory dirents = append(dirents, '.', '.') // name diff --git a/internal/sys/fs.go b/internal/sys/fs.go index 52be57bc..760de604 100644 --- a/internal/sys/fs.go +++ b/internal/sys/fs.go @@ -4,7 +4,6 @@ import ( "io" "io/fs" "net" - "path" "syscall" "github.com/tetratelabs/wazero/internal/descriptor" @@ -160,15 +159,8 @@ func synthesizeDotEntries(f *FileEntry) (result []fsapi.Dirent, errno syscall.Er return nil, errno } result = append(result, fsapi.Dirent{Name: ".", Ino: dotIno, Type: fs.ModeDir}) - dotDotIno := uint64(0) - if !f.IsPreopen && f.Name != "." { - if st, errno := f.FS.Stat(path.Dir(f.Name)); errno != 0 { - return nil, errno - } else { - dotDotIno = st.Ino - } - } - result = append(result, fsapi.Dirent{Name: "..", Ino: dotDotIno, Type: fs.ModeDir}) + // See /RATIONALE.md for why we don't attempt to get an inode for ".." + result = append(result, fsapi.Dirent{Name: "..", Ino: 0, Type: fs.ModeDir}) return result, 0 }