| Age | Commit message (Collapse) | Author |
|---|
|
Update the following from syserror to the linuxerr equivalent:
EEXIST
EFAULT
ENOTDIR
ENOTTY
EOPNOTSUPP
ERANGE
ESRCH
PiperOrigin-RevId: 384329869
|
|
This change makes the checklocks analyzer considerable more powerful, adding:
* The ability to traverse complex structures, e.g. to have multiple nested
fields as part of the annotation.
* The ability to resolve simple anonymous functions and closures, and perform
lock analysis across these invocations. This does not apply to closures that
are passed elsewhere, since it is not possible to know the context in which
they might be invoked.
* The ability to annotate return values in addition to receivers and other
parameters, with the same complex structures noted above.
* Ignoring locking semantics for "fresh" objects, i.e. objects that are
allocated in the local frame (typically a new-style function).
* Sanity checking of locking state across block transitions and returns, to
ensure that no unexpected locks are held.
Note that initially, most of these findings are excluded by a comprehensive
nogo.yaml. The findings that are included are fundamental lock violations.
The changes here should be relatively low risk, minor refactorings to either
include necessary annotations to simplify the code structure (in general
removing closures in favor of methods) so that the analyzer can be easily
track the lock state.
This change additional includes two changes to nogo itself:
* Sanity checking of all types to ensure that the binary and ast-derived
types have a consistent objectpath, to prevent the bug above from occurring
silently (and causing much confusion). This also requires a trick in
order to ensure that serialized facts are consumable downstream. This can
be removed with https://go-review.googlesource.com/c/tools/+/331789 merged.
* A minor refactoring to isolation the objdump settings in its own package.
This was originally used to implement the sanity check above, but this
information is now being passed another way. The minor refactor is preserved
however, since it cleans up the code slightly and is minimal risk.
PiperOrigin-RevId: 382613300
|
|
Update/remove most syserror errors to linuxerr equivalents. For list
of removed errors, see //pkg/syserror/syserror.go.
PiperOrigin-RevId: 382574582
|
|
Update all instances of the above errors to the faster linuxerr implementation.
With the temporary linuxerr.Equals(), no logical changes are made.
PiperOrigin-RevId: 382306655
|
|
Remove three syserror entries duplicated in linuxerr. Because of the
linuxerr.Equals method, this is a mere change of return values from
syserror to linuxerr definitions.
Done with only these three errnos as CLs removing all grow to a significantly
large size.
PiperOrigin-RevId: 382173835
|
|
PiperOrigin-RevId: 381982257
|
|
Add Equals method to compare syserror and unix.Errno errors to linuxerr errors.
This will facilitate removal of syserror definitions in a followup, and
finding needed conversions from unix.Errno to linuxerr.
PiperOrigin-RevId: 380909667
|
|
- Allow the gofer client to use most xattr namespaces. As documented by the
updated comment, this is consistent with e.g. Linux's FUSE client, and allows
gofers to provide extended attributes from FUSE filesystems.
- Make tmpfs' listxattr omit xattrs in the "trusted" namespace for
non-privileged users.
PiperOrigin-RevId: 378778854
|
|
PiperOrigin-RevId: 377966969
|
|
This metric is replaced by /cloud/gvisor/sandbox/sentry/suspicious_operations
metric with field value opened_write_execute_file.
PiperOrigin-RevId: 374509823
|
|
The new metric contains fields and will replace the below existing metric:
- opened_write_execute_file
PiperOrigin-RevId: 373884604
|
|
O_PATH is now implemented in vfs2.
Fixes #2782.
PiperOrigin-RevId: 373861410
|
|
When a child is added to a parent (directory) dentry, both child and parent are
queued for checkCachingLocked(). Make sure that the parent is queued first
because the parent gained a ref and so could be removed from the LRU cache hence
making space for the new child. This could prevent an LRU cache eviction.
In practice, this did seem to help. ~800 RPCs were reduced while building
//absl/... (ABSL build benchmark). Evictions hurt in 2 ways - create renameMu
contention and destroy a possibly useful dentry which will have to be re-walked
and re-opened later.
Follow up fix for #5859.
PiperOrigin-RevId: 371509392
|
|
Originally we were making a WalkGetAttrOne RPC to confirm that a file does not
exist on the remote filesystem - when there was no cached information about the
existence of a dentry at that position.
This change avoids making that RPC and speculatively makes the
mkdir/mknod/linkat/symlink RPC. They will fail with EEXIST if a file exists at
that position as we want.
However the error ordering is important. Existence check comes before
writability check. So we make the existence check when the writability check
fails and give it precedence.
This change saves ~76,000 RPCs while building //absl/... (ABSL build benchmark).
That is 10% of all RPCs made while running that workload.
PiperOrigin-RevId: 371225633
|
|
PiperOrigin-RevId: 371015541
|
|
PiperOrigin-RevId: 369686285
|
|
The gofer client's LRU cache has a default limit of 1000 dentries. Any attempt
to cache more dentries than that will make the LRU cache evict and destroy the
least recently used dentry. However, the eviction is expensive because it
requires holding fs.renameMu for writing - which in turn creates a lot of
contention. All filesystem operations that involve path traversal require
fs.renameMu for reading atleast.
Therefore, it is in our best interest to keep the cache small and clean.
When a dentry is inserted in the dentry tree, it grabs a ref on its parent for
its entire lifetime. Hence the parent is longer evictable (because refs > 0).
This change additionally calls checkCachingLocked on directories that have been
added to so that they can be removed from the LRU cache if needed.
This change implies that the LRU cache will only contain the leaves from the
filesystem tree which significantly reduces the LRU cache size and consequently
reduces the number of expensive LRU cache evictions.
> Why are opened dentries not removed from LRU cache?
When a file description is open(2)-ed, the file description holds a ref on its
dentry for its entire lifetime. However, calling checkCachingLocked() on opened
dentries actually ends up hurting performance. Applications usually open file
descriptors for a short duration. So upon close(2), the dentry is reinserted
into the cache anyway. So the precautionary work done in removing the opened
dentry from the cache went for waste as it did not really reduce an eviction.
Local benchmarking has shown that this change improves performance by 3-4%.
Across 6 runs, without this change it took 296.127 seconds to build runsc while
with this change it took only 285.136 seconds.
PiperOrigin-RevId: 369510494
|
|
Remote revalidating requires to update file size on every write
on a file opened with O_APPEND. If host FD exists, it can be
used to update the size and skip round trip to the gofer. With
this change, O_APPEND writes with remote revalidating is almost
as fast as exclusive mode:
BM_Append
VFS1 60.7us
VFS2 56.8us
VFS2 exclusive 14.2us
This change 15.8us
Updates #1792
PiperOrigin-RevId: 369486801
|
|
While using remote-validation, the vast majority of time spent during
FS operations is re-walking the path to check for modifications and
then closing the file given that in most cases it has not been
modified externally.
This change introduces a new 9P message called MultiGetAttr which bulks
query attributes of several files in one shot. The returned attributes are
then used to update cached dentries before they are walked. File attributes
are updated for files that still exist. Dentries that have been deleted are
removed from the cache. And negative cache entries are removed if a new
file/directory was created externally. Similarly, synthetic dentries are
replaced if a file/directory is created externally.
The bulk update needs to be carefull not to follow symlinks, cross mount
points, because the gofer doesn't know how to resolve symlinks and where
mounts points are located. It also doesn't walk to the parent ("..") to
avoid deadlocks.
Here are the results:
Workload VFS1 VFS2 Change
bazel action 115s 70s 28.8s
Stat/100 11,043us 7,623us 974us
Updates #1638
PiperOrigin-RevId: 369325957
|
|
Runsc build benchmark's mutex profile shows that we are wasting roughly 25-30
seconds waiting for filesystem.renameMu to get unlocked. Earlier
checkCachingLocked required the renameMu to be locked for writing. This is a
filesystem wide lock which puts all other filesystem operations on hold and
hence is really expensive. Something to note is that all path resolution
operations hold renameMu for reading.
With this change, we allow to check for caching without even holding renameMu.
This change introduces more fine grained locks (fs.cacheMu and dentry.cachingMu)
which protect the cache (removing the requirement to hold renameMu for writing
to modify the cache) and synchronize concurrent dentry caching attempts on a per
dentry basis. We still require to hold renameMu for writing while destroying
dentries and evicting from the cache but this still significantly reduces the
write locking critical section.
Local benchmarking showed that this improved runsc build benchmark time by 4-5%.
Across 6 runs, without this change it took 310.9025 seconds to build runsc
while with this change it took 296.127 seconds.
Runsc build benchmark's mutex profile: https://gvisor.dev/profile/gvisor-buildkite/78a3f968-36ca-4944-93f7-77a8792d56b4/28a1d260-790b-4a9e-94da-a4daede08ee3/tmp/profile/ptrace/BenchmarkBuildRunsc/page_cache.clean/filesystem.bindfs/benchmarks/runsc/mutex.pprof/flamegraph
PiperOrigin-RevId: 368958136
|
|
Without this change, we ask the gofer server to update the permissions
whenever the UID, GID or size is updated via SetStat. Consequently, we don not
generate inotify events when the permissions actually change due to SGID bit
getting cleared.
With this change, we will update the permissions only when needed and generate
inotify events.
PiperOrigin-RevId: 366946842
|
|
PiperOrigin-RevId: 366462448
|
|
Split usermem package to help remove syserror dependency in go_marshal.
New hostarch package contains code not dependent on syserror.
PiperOrigin-RevId: 365651233
|
|
Also adds support for clearing the setuid bit when appropriate (writing,
truncating, changing size, changing UID, or changing GID).
VFS2 only.
PiperOrigin-RevId: 364661835
|
|
If there was a partial write (when not using the host FD) which did not generate
an error, we were incorrectly returning the number of bytes attempted to write
instead of the number of bytes actually written.
PiperOrigin-RevId: 363058989
|
|
PiperOrigin-RevId: 362406813
|
|
The syscall package has been deprecated in favor of golang.org/x/sys.
Note that syscall is still used in the following places:
- pkg/sentry/socket/hostinet/stack.go: some netlink related functionalities
are not yet available in golang.org/x/sys.
- syscall.Stat_t is still used in some places because os.FileInfo.Sys() still
returns it and not unix.Stat_t.
Updates #214
PiperOrigin-RevId: 360701387
|
|
Our implementation of vfs.CheckDeleteSticky was not consistent with Linux,
specifically not consistent with fs/linux.h:check_sticky().
One of the biggest differences was that the vfs implementation did not
allow the owner of the sticky directory to delete files inside it that belonged
to other users.
This change makes our implementation consistent with Linux.
Also adds an integration test to check for this. This bug is also present in
VFS1.
Updates #3027
PiperOrigin-RevId: 355557425
|
|
When file is regular and metadata cache is authoritative, metadata lock
is taken. The code deadlocks trying to acquire the metadata lock
again to update time stampts.
PiperOrigin-RevId: 354584594
|
|
Contrary to the comment on the socket test, the failure was due to an issue
with goferfs rather than kernfs.
PiperOrigin-RevId: 353918021
|
|
Fixes #5113.
PiperOrigin-RevId: 353313374
|
|
PiperOrigin-RevId: 352904728
|
|
Fixes #5263
PiperOrigin-RevId: 352903844
|
|
Return EEXIST when overwritting a file as long as the caller has exec
permission on the parent directory, even if the caller doesn't have
write permission.
Also reordered the mount write check, which happens before permission
is checked.
Closes #5164
PiperOrigin-RevId: 351868123
|
|
PiperOrigin-RevId: 343959348
|
|
PiperOrigin-RevId: 343196927
|
|
PiperOrigin-RevId: 342161204
|
|
This lets us avoid treating a value of 0 as one reference. All references
using the refsvfs2 template must call InitRefs() before the reference is
incremented/decremented, or else a panic will occur. Therefore, it should be
pretty easy to identify missing InitRef calls during testing.
Updates #1486.
PiperOrigin-RevId: 341411151
|
|
This is necessary to allow writes to files opened with O_WRONLY to go through
host FDs.
PiperOrigin-RevId: 341174509
|
|
This is consistent with what Linux does. This was causing a PHP runtime test
failure. Fixed it for VFS2.
PiperOrigin-RevId: 341155209
|
|
The default pipe size already matched linux, and is unchanged.
Furthermore `atomicIOBytes` is made a proper constant (as it is in Linux). We
were plumbing usermem.PageSize everywhere, so this is no functional change.
PiperOrigin-RevId: 340497006
|
|
Also refactor the template and CheckedObject interface to make this cleaner.
Updates #1486.
PiperOrigin-RevId: 339577120
|
|
Much like the VFS2 gofer client, kernfs too now caches dentries. The size of the
LRU cache is configurable via mount options.
Have adopted the same reference semantics from gofer client dentry.
Only sysfs and procfs use this LRU cache. The rest of the kernfs users (devpts,
fusefs, host, pipefs, sockfs) still use the no cache approach.
PiperOrigin-RevId: 339139835
|
|
Updates #1486.
PiperOrigin-RevId: 338832085
|
|
Inode number consistency checks are now skipped in save/restore tests for
reasons described in greatest detail in StatTest.StateDoesntChangeAfterRename.
They pass in VFS1 due to the bug described in new test case
SimpleStatTest.DifferentFilesHaveDifferentDeviceInodeNumberPairs.
Fixes #1663
PiperOrigin-RevId: 338776148
|
|
Added the following fields in kernfs.InodeAttr:
- blockSize
- atime
- mtime
- ctime
Also resolved all TODOs for #1193.
Fixes #1193
PiperOrigin-RevId: 338714527
|
|
The sentry page cache stores file contents at page granularity; this is
necessary for memory mappings. Thus file offset ranges passed to
fsutil.FileRangeSet.Fill() must be page-aligned. If the read callback passed to
Fill() returns (partial read, nil error) when reading up to EOF (which is the
case for p9.ClientFile.ReadAt() since 9P's Rread cannot convey both a partial
read and EOF), Fill() will re-invoke the read callback to try to read from EOF
to the end of the containing page, which is harmless but needlessly expensive.
Fix this by handling file size explicitly in fsutil.FileRangeSet.Fill().
PiperOrigin-RevId: 336934075
|
|
This fixes reference leaks related to accidentally forgetting to DecRef()
after calling one or the other.
PiperOrigin-RevId: 336918922
|
|
Singleton filesystem like devpts and devtmpfs have a single filesystem shared
among all mounts, so they acquire a "self-reference" when initialized that
must be released when the entire virtual filesystem is released at sandbox
exit.
PiperOrigin-RevId: 336828852
|
|
Fixes #1479, #317.
PiperOrigin-RevId: 334258052
|
