From: David Howells <dhowells@redhat.com> Date: 2020-08-03 15:28:02
Hi Linus,
Here's a set of patches to add notifications for mount topology events,
such as mounting, unmounting, mount expiry, mount reconfiguration.
The first patch in the series adds a hard limit on the number of watches
that any particular user can add. The RLIMIT_NOFILE value for the process
adding a watch is used as the limit. Even if you don't take the rest of
the series, can you at least take this one?
An LSM hook is included for an LSM to rule on whether or not a mount watch
may be set on a particular path.
This series is intended to be taken in conjunction with the fsinfo series
which I'll post a pull request for shortly and which is dependent on it.
Karel Zak[*] has created preliminary patches that add support to libmount
and Ian Kent has started working on making systemd use them.
[*] https://github.com/karelzak/util-linux/commits/topic/fsinfo
Note that there have been some last minute changes to the patchset: you
wanted something adding and Miklós wanted some bits taking out/changing.
I've placed a tag, fsinfo-core-20200724 on the aggregate of these two
patchsets that can be compared to fsinfo-core-20200803.
To summarise the changes: I added the limiter that you wanted; removed an
unused symbol; made the mount ID fields in the notificaion 64-bit (the
fsinfo patchset has a change to convey the mount uniquifier instead of the
mount ID); removed the event counters from the mount notification and moved
the event counters into the fsinfo patchset.
====
WHY?
====
Why do we want mount notifications? Whilst /proc/mounts can be polled, it
only tells you that something changed in your namespace. To find out, you
have to trawl /proc/mounts or similar to work out what changed in the mount
object attributes and mount topology. I'm told that the proc file holding
the namespace_sem is a point of contention, especially as the process of
generating the text descriptions of the mounts/superblocks can be quite
involved.
The notification generated here directly indicates the mounts involved in
any particular event and gives an idea of what the change was.
This is combined with a new fsinfo() system call that allows, amongst other
things, the ability to retrieve in one go an { id, change_counter } tuple
from all the children of a specified mount, allowing buffer overruns to be
dealt with quickly.
This is of use to systemd to improve efficiency:
https://lore.kernel.org/linux-fsdevel/20200227151421.3u74ijhqt6ekbiss@ws.net.home/
And it's not just Red Hat that's potentially interested in this:
https://lore.kernel.org/linux-fsdevel/293c9bd3-f530-d75e-c353-ddeabac27cf6@6wind.com/
David
---
The following changes since commit ba47d845d715a010f7b51f6f89bae32845e6acb7:
Linux 5.8-rc6 (2020-07-19 15:41:18 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/mount-notifications-20200803
for you to fetch changes up to 841a0dfa511364fa9a8d67512e0643669f1f03e3:
watch_queue: sample: Display mount tree change notifications (2020-08-03 12:15:38 +0100)
----------------------------------------------------------------
Mount notifications
----------------------------------------------------------------
David Howells (5):
watch_queue: Limit the number of watches a user can hold
watch_queue: Make watch_sizeof() check record size
watch_queue: Add security hooks to rule on setting mount watches
watch_queue: Implement mount topology and attribute change notifications
watch_queue: sample: Display mount tree change notifications
Documentation/watch_queue.rst | 12 +-
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
fs/Kconfig | 9 ++
fs/Makefile | 1 +
fs/mount.h | 18 +++
fs/mount_notify.c | 222 ++++++++++++++++++++++++++++
fs/namespace.c | 22 +++
include/linux/dcache.h | 1 +
include/linux/lsm_hook_defs.h | 3 +
include/linux/lsm_hooks.h | 6 +
include/linux/sched/user.h | 3 +
include/linux/security.h | 8 +
include/linux/syscalls.h | 2 +
include/linux/watch_queue.h | 7 +-
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/watch_queue.h | 31 +++-
kernel/sys_ni.c | 3 +
kernel/watch_queue.c | 8 +
samples/watch_queue/watch_test.c | 41 ++++-
security/security.c | 7 +
37 files changed, 422 insertions(+), 6 deletions(-)
create mode 100644 fs/mount_notify.c
From: David Howells <dhowells@redhat.com> Date: 2020-08-03 15:50:20
Hi Linus,
Here's a set of patches that adds a system call, fsinfo(), that allows
information about the VFS, mount topology, superblock and files to be
retrieved.
The patchset is based on top of the mount notifications patchset so that
the mount notification mechanism can be hooked to provide event counters
that can be retrieved with fsinfo(), thereby making it a lot faster to work
out which mounts have changed.
Note that there was a last minute change requested by Miklós: the event
counter bits got moved from the mount notification patchset to this one.
The counters got made atomic_long_t inside the kernel and __u64 in the
UAPI. The aggregate changes can be assessed by comparing pre-change tag,
fsinfo-core-20200724 to the requested pull tag.
Karel Zak has created preliminary patches that add support to libmount[*]
and Ian Kent has started working on making systemd use these and mount
notifications[**].
[*] https://github.com/karelzak/util-linux/commits/topic/fsinfo
[**] https://lists.freedesktop.org/archives/systemd-devel/2020-July/044914.html
=======
THE WHY
=======
Why do we want this?
Using /proc/mounts (or similar) has problems:
(1) Reading from it holds a global lock (namespace_sem) that prevents
mounting and unmounting. Lots of data is encoded and mangled into
text whilst the lock is held, including superblock option strings and
mount point paths. This causes performance problems when there are a
lot of mount objects in a system.
(2) Even though namespace_sem is held during a read, reading the whole
file isn't necessarily atomic with respect to mount-type operations.
If a read isn't satisfied in one go, then it may return to userspace
briefly and then continue reading some way into the file. But changes
can occur in the interval that may then go unseen.
(3) Determining what has changed means parsing and comparing consecutive
outputs of /proc/mounts.
(4) Querying a specific mount or superblock means searching through
/proc/mounts and searching by path or mount ID - but we might have an
fd we want to query.
(5) Whilst you can poll() it for events, it only tells you that something
changed in the namespace, not what or whether you can even see the
change.
To fix the notification issues, the preceding notifications patchset added
mount watch notifications whereby you can watch for notifications in a
specific mount subtree. The notification messages include the ID(s) of the
affected mounts.
To support notifications, however, we need to be able to handle overruns in
the notification queue. I added a number of event counters to struct
super_block and struct mount to allow you to pin down the changes, but
there needs to be a way to retrieve them. Exposing them through /proc
would require adding yet another /proc/mounts-type file. We could add
per-mount directories full of attributes in sysfs, but that has issues also
(see below).
Adding an extensible system call interface for retrieving filesystem
information also allows other things to be exposed:
(1) Jeff Layton's error handling changes need a way to allow error event
information to be retrieved.
(2) Bits in masks returned by things like statx() and FS_IOC_GETFLAGS are
actually 3-state { Set, Unset, Not supported }. It could be useful to
provide a way to expose information like this[*].
(3) Limits of the numerical metadata values in a filesystem[*].
(4) Filesystem capability information[*]. Filesystems don't all have the
same capabilities, and even different instances may have different
capabilities, particularly with network filesystems where the set of
may be server-dependent. Capabilities might even vary at file
granularity - though possibly such information should be conveyed
through statx() instead.
(5) ID mapping/shifting tables in use for a superblock.
(6) Filesystem-specific information. I need something for AFS so that I
can do pioctl()-emulation, thereby allowing me to implement certain of
the AFS command line utilities that query state of a particular file.
This could also have application for other filesystems, such as NFS,
CIFS and ext4.
[*] In a lot of cases these are probably invariant and can be memcpy'd
from static data.
There's a further consideration: I want to make it possible to have
fsconfig(fd, FSCONFIG_CMD_CREATE) be intercepted by a container manager
such that the manager can supervise a mount attempted inside the container.
The manager would be given an fd pointing to the fs_context struct and
would then need some way to query it (fsinfo()) and modify it (fsconfig()).
This could also be used to arbitrate user-requested mounts when containers
are not in play.
================
DESIGN DECISIONS
================
(1) Information is partitioned into sets of attributes.
(2) Attribute IDs are integers as they're fast to compare.
(3) Attribute values are typed (struct, list of structs, string, opaque
blob). They type is fixed for a particular attribute.
(4) For structure types, the length is also a version. New fields can be
tacked onto the end.
(5) When copying a versioned struct to userspace, the core handles a
version mismatch by truncating or zero-padding the data as necessary.
This is transparent to the filesystem.
(6) The core handles all the buffering and buffer resizing.
(7) The filesystem never gets any access to the userspace parameter buffer
or result buffer.
(8) "Meta" attributes can describe other attributes.
========
OVERVIEW
========
fsinfo() is a system call that allows information about the filesystem at a
particular path point to be queried as a set of attributes.
Attribute values are of four basic types:
(1) Structure with version-dependent length (the length is the version).
(2) Variable-length string.
(3) List of structures (all the same length).
(4) Opaque blob.
Attributes can have multiple values either as a sequence of values or a
sequence-of-sequences of values and all the values of a particular
attribute must be of the same type. Values can be up to INT_MAX size,
subject to memory availability.
Note that the values of an attribute *are* allowed to vary between dentries
within a single superblock, depending on the specific dentry that you're
looking at, but the values still have to be of the type for that attribute.
I've tried to make the interface as light as possible, so integer attribute
IDs rather than string and the core does all the buffer allocation and
expansion and all the extensibility support work rather than leaving that
to the filesystems. This also means that userspace pointers are not
exposed to the filesystem.
fsinfo() allows a variety of information to be retrieved about a filesystem
and the mount topology:
(1) General superblock attributes:
- Filesystem identifiers (UUID, volume label, device numbers, ...)
- The limits on a filesystem's capabilities
- Information on supported statx fields and attributes and IOC flags.
- A variety single-bit flags indicating supported capabilities.
- Timestamp resolution and range.
- The amount of space/free space in a filesystem (as statfs()).
- Superblock notification counter.
(2) Filesystem-specific superblock attributes:
- Superblock-level timestamps.
- Cell name, workgroup or other netfs grouping concept.
- Server names and addresses.
(3) VFS information:
- Mount topology information.
- Mount attributes.
- Mount notification counter.
- Mount point path.
(4) Information about what the fsinfo() syscall itself supports, including
the type and struct size of attributes.
The system is extensible:
(1) New attributes can be added. There is no requirement that a
filesystem implement every attribute. A helper function is provided
to scan a list of attributes and a filesystem can have multiple such
lists.
(2) Version length-dependent structure attributes can be made larger and
have additional information tacked on the end, provided it keeps the
layout of the existing fields. If an older process asks for a shorter
structure, it will only be given the bits it asks for. If a newer
process asks for a longer structure on an older kernel, the extra
space will be set to 0. In all cases, the size of the data actually
available is returned.
In essence, the size of a structure is that structure's version: a
smaller size is an earlier version and a later version includes
everything that the earlier version did.
(3) New single-bit capability flags can be added. This is a structure-typed
attribute and, as such, (2) applies. Any bits you wanted but the kernel
doesn't support are automatically set to 0.
fsinfo() may be called like the following, for example:
struct fsinfo_params params = {
.at_flags = AT_SYMLINK_NOFOLLOW,
.flags = FSINFO_FLAGS_QUERY_PATH,
.request = FSINFO_ATTR_AFS_SERVER_ADDRESSES,
.Nth = 2,
};
struct fsinfo_server_address address;
len = fsinfo(AT_FDCWD, "/afs/grand.central.org/doc",
¶ms, sizeof(params),
&address, sizeof(address));
The above example would query an AFS filesystem to retrieve the address
list for the 3rd server, and:
struct fsinfo_params params = {
.at_flags = AT_SYMLINK_NOFOLLOW,
.flags = FSINFO_FLAGS_QUERY_PATH,
.request = FSINFO_ATTR_NFS_SERVER_NAME;
};
char server_name[256];
len = fsinfo(AT_FDCWD, "/home/dhowells/",
¶ms, sizeof(params),
&server_name, sizeof(server_name));
would retrieve the name of the NFS server as a string.
In future, I want to make fsinfo() capable of querying a context created by
fsopen() or fspick(), e.g.:
fd = fsopen("ext4", 0);
struct fsinfo_params params = {
.flags = FSINFO_FLAGS_QUERY_FSCONTEXT,
.request = FSINFO_ATTR_CONFIGURATION;
};
char buffer[65536];
fsinfo(fd, NULL, ¶ms, sizeof(params), &buffer, sizeof(buffer));
even if that context doesn't currently have a superblock attached.
David
---
The following changes since commit 841a0dfa511364fa9a8d67512e0643669f1f03e3:
watch_queue: sample: Display mount tree change notifications (2020-08-03 12:15:38 +0100)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/fsinfo-core-20200803
for you to fetch changes up to 19d687f5902b65a158c5bec904d65d0525ea4efe:
samples: add error state information to test-fsinfo.c (2020-08-03 13:43:46 +0100)
----------------------------------------------------------------
Filesystem information
----------------------------------------------------------------
David Howells (15):
fsinfo: Introduce a non-repeating system-unique superblock ID
fsinfo: Add fsinfo() syscall to query filesystem information
fsinfo: Provide a bitmap of the features a filesystem supports
fsinfo: Allow retrieval of superblock devname, options and stats
fsinfo: Allow fsinfo() to look up a mount object by ID
fsinfo: Add a uniquifier ID to struct mount
fsinfo: Allow mount information to be queried
fsinfo: Allow mount topology and propagation info to be retrieved
watch_queue: Mount event counters
fsinfo: Provide notification overrun handling support
fsinfo: sample: Mount listing program
fsinfo: Add API documentation
fsinfo: Add support for AFS
fsinfo: Add support to ext4
fsinfo: Add an attribute that lists all the visible mounts in a namespace
Jeff Layton (3):
errseq: add a new errseq_scrape function
vfs: allow fsinfo to fetch the current state of s_wb_err
samples: add error state information to test-fsinfo.c
Documentation/filesystems/fsinfo.rst | 574 ++++++++++++++++++
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
fs/Kconfig | 7 +
fs/Makefile | 1 +
fs/afs/internal.h | 1 +
fs/afs/super.c | 216 ++++++-
fs/d_path.c | 2 +-
fs/ext4/Makefile | 1 +
fs/ext4/ext4.h | 6 +
fs/ext4/fsinfo.c | 97 +++
fs/ext4/super.c | 3 +
fs/fsinfo.c | 748 +++++++++++++++++++++++
fs/internal.h | 16 +
fs/mount.h | 6 +
fs/mount_notify.c | 10 +-
fs/namespace.c | 427 +++++++++++++-
fs/super.c | 2 +
include/linux/errseq.h | 1 +
include/linux/fs.h | 7 +
include/linux/fsinfo.h | 112 ++++
include/linux/syscalls.h | 4 +
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/fsinfo.h | 344 +++++++++++
include/uapi/linux/mount.h | 13 +-
kernel/sys_ni.c | 1 +
lib/errseq.c | 33 +-
samples/vfs/Makefile | 6 +-
samples/vfs/test-fsinfo.c | 883 ++++++++++++++++++++++++++++
samples/vfs/test-mntinfo.c | 277 +++++++++
46 files changed, 3808 insertions(+), 14 deletions(-)
create mode 100644 Documentation/filesystems/fsinfo.rst
create mode 100644 fs/ext4/fsinfo.c
create mode 100644 fs/fsinfo.c
create mode 100644 include/linux/fsinfo.h
create mode 100644 include/uapi/linux/fsinfo.h
create mode 100644 samples/vfs/test-fsinfo.c
create mode 100644 samples/vfs/test-mntinfo.c
On Mon, Aug 3, 2020 at 5:50 PM David Howells [off-list ref] wrote:
Hi Linus,
Here's a set of patches that adds a system call, fsinfo(), that allows
information about the VFS, mount topology, superblock and files to be
retrieved.
The patchset is based on top of the mount notifications patchset so that
the mount notification mechanism can be hooked to provide event counters
that can be retrieved with fsinfo(), thereby making it a lot faster to work
out which mounts have changed.
Note that there was a last minute change requested by Miklós: the event
counter bits got moved from the mount notification patchset to this one.
The counters got made atomic_long_t inside the kernel and __u64 in the
UAPI. The aggregate changes can be assessed by comparing pre-change tag,
fsinfo-core-20200724 to the requested pull tag.
Karel Zak has created preliminary patches that add support to libmount[*]
and Ian Kent has started working on making systemd use these and mount
notifications[**].
So why are you asking to pull at this stage?
Has anyone done a review of the patchset?
I think it's obvious that this API needs more work. The integration
work done by Ian is a good direction, but it's not quite the full
validation and review that a complex new API needs.
At least that's my opinion.
Thanks,
Miklos
From: Ian Kent <raven@themaw.net> Date: 2020-08-03 22:48:59
On Mon, 2020-08-03 at 16:27 +0100, David Howells wrote:
Hi Linus,
Here's a set of patches to add notifications for mount topology
events,
such as mounting, unmounting, mount expiry, mount reconfiguration.
The first patch in the series adds a hard limit on the number of
watches
that any particular user can add. The RLIMIT_NOFILE value for the
process
adding a watch is used as the limit. Even if you don't take the rest
of
the series, can you at least take this one?
An LSM hook is included for an LSM to rule on whether or not a mount
watch
may be set on a particular path.
This series is intended to be taken in conjunction with the fsinfo
series
which I'll post a pull request for shortly and which is dependent on
it.
Karel Zak[*] has created preliminary patches that add support to
libmount
and Ian Kent has started working on making systemd use them.
[*] https://github.com/karelzak/util-linux/commits/topic/fsinfo
Note that there have been some last minute changes to the patchset:
you
wanted something adding and Miklós wanted some bits taking
out/changing.
I've placed a tag, fsinfo-core-20200724 on the aggregate of these two
patchsets that can be compared to fsinfo-core-20200803.
To summarise the changes: I added the limiter that you wanted;
removed an
unused symbol; made the mount ID fields in the notificaion 64-bit
(the
fsinfo patchset has a change to convey the mount uniquifier instead
of the
mount ID); removed the event counters from the mount notification and
moved
the event counters into the fsinfo patchset.
I've pushed my systemd changes to a github repo.
I haven't yet updated it with the changes above but will get to it.
They can be found at:
https://github.com/raven-au/systemd.git branch notifications-devel
====
WHY?
====
Why do we want mount notifications? Whilst /proc/mounts can be
polled, it
only tells you that something changed in your namespace. To find
out, you
have to trawl /proc/mounts or similar to work out what changed in the
mount
object attributes and mount topology. I'm told that the proc file
holding
the namespace_sem is a point of contention, especially as the process
of
generating the text descriptions of the mounts/superblocks can be
quite
involved.
The notification generated here directly indicates the mounts
involved in
any particular event and gives an idea of what the change was.
This is combined with a new fsinfo() system call that allows, amongst
other
things, the ability to retrieve in one go an { id, change_counter }
tuple
from all the children of a specified mount, allowing buffer overruns
to be
dealt with quickly.
This is of use to systemd to improve efficiency:
https://lore.kernel.org/linux-fsdevel/20200227151421.3u74ijhqt6ekbiss@ws.net.home/
And it's not just Red Hat that's potentially interested in this:
https://lore.kernel.org/linux-fsdevel/293c9bd3-f530-d75e-c353-ddeabac27cf6@6wind.com/
David
---
The following changes since commit
ba47d845d715a010f7b51f6f89bae32845e6acb7:
Linux 5.8-rc6 (2020-07-19 15:41:18 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
tags/mount-notifications-20200803
for you to fetch changes up to
841a0dfa511364fa9a8d67512e0643669f1f03e3:
watch_queue: sample: Display mount tree change notifications (2020-
08-03 12:15:38 +0100)
----------------------------------------------------------------
Mount notifications
----------------------------------------------------------------
David Howells (5):
watch_queue: Limit the number of watches a user can hold
watch_queue: Make watch_sizeof() check record size
watch_queue: Add security hooks to rule on setting mount
watches
watch_queue: Implement mount topology and attribute change
notifications
watch_queue: sample: Display mount tree change notifications
Documentation/watch_queue.rst | 12 +-
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
fs/Kconfig | 9 ++
fs/Makefile | 1 +
fs/mount.h | 18 +++
fs/mount_notify.c | 222
++++++++++++++++++++++++++++
fs/namespace.c | 22 +++
include/linux/dcache.h | 1 +
include/linux/lsm_hook_defs.h | 3 +
include/linux/lsm_hooks.h | 6 +
include/linux/sched/user.h | 3 +
include/linux/security.h | 8 +
include/linux/syscalls.h | 2 +
include/linux/watch_queue.h | 7 +-
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/watch_queue.h | 31 +++-
kernel/sys_ni.c | 3 +
kernel/watch_queue.c | 8 +
samples/watch_queue/watch_test.c | 41 ++++-
security/security.c | 7 +
37 files changed, 422 insertions(+), 6 deletions(-)
create mode 100644 fs/mount_notify.c
From: Ian Kent <raven@themaw.net> Date: 2020-08-04 02:16:03
On Mon, 2020-08-03 at 18:42 +0200, Miklos Szeredi wrote:
On Mon, Aug 3, 2020 at 5:50 PM David Howells [off-list ref]
wrote:
quoted
Hi Linus,
Here's a set of patches that adds a system call, fsinfo(), that
allows
information about the VFS, mount topology, superblock and files to
be
retrieved.
The patchset is based on top of the mount notifications patchset so
that
the mount notification mechanism can be hooked to provide event
counters
that can be retrieved with fsinfo(), thereby making it a lot faster
to work
out which mounts have changed.
Note that there was a last minute change requested by Miklós: the
event
counter bits got moved from the mount notification patchset to this
one.
The counters got made atomic_long_t inside the kernel and __u64 in
the
UAPI. The aggregate changes can be assessed by comparing pre-
change tag,
fsinfo-core-20200724 to the requested pull tag.
Karel Zak has created preliminary patches that add support to
libmount[*]
and Ian Kent has started working on making systemd use these and
mount
notifications[**].
So why are you asking to pull at this stage?
Has anyone done a review of the patchset?
I have been working with the patch set as it has evolved for quite a
while now.
I've been reading the kernel code quite a bit and forwarded questions
and minor changes to David as they arose.
As for a review, not specifically, but while the series implements a
rather large change it's surprisingly straight forward to read.
In the time I have been working with it I haven't noticed any problems
except for those few minor things that I reported to David early on (in
some cases accompanied by simple patches).
And more recently (obviously) I've been working with the mount
notifications changes and, from a readability POV, I find it's the
same as the fsinfo() code.
I think it's obvious that this API needs more work. The integration
work done by Ian is a good direction, but it's not quite the full
validation and review that a complex new API needs.
Maybe but the system call is fundamental to making notifications useful
and, as I say, after working with it for quite a while I don't fell
there's missing features (that David hasn't added along the way) and
have found it provides what's needed for what I'm doing (for mount
notifications at least).
I'll be posting a github PR for systemd for discussion soon while I
get on with completing the systemd change. Like overflow handling and
meson build system changes to allow building with and without the
util-linux libmount changes.
So, ideally, I'd like to see the series merged, we've been working on
it for quite a considerable time now.
Ian
On Tue, Aug 4, 2020 at 4:15 AM Ian Kent [off-list ref] wrote:
On Mon, 2020-08-03 at 18:42 +0200, Miklos Szeredi wrote:
quoted
On Mon, Aug 3, 2020 at 5:50 PM David Howells [off-list ref]
wrote:
quoted
Hi Linus,
Here's a set of patches that adds a system call, fsinfo(), that
allows
information about the VFS, mount topology, superblock and files to
be
retrieved.
The patchset is based on top of the mount notifications patchset so
that
the mount notification mechanism can be hooked to provide event
counters
that can be retrieved with fsinfo(), thereby making it a lot faster
to work
out which mounts have changed.
Note that there was a last minute change requested by Miklós: the
event
counter bits got moved from the mount notification patchset to this
one.
The counters got made atomic_long_t inside the kernel and __u64 in
the
UAPI. The aggregate changes can be assessed by comparing pre-
change tag,
fsinfo-core-20200724 to the requested pull tag.
Karel Zak has created preliminary patches that add support to
libmount[*]
and Ian Kent has started working on making systemd use these and
mount
notifications[**].
So why are you asking to pull at this stage?
Has anyone done a review of the patchset?
I have been working with the patch set as it has evolved for quite a
while now.
I've been reading the kernel code quite a bit and forwarded questions
and minor changes to David as they arose.
As for a review, not specifically, but while the series implements a
rather large change it's surprisingly straight forward to read.
In the time I have been working with it I haven't noticed any problems
except for those few minor things that I reported to David early on (in
some cases accompanied by simple patches).
And more recently (obviously) I've been working with the mount
notifications changes and, from a readability POV, I find it's the
same as the fsinfo() code.
quoted
I think it's obvious that this API needs more work. The integration
work done by Ian is a good direction, but it's not quite the full
validation and review that a complex new API needs.
Maybe but the system call is fundamental to making notifications useful
and, as I say, after working with it for quite a while I don't fell
there's missing features (that David hasn't added along the way) and
have found it provides what's needed for what I'm doing (for mount
notifications at least).
Apart from the various issues related to the various mount ID's and
their sizes, my general comment is (and was always): why are we adding
a multiplexer for retrieval of mostly unrelated binary structures?
<linux/fsinfo.h> is 345 lines. This is not a simple and clean API.
A simple and clean replacement API would be:
int get_mount_attribute(int dfd, const char *path, const char
*attr_name, char *value_buf, size_t buf_size, int flags);
No header file needed with dubiously sized binary values.
The only argument was performance, but apart from purely synthetic
microbenchmarks that hasn't been proven to be an issue.
And notice how similar the above interface is to getxattr(), or the
proposed readfile(). Where has the "everything is a file" philosophy
gone?
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
Still I think it would be nice to have a general purpose attribute
retrieval API instead of the multiplicity of binary ioctls, xattrs,
etc.
Is that totally crazy? Nobody missing the beauty in recently introduced APIs?
Thanks,
Miklos
From: Ian Kent <raven@themaw.net> Date: 2020-08-05 01:34:00
On Tue, 2020-08-04 at 16:36 +0200, Miklos Szeredi wrote:
On Tue, Aug 4, 2020 at 4:15 AM Ian Kent [off-list ref] wrote:
quoted
On Mon, 2020-08-03 at 18:42 +0200, Miklos Szeredi wrote:
quoted
On Mon, Aug 3, 2020 at 5:50 PM David Howells <dhowells@redhat.com
quoted
wrote:
quoted
Hi Linus,
Here's a set of patches that adds a system call, fsinfo(), that
allows
information about the VFS, mount topology, superblock and files
to
be
retrieved.
The patchset is based on top of the mount notifications
patchset so
that
the mount notification mechanism can be hooked to provide event
counters
that can be retrieved with fsinfo(), thereby making it a lot
faster
to work
out which mounts have changed.
Note that there was a last minute change requested by Miklós:
the
event
counter bits got moved from the mount notification patchset to
this
one.
The counters got made atomic_long_t inside the kernel and __u64
in
the
UAPI. The aggregate changes can be assessed by comparing pre-
change tag,
fsinfo-core-20200724 to the requested pull tag.
Karel Zak has created preliminary patches that add support to
libmount[*]
and Ian Kent has started working on making systemd use these
and
mount
notifications[**].
So why are you asking to pull at this stage?
Has anyone done a review of the patchset?
I have been working with the patch set as it has evolved for quite
a
while now.
I've been reading the kernel code quite a bit and forwarded
questions
and minor changes to David as they arose.
As for a review, not specifically, but while the series implements
a
rather large change it's surprisingly straight forward to read.
In the time I have been working with it I haven't noticed any
problems
except for those few minor things that I reported to David early on
(in
some cases accompanied by simple patches).
And more recently (obviously) I've been working with the mount
notifications changes and, from a readability POV, I find it's the
same as the fsinfo() code.
quoted
I think it's obvious that this API needs more work. The
integration
work done by Ian is a good direction, but it's not quite the full
validation and review that a complex new API needs.
Maybe but the system call is fundamental to making notifications
useful
and, as I say, after working with it for quite a while I don't fell
there's missing features (that David hasn't added along the way)
and
have found it provides what's needed for what I'm doing (for mount
notifications at least).
Apart from the various issues related to the various mount ID's and
their sizes, my general comment is (and was always): why are we
adding
a multiplexer for retrieval of mostly unrelated binary structures?
<linux/fsinfo.h> is 345 lines. This is not a simple and clean API.
A simple and clean replacement API would be:
int get_mount_attribute(int dfd, const char *path, const char
*attr_name, char *value_buf, size_t buf_size, int flags);
No header file needed with dubiously sized binary values.
The only argument was performance, but apart from purely synthetic
microbenchmarks that hasn't been proven to be an issue.
And notice how similar the above interface is to getxattr(), or the
proposed readfile(). Where has the "everything is a file"
philosophy
gone?
Maybe, but that philosophy (in a roundabout way) is what's resulted
in some of the problems we now have. Granted it's blind application
of that philosophy rather than the philosophy itself but that is
what happens.
I get that your comments are driven by the way that philosophy should
be applied which is more of a "if it works best doing it that way then
do it that way, and that's usually a file".
In this case there is a logical division of various types of file
system information and the underlying suggestion is maybe it's time
to move away from the "everything is a file" hard and fast rule,
and get rid of some of the problems that have resulted from it.
The notifications is an example, yes, the delivery mechanism is
a "file" but the design of the queueing mechanism makes a lot of
sense for the throughput that's going to be needed as time marches
on. Then there's different sub-systems each with unique information
that needs to be deliverable some other way because delivering "all"
the information via the notification would be just plain wrong so
a multi-faceted information delivery mechanism makes the most
sense to allow specific targeted retrieval of individual items of
information.
But that also supposes your at least open to the idea that "maybe
not everything should be a file".
I think we already lost that with the xattr API, that should have
been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
Still I think it would be nice to have a general purpose attribute
retrieval API instead of the multiplicity of binary ioctls, xattrs,
etc.
Is that totally crazy? Nobody missing the beauty in recently
introduced APIs?
Thanks,
Miklos
On Wed, Aug 5, 2020 at 3:33 AM Ian Kent [off-list ref] wrote:
On Tue, 2020-08-04 at 16:36 +0200, Miklos Szeredi wrote:
quoted
And notice how similar the above interface is to getxattr(), or the
proposed readfile(). Where has the "everything is a file"
philosophy
gone?
Maybe, but that philosophy (in a roundabout way) is what's resulted
in some of the problems we now have. Granted it's blind application
of that philosophy rather than the philosophy itself but that is
what happens.
Agree. What people don't seem to realize, even though there are
blindingly obvious examples, that binary interfaces like the proposed
fsinfo(2) syscall can also result in a multitude of problems at the
same time as solving some others.
There's no magic solution in API design, it's not balck and white.
We just need to strive for a good enough solution. The problem seems
to be that trying to discuss the merits of other approaches seems to
hit a brick wall. We just see repeated pull requests from David,
without any real discussion of the proposed alternatives.
I get that your comments are driven by the way that philosophy should
be applied which is more of a "if it works best doing it that way then
do it that way, and that's usually a file".
In this case there is a logical division of various types of file
system information and the underlying suggestion is maybe it's time
to move away from the "everything is a file" hard and fast rule,
and get rid of some of the problems that have resulted from it.
The notifications is an example, yes, the delivery mechanism is
a "file" but the design of the queueing mechanism makes a lot of
sense for the throughput that's going to be needed as time marches
on. Then there's different sub-systems each with unique information
that needs to be deliverable some other way because delivering "all"
the information via the notification would be just plain wrong so
a multi-faceted information delivery mechanism makes the most
sense to allow specific targeted retrieval of individual items of
information.
But that also supposes your at least open to the idea that "maybe
not everything should be a file".
Sure. I've learned pragmatism, although idealist at heart. And I'm
not saying all API's from David are shit. statx(2) is doing fine.
It's a simple binary interface that does its job well. Compare the
header files for statx and fsinfo, though, and maybe you'll see what
I'm getting at...
Thanks,
Miklos
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi [off-list ref] wrote:
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Yes, the implementation might be somewhat tricky, but that's another
question. Also I'm pretty sure that we should be reducing the
POSIX-ness of anything below "//" to the bare minimum. No seeking,
etc....
I think this would open up some nice possibilities beyond the fsinfo thing.
Thanks,
Miklos
From: Ian Kent <raven@themaw.net> Date: 2020-08-05 20:14:29
On Wed, 2020-08-05 at 10:00 +0200, Miklos Szeredi wrote:
On Wed, Aug 5, 2020 at 3:33 AM Ian Kent [off-list ref] wrote:
quoted
On Tue, 2020-08-04 at 16:36 +0200, Miklos Szeredi wrote:
quoted
And notice how similar the above interface is to getxattr(), or
the
proposed readfile(). Where has the "everything is a file"
philosophy
gone?
Maybe, but that philosophy (in a roundabout way) is what's resulted
in some of the problems we now have. Granted it's blind application
of that philosophy rather than the philosophy itself but that is
what happens.
Agree. What people don't seem to realize, even though there are
blindingly obvious examples, that binary interfaces like the proposed
fsinfo(2) syscall can also result in a multitude of problems at the
same time as solving some others.
There's no magic solution in API design, it's not balck and white.
We just need to strive for a good enough solution. The problem seems
to be that trying to discuss the merits of other approaches seems to
hit a brick wall. We just see repeated pull requests from David,
without any real discussion of the proposed alternatives.
quoted
I get that your comments are driven by the way that philosophy
should
be applied which is more of a "if it works best doing it that way
then
do it that way, and that's usually a file".
In this case there is a logical division of various types of file
system information and the underlying suggestion is maybe it's time
to move away from the "everything is a file" hard and fast rule,
and get rid of some of the problems that have resulted from it.
The notifications is an example, yes, the delivery mechanism is
a "file" but the design of the queueing mechanism makes a lot of
sense for the throughput that's going to be needed as time marches
on. Then there's different sub-systems each with unique information
that needs to be deliverable some other way because delivering
"all"
the information via the notification would be just plain wrong so
a multi-faceted information delivery mechanism makes the most
sense to allow specific targeted retrieval of individual items of
information.
But that also supposes your at least open to the idea that "maybe
not everything should be a file".
Sure. I've learned pragmatism, although idealist at heart. And I'm
not saying all API's from David are shit. statx(2) is doing fine.
It's a simple binary interface that does its job well. Compare the
header files for statx and fsinfo, though, and maybe you'll see what
I'm getting at...
Yeah, but I'm biased so not much joy there ... ;)
Ian
On Wed, Aug 05, 2020 at 10:24:23AM +0200, Miklos Szeredi wrote:
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi [off-list ref] wrote:
quoted
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
Opted for triple slash in the hope that just maybe we could add a global
/proc/sys/fs/resolve_alt knob to optionally turn on alternative (non-POSIX) path
resolution without breaking too many things. Will try that later...
Comments?
Thanks,
Miklos
cat_alt.c:
-------- >8 --------
#define _GNU_SOURCE
#include <err.h>
#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <stdlib.h>
#include <linux/unistd.h>
#include <linux/openat2.h>
#define RESOLVE_ALT 0x20 /* Alternative path walk mode where
multiple slashes have special meaning */
int main(int argc, char *argv[])
{
struct open_how how = {
.flags = O_RDONLY,
.resolve = RESOLVE_ALT,
};
int fd, res, i;
char buf[65536], *end;
const char *path = argv[1];
int dfd = AT_FDCWD;
if (argc < 2 || argc > 4)
errx(1, "usage: %s path [dirfd] [--nofollow]", argv[0]);
for (i = 2; i < argc; i++) {
if (strcmp(argv[i], "--nofollow") == 0) {
how.flags |= O_NOFOLLOW;
} else {
dfd = strtoul(argv[i], &end, 0);
if (end == argv[i] || *end)
errx(1, "invalid dirfd: %s", argv[i]);
}
}
fd = syscall(__NR_openat2, dfd, path, &how, sizeof(how));
if (fd == -1)
err(1, "failed to open %s", argv[1]);
while (1) {
res = read(fd, buf, sizeof(buf));
if (res == -1)
err(1, "failed to read file");
if (res == 0)
break;
write(1, buf, res);
}
close(fd);
return 0;
}
-------- >8 --------
---
fs/Makefile | 2
fs/file_table.c | 70 ++++++++++++++--------
fs/fsmeta.c | 135 +++++++++++++++++++++++++++++++++++++++++++
fs/internal.h | 9 ++
fs/mount.h | 4 +
fs/namei.c | 77 +++++++++++++++++++++---
fs/namespace.c | 12 +++
fs/open.c | 2
fs/proc_namespace.c | 2
include/linux/fcntl.h | 2
include/linux/namei.h | 3
include/uapi/linux/magic.h | 1
include/uapi/linux/openat2.h | 2
13 files changed, 282 insertions(+), 39 deletions(-)
@@ -209,31 +196,66 @@ static struct file *alloc_file(const strfile->f_op=fop; if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)i_readcount_inc(path->dentry->d_inode);+}++/**+*alloc_file-allocateandinitializea'struct file'+*+*@path:the(dentry,vfsmount)pairforthenewfile+*@flags:O_...flagswithwhichthenewfilewillbeopened+*@fop:the'struct file_operations'forthenewfile+*/+staticstructfile*alloc_file(conststructpath*path,intflags,+conststructfile_operations*fop)+{+structfile*file;++file=alloc_empty_file(flags,current_cred());+ if (IS_ERR(file))+returnfile;++init_file(file,path,flags,fop);+returnfile;}-structfile*alloc_file_pseudo(structinode*inode,structvfsmount*mnt,-constchar*name,intflags,-conststructfile_operations*fops)+intinit_file_pseudo(structfile*file,structinode*inode,+structvfsmount*mnt,constchar*name,intflags,+conststructfile_operations*fops){staticconststructdentry_operationsanon_ops={.d_dname=simple_dname};structqstrthis=QSTR_INIT(name,strlen(name));structpathpath;-structfile*file;path.dentry=d_alloc_pseudo(mnt->mnt_sb,&this); if (!path.dentry)-returnERR_PTR(-ENOMEM);+return-ENOMEM; if (!mnt->mnt_sb->s_d_op)d_set_d_op(path.dentry,&anon_ops);path.mnt=mntget(mnt);d_instantiate(path.dentry,inode);-file=alloc_file(&path,flags,fops);- if (IS_ERR(file)) {-ihold(inode);-path_put(&path);+init_file(file,&path,flags,fops);++return0;+}++structfile*alloc_file_pseudo(structinode*inode,structvfsmount*mnt,+constchar*name,intflags,+conststructfile_operations*fops)+{+structfile*file;+interr;++file=alloc_empty_file(flags,current_cred());+ if (IS_ERR(file))+returnfile;++err=init_file_pseudo(file,inode,mnt,name,flags,fops);+ if (err) {+fput(file);+file=ERR_PTR(err);}returnfile;}---/dev/null+++b/fs/fsmeta.c
@@ -0,0 +1,135 @@+#include <linux/fs.h>+#include <linux/slab.h>+#include <linux/magic.h>+#include <linux/seq_file.h>+#include <linux/fs_struct.h>+#include <linux/pseudo_fs.h>++#include "mount.h"+#include "internal.h"++staticstructvfsmount*fsmeta_mnt;+staticstructinode*fsmeta_inode;+++staticstructvfsmount*fsmeta_mnt_info_get_mnt(structseq_file*seq)+{+structproc_mounts*p=seq->private;++return&list_entry(p->cursor.mnt_list.next,structmount,mnt_list)->mnt;+}++staticvoid*fsmeta_mnt_info_start(structseq_file*seq,loff_t*pos)+{+mnt_namespace_lock_read();+return*pos==0?fsmeta_mnt_info_get_mnt(seq):NULL;+}++staticvoid*fsmeta_mnt_info_next(structseq_file*seq,void*v,loff_t*pos)+{+++*pos;+returnNULL;+}++staticvoidfsmeta_mnt_info_stop(structseq_file*seq,void*v)+{+mnt_namespace_unlock_read();+}++staticintfsmeta_mnt_info_show(structseq_file*seq,void*v)+{+returnshow_mountinfo(seq,v);+}++staticconststructseq_operationsfsmeta_mnt_info_sops={+.start=fsmeta_mnt_info_start,+.next=fsmeta_mnt_info_next,+.stop=fsmeta_mnt_info_stop,+.show=fsmeta_mnt_info_show,+};++staticintfsmeta_mnt_info_release(structinode*inode,structfile*file)+{+ if (file->private_data) {+structseq_file*seq=file->private_data;+structproc_mounts*p=seq->private;++mntput(fsmeta_mnt_info_get_mnt(seq));+path_put(&p->root);++returnseq_release_private(inode,file);+}+return0;+}++staticconststructfile_operationsfsmeta_mnt_info_fops={+.release=fsmeta_mnt_info_release,+.read=seq_read,+.llseek=no_llseek,+};++staticintfsmeta_mnt_info_open(structfile*file,conststructpath*path,+conststructopen_flags*op)+{+structproc_mounts*p;+interr;++err=init_file_pseudo(file,fsmeta_inode,fsmeta_mnt,"[mnt.info]",+op->open_flag,&fsmeta_mnt_info_fops);+ if (err)+returnerr;+/*+*Thisreferenceisnowsunkinfile->f_path.dentry->d_inodeandwill+*bereleasedbyfput()+*/+ihold(fsmeta_inode);++err=seq_open_private(file,&fsmeta_mnt_info_sops,sizeof(*p));+ if (err)+returnerr;++p=((structseq_file*)file->private_data)->private;+get_fs_root(current->fs,&p->root);+p->cursor.mnt_list.next=&real_mount(mntget(path->mnt))->mnt_list;++return0;+}++intfsmeta_open(constchar*meta_name,conststructpath*path,+structfile*file,conststructopen_flags*op)+{+ if (op->open_flag & ~(O_LARGEFILE | O_CLOEXEC | O_NOFOLLOW))+return-EINVAL;++ if (strcmp(meta_name, "mnt/info") == 0)+returnfsmeta_mnt_info_open(file,path,op);++pr_info("invalid fsmeta file <%s> on %pd4\n",meta_name,path->dentry);+return-EINVAL;+}++staticintfsmeta_init_fs_context(structfs_context*fc)+{+returninit_pseudo(fc,FSMETA_MAGIC)?0:-ENOMEM;+}++staticstructfile_system_typefsmeta_fs_type={+.name="fsmeta",+.init_fs_context=fsmeta_init_fs_context,+.kill_sb=kill_anon_super,+};++staticint__initfsmeta_init(void)+{+fsmeta_mnt=kern_mount(&fsmeta_fs_type);+ if (IS_ERR(fsmeta_mnt))+panic("fsmeta_init() kernel mount failed (%ld)\n",PTR_ERR(fsmeta_mnt));++fsmeta_inode=alloc_anon_inode(fsmeta_mnt->mnt_sb);+ if (IS_ERR(fsmeta_inode))+panic("fsmeta_init() inode allocation failed (%ld)\n",PTR_ERR(fsmeta_inode));++return0;+}+fs_initcall(fsmeta_init);+---a/fs/internal.h+++b/fs/internal.h
@@ -2094,6 +2094,30 @@ static inline u64 hash_name(const void *#endif+staticintlookup_alt(constchar*name,structnameidata*nd)+{+ if ((nd->flags & LOOKUP_RCU) && unlazy_walk(nd) != 0)+return-ECHILD;++nd->last.name=name+3;+nd->last_type=LAST_META;++return0;+}++staticboolis_alt(constchar*name,structnameidata*nd,intdepth)+{+ if (!(nd->flags & LOOKUP_ALT))+returnfalse;++/*noalternativelookupinsidesymlinks*/+ if (depth)+returnfalse;++/*name[0]hasalreadybeenverifiedtobeaslash*/+returnname[1]=='/'&&name[2]=='/'&&name[3]!='/';+}+/**Nameresolution.*Thisisthebasicnameresolutionfunction,turningapathnameinto
@@ -2111,8 +2135,13 @@ static int link_path_walk(const char *nand->flags|=LOOKUP_PARENT; if (IS_ERR(name))returnPTR_ERR(name);-while(*name=='/')-name++;+ if (*name == '/') {+ if (!is_alt(name, nd, depth)) {+do{+name++;+}while(*name=='/');+}+} if (!*name)return0;
@@ -2122,6 +2151,9 @@ static int link_path_walk(const char *nau64hash_len;inttype;+ if (*name == '/')+returnlookup_alt(name,nd);+err=may_lookup(nd); if (err)returnerr;
@@ -2163,6 +2195,13 @@ static int link_path_walk(const char *na*Ifitwasn't NUL, we know it was '/'.Skipthat*slash,andcontinueuntilnomoreslashes.*/+ if (is_alt(name, nd, depth)) {+link=walk_component(nd,WALK_TRAILING);+ if (unlikely(link))+gotoLINK;++returnlookup_alt(name,nd);+}do{name++;}while(unlikely(*name=='/'));
@@ -2183,6 +2222,7 @@ static int link_path_walk(const char *nalink=walk_component(nd,WALK_MORE);} if (unlikely(link)) {+LINK: if (IS_ERR(link))returnPTR_ERR(link);/*asymlinktofollow*/
@@ -2272,7 +2312,8 @@ static const char *path_init(struct namedentry=f.file->f_path.dentry;- if (*s && unlikely(!d_can_lookup(dentry))) {+ if (*s && unlikely(!d_can_lookup(dentry)) &&+!is_alt(s,nd,0)){fdput(f);returnERR_PTR(-ENOTDIR);}
@@ -2303,6 +2344,9 @@ static const char *path_init(struct namestaticinlineconstchar*lookup_last(structnameidata*nd){+ if (nd->last_type == LAST_META)+returnERR_PTR(-EINVAL);+ if (nd->last_type == LAST_NORM && nd->last.name[nd->last.len])nd->flags|=LOOKUP_FOLLOW|LOOKUP_DIRECTORY;
@@ -2331,7 +2375,7 @@ static int path_lookupat(struct nameidatwhile(!(err=link_path_walk(s,nd))&&(s=lookup_last(nd))!=NULL)-;+nd->flags&=~LOOKUP_ALT; if (!err)err=complete_walk(nd);
@@ -2410,9 +2454,15 @@ static struct filename *filename_parenta if (unlikely(retval == -ESTALE))retval=path_parentat(&nd,flags|LOOKUP_REVAL,parent); if (likely(!retval)) {-*last=nd.last;-*type=nd.last_type;-audit_inode(name,parent->dentry,AUDIT_INODE_PARENT);+ if (nd.last_type == LAST_META) {+path_put(parent);+putname(name);+name=ERR_PTR(-EINVAL);+}else{+*last=nd.last;+*type=nd.last_type;+audit_inode(name,parent->dentry,AUDIT_INODE_PARENT);+}}else{putname(name);name=ERR_PTR(retval);
@@ -3123,6 +3173,10 @@ static const char *open_last_lookups(strnd->flags|=op->intent; if (nd->last_type != LAST_NORM) {+ if (nd->last_type == LAST_META) {+returnERR_PTR(fsmeta_open(nd->last.name,&nd->path,+file,op));+} if (nd->depth)put_link(nd);returnhandle_dots(nd,nd->last_type);
@@ -3206,6 +3260,9 @@ static int do_open(struct nameidata *nd,intacc_mode;interror;+ if (nd->last_type == LAST_META)+return0;+ if (!(file->f_mode & (FMODE_OPENED | FMODE_CREATED))) {error=complete_walk(nd); if (error)
@@ -1098,6 +1098,8 @@ inline int build_open_flags(const structlookup_flags|=LOOKUP_BENEATH; if (how->resolve & RESOLVE_IN_ROOT)lookup_flags|=LOOKUP_IN_ROOT;+ if (how->resolve & RESOLVE_ALT)+lookup_flags|=LOOKUP_ALT;op->lookup_flags=lookup_flags;return0;---a/fs/proc_namespace.c+++b/fs/proc_namespace.c
@@ -128,7 +128,7 @@ static int show_vfsmnt(struct seq_file *returnerr;}-staticintshow_mountinfo(structseq_file*m,structvfsmount*mnt)+intshow_mountinfo(structseq_file*m,structvfsmount*mnt){structproc_mounts*p=m->private;structmount*r=real_mount(mnt);---a/include/linux/fcntl.h+++b/include/linux/fcntl.h
@@ -19,7 +19,7 @@/* List of all valid flags for the how->resolve argument:*/#define VALID_RESOLVE_FLAGS \(RESOLVE_NO_XDEV|RESOLVE_NO_MAGICLINKS|RESOLVE_NO_SYMLINKS|\-RESOLVE_BENEATH|RESOLVE_IN_ROOT)+RESOLVE_BENEATH|RESOLVE_IN_ROOT|RESOLVE_ALT)/*Listofallopen_how"versions".*/#define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */---a/include/linux/namei.h+++b/include/linux/namei.h
@@ -15,7 +15,7 @@ enum { MAX_NESTED_LINKS = 8 };/**TypeofthelastcomponentonLOOKUP_PARENT*/-enum{LAST_NORM,LAST_ROOT,LAST_DOT,LAST_DOTDOT};+enum{LAST_NORM,LAST_ROOT,LAST_DOT,LAST_DOTDOT,LAST_META};/*pathwalkmode*/#define LOOKUP_FOLLOW 0x0001 /* follow links at the end */
@@ -27,6 +27,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LA#define LOOKUP_REVAL 0x0020 /* tell ->d_revalidate() to trust no cache */#define LOOKUP_RCU 0x0040 /* RCU pathwalk mode; semi-internal */+#define LOOKUP_ALT 0x200000 /* Alternative path walk mode *//*Thesetellfilesystemmethodsthatwearedealingwiththefinalcomponent...*/#define LOOKUP_OPEN 0x0100 /* ... in open */---a/include/uapi/linux/magic.h+++b/include/uapi/linux/magic.h
@@ -35,5 +35,7 @@ struct open_how {#define RESOLVE_IN_ROOT 0x10 /* Make all jumps to "/" and ".."bescopedinsidethedirfd(similartochroot(2)).*/+#define RESOLVE_ALT 0x20 /* Alternative path walk mode where+multipleslasheshavespecialmeaning*/#endif /* _UAPI_LINUX_OPENAT2_H */
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-11 14:08:49
On Tue, Aug 11, 2020 at 03:54:19PM +0200, Miklos Szeredi wrote:
On Wed, Aug 05, 2020 at 10:24:23AM +0200, Miklos Szeredi wrote:
quoted
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi [off-list ref] wrote:
quoted
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
Opted for triple slash in the hope that just maybe we could add a global
/proc/sys/fs/resolve_alt knob to optionally turn on alternative (non-POSIX) path
resolution without breaking too many things. Will try that later...
Comments?
Hell, NO. This is unspeakably tasteless. And full of lovely corner cases wrt
symlink bodies, etc.
Consider that one NAKed. I'm seriously unhappy with the entire fsinfo thing
in general, but this one is really over the top.
On Tue, Aug 11, 2020 at 4:08 PM Al Viro [off-list ref] wrote:
On Tue, Aug 11, 2020 at 03:54:19PM +0200, Miklos Szeredi wrote:
quoted
On Wed, Aug 05, 2020 at 10:24:23AM +0200, Miklos Szeredi wrote:
quoted
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi [off-list ref] wrote:
quoted
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
Opted for triple slash in the hope that just maybe we could add a global
/proc/sys/fs/resolve_alt knob to optionally turn on alternative (non-POSIX) path
resolution without breaking too many things. Will try that later...
Comments?
Hell, NO. This is unspeakably tasteless. And full of lovely corner cases wrt
symlink bodies, etc.
It's disabled inside symlink body resolution.
Rules are simple:
- strip off trailing part after first instance of ///
- perform path lookup as normal
- resolve meta path after /// on result of normal lookup
Thanks,
Miklos
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-11 14:31:18
On Tue, Aug 11, 2020 at 04:22:19PM +0200, Miklos Szeredi wrote:
On Tue, Aug 11, 2020 at 4:08 PM Al Viro [off-list ref] wrote:
quoted
On Tue, Aug 11, 2020 at 03:54:19PM +0200, Miklos Szeredi wrote:
quoted
On Wed, Aug 05, 2020 at 10:24:23AM +0200, Miklos Szeredi wrote:
quoted
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi [off-list ref] wrote:
quoted
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
Opted for triple slash in the hope that just maybe we could add a global
/proc/sys/fs/resolve_alt knob to optionally turn on alternative (non-POSIX) path
resolution without breaking too many things. Will try that later...
Comments?
Hell, NO. This is unspeakably tasteless. And full of lovely corner cases wrt
symlink bodies, etc.
It's disabled inside symlink body resolution.
Rules are simple:
- strip off trailing part after first instance of ///
- perform path lookup as normal
- resolve meta path after /// on result of normal lookup
... and interpolation of relative symlink body into the pathname does change
behaviour now, *including* the cases when said symlink body does not contain
that triple-X^Hslash garbage. Wonderful...
On Tue, Aug 11, 2020 at 4:31 PM Al Viro [off-list ref] wrote:
On Tue, Aug 11, 2020 at 04:22:19PM +0200, Miklos Szeredi wrote:
quoted
On Tue, Aug 11, 2020 at 4:08 PM Al Viro [off-list ref] wrote:
quoted
On Tue, Aug 11, 2020 at 03:54:19PM +0200, Miklos Szeredi wrote:
quoted
On Wed, Aug 05, 2020 at 10:24:23AM +0200, Miklos Szeredi wrote:
quoted
On Tue, Aug 4, 2020 at 4:36 PM Miklos Szeredi [off-list ref] wrote:
quoted
I think we already lost that with the xattr API, that should have been
done in a way that fits this philosophy. But given that we have "/"
as the only special purpose char in filenames, and even repetitions
are allowed, it's hard to think of a good way to do that. Pity.
One way this could be solved is to allow opting into an alternative
path resolution mode.
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
Opted for triple slash in the hope that just maybe we could add a global
/proc/sys/fs/resolve_alt knob to optionally turn on alternative (non-POSIX) path
resolution without breaking too many things. Will try that later...
Comments?
Hell, NO. This is unspeakably tasteless. And full of lovely corner cases wrt
symlink bodies, etc.
It's disabled inside symlink body resolution.
Rules are simple:
- strip off trailing part after first instance of ///
- perform path lookup as normal
- resolve meta path after /// on result of normal lookup
... and interpolation of relative symlink body into the pathname does change
behaviour now, *including* the cases when said symlink body does not contain
that triple-X^Hslash garbage. Wonderful...
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-11 14:42:53
On Tue, Aug 11, 2020 at 04:36:32PM +0200, Miklos Szeredi wrote:
quoted
quoted
- strip off trailing part after first instance of ///
- perform path lookup as normal
- resolve meta path after /// on result of normal lookup
... and interpolation of relative symlink body into the pathname does change
behaviour now, *including* the cases when said symlink body does not contain
that triple-X^Hslash garbage. Wonderful...
Can you please explain?
Currently substituting the body of a relative symlink in place of its name
results in equivalent pathname. With your patch that is not just no longer
true, it's no longer true even when the symlink body does not contain that
/// kludge - it can come in part from the symlink body and in part from the
rest of pathname. I.e. you can't even tell if substitution is an equivalent
replacement by looking at the symlink body alone.
On Tue, Aug 11, 2020 at 4:42 PM Al Viro [off-list ref] wrote:
On Tue, Aug 11, 2020 at 04:36:32PM +0200, Miklos Szeredi wrote:
quoted
quoted
quoted
- strip off trailing part after first instance of ///
- perform path lookup as normal
- resolve meta path after /// on result of normal lookup
... and interpolation of relative symlink body into the pathname does change
behaviour now, *including* the cases when said symlink body does not contain
that triple-X^Hslash garbage. Wonderful...
Can you please explain?
Currently substituting the body of a relative symlink in place of its name
results in equivalent pathname.
Except proc symlinks, that is.
With your patch that is not just no longer
true, it's no longer true even when the symlink body does not contain that
/// kludge - it can come in part from the symlink body and in part from the
rest of pathname. I.e. you can't even tell if substitution is an equivalent
replacement by looking at the symlink body alone.
Yes, that's true not just for symlink bodies but any concatenation of
two path segments.
That's why it's enabled with RESOLVE_ALT. I've said that I plan to
experiment with turning this on globally, but that doesn't mean it's
necessarily a good idea. The posted patch contains nothing of that
sort.
Thanks,
Miklos
[ I missed the beginning of this discussion, so maybe this was already
suggested ]
On Tue, Aug 11, 2020 at 6:54 AM Miklos Szeredi [off-list ref] wrote:
quoted
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
I don't think this works for the reasons Al says, but a slight
modification might.
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
So you couldn't do it with _one_ pathname, because that is always
fundamentally going to hit pathname lookup rules.
But if you start a new path lookup with new rules, that's fine.
This is what I think xattrs should always have done, because they are
broken garbage.
In fact, if we do it right, I think we could have "getxattr()" be 100%
equivalent to (modulo all the error handling that this doesn't do, of
course):
ssize_t getxattr(const char *path, const char *name,
void *value, size_t size)
{
int fd, attrfd;
fd = open(path, O_PATH);
attrfd = openat(fd, name, O_ALT);
close(fd);
read(attrfd, value, size);
close(attrfd);
}
and you'd still use getxattr() and friends as a shorthand (and for
POSIX compatibility), but internally in the kernel we'd have a
interface around that "xattrs are just file handles" model.
Linus
On Tue, Aug 11, 2020 at 5:20 PM Linus Torvalds
[off-list ref] wrote:
[ I missed the beginning of this discussion, so maybe this was already
suggested ]
On Tue, Aug 11, 2020 at 6:54 AM Miklos Szeredi [off-list ref] wrote:
quoted
quoted
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
I don't think this works for the reasons Al says, but a slight
modification might.
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
That would have been my backup suggestion, in case the unified
namespace doesn't work out.
I wouldn't think the normal lookup rules really get in the way if we
explicitly enable alternative path lookup with a flag. The rules just
need to be documented.
What's the disadvantage of doing it with a single lookup WITH an enabling flag?
It's definitely not going to break anything, so no backward
compatibility issues whatsoever.
Thanks,
Miklos
From: Andy Lutomirski <luto@amacapital.net> Date: 2020-08-11 15:39:36
On Aug 11, 2020, at 8:20 AM, Linus Torvalds [off-list ref] wrote:
[ I missed the beginning of this discussion, so maybe this was already
suggested ]
quoted
On Tue, Aug 11, 2020 at 6:54 AM Miklos Szeredi [off-list ref] wrote:
quoted
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
I don't think this works for the reasons Al says, but a slight
modification might.
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
So you couldn't do it with _one_ pathname, because that is always
fundamentally going to hit pathname lookup rules.
But if you start a new path lookup with new rules, that's fine.
This is what I think xattrs should always have done, because they are
broken garbage.
In fact, if we do it right, I think we could have "getxattr()" be 100%
equivalent to (modulo all the error handling that this doesn't do, of
course):
ssize_t getxattr(const char *path, const char *name,
void *value, size_t size)
{
int fd, attrfd;
fd = open(path, O_PATH);
attrfd = openat(fd, name, O_ALT);
close(fd);
read(attrfd, value, size);
close(attrfd);
}
and you'd still use getxattr() and friends as a shorthand (and for
POSIX compatibility), but internally in the kernel we'd have a
interface around that "xattrs are just file handles" model.
This is a lot like a less nutty version of NTFS streams, whereas the /// idea is kind of like an extra-nutty version of NTFS streams.
I am personally not a fan of the in-band signaling implications of overloading /. For example, there is plenty of code out there that thinks that (a + “/“ + b) concatenates paths. With /// overloaded, this stops being true.
On Tue, Aug 11, 2020 at 8:30 AM Miklos Szeredi [off-list ref] wrote:
What's the disadvantage of doing it with a single lookup WITH an enabling flag?
It's definitely not going to break anything, so no backward
compatibility issues whatsoever.
No backwards compatibility issues for existing programs, no.
But your suggestion is fundamentally ambiguous, and you most
definitely *can* hit that if people start using this in new programs.
Where does that "unified" pathname come from? It will be generated
from "base filename + metadata name" in user space, and
(a) the base filename might have double or triple slashes in it for
whatever reasons.
This is not some "made-up gotcha" thing - I see double slashes *all*
the time when we have things like Makefiles doing
srctree=../../src/
and then people do "$(srctree)/". If you haven't seen that kind of
pattern where the pathname has two (or sometimes more!) slashes in the
middle, you've led a very sheltered life.
(b) even if the new user space were to think about that, and remove
those (hah! when have you ever seen user space do that?), as Al
mentioned, the user *filesystem* might have pathnames with double
slashes as part of symlinks.
So now we'd have to make sure that when we traverse symlinks, that
O_ALT gets cleared. Which means that it's not a unified namespace
after all, because you can't make symlinks point to metadata.
Or we'd retroactively change the semantics of a symlink, and that _is_
a backwards compatibility issue. Not with old software, no, but it
changes the meaning of old symlinks!
So no, I don't think a unified namespace ends up working.
And I say that as somebody who actually loves the concept. Ask Al: I
have a few times pushed for "let's allow directory behavior on regular
files", so that you could do things like a tar-filesystem, and access
the contents of a tar-file by just doing
cat my-file.tar/inside/the/archive.c
or similar.
Al has convinced me it's a horrible idea (and there you have a
non-ambiguous marker: the slash at the end of a pathname that
otherwise looks and acts as a non-directory)
Linus
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-11 16:05:50
On Tue, Aug 11, 2020 at 08:20:24AM -0700, Linus Torvalds wrote:
I don't think this works for the reasons Al says, but a slight
modification might.
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
So you couldn't do it with _one_ pathname, because that is always
fundamentally going to hit pathname lookup rules.
But if you start a new path lookup with new rules, that's fine.
Except that you suddenly see non-directory dentries get children.
And a lot of dcache-related logics needs to be changed if that
becomes possible.
I agree that xattrs are garbage, but this approach won't be
a straightforward solution. Can those suckers be passed to
...at() as starting points? Can they be bound in namespace?
Can something be bound *on* them? What do they have for inodes
and what maintains their inumbers (and st_dev, while we are at
it)? Can _they_ have secondaries like that (sensu Swift)?
Is that a flat space, or can they be directories?
Only a part of the problems is implementation-related (and those are
not trivial at all); most the fun comes from semantics of those things.
And answers to the implementation questions are seriously dependent upon
that...
On Tue, Aug 11, 2020 at 9:05 AM Al Viro [off-list ref] wrote:
Except that you suddenly see non-directory dentries get children.
And a lot of dcache-related logics needs to be changed if that
becomes possible.
Yeah, I think you'd basically need to associate a (dynamic)
mount-point to that path when you start doing O_ALT. Or something.
And it might not be reasonably implementable. I just think that as
_interface_ it's unambiguous and fairly clean, and if Miklos can
implement something like that, I think it would be maintainable.
No?
Linus
On Aug 11, 2020, at 8:20 AM, Linus Torvalds [off-list ref] wrote:
[ I missed the beginning of this discussion, so maybe this was already
suggested ]
quoted
On Tue, Aug 11, 2020 at 6:54 AM Miklos Szeredi [off-list ref] wrote:
quoted
E.g.
openat(AT_FDCWD, "foo/bar//mnt/info", O_RDONLY | O_ALT);
Proof of concept patch and test program below.
I don't think this works for the reasons Al says, but a slight
modification might.
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
So you couldn't do it with _one_ pathname, because that is always
fundamentally going to hit pathname lookup rules.
But if you start a new path lookup with new rules, that's fine.
This is what I think xattrs should always have done, because they are
broken garbage.
In fact, if we do it right, I think we could have "getxattr()" be 100%
equivalent to (modulo all the error handling that this doesn't do, of
course):
ssize_t getxattr(const char *path, const char *name,
void *value, size_t size)
{known
int fd, attrfd;
fd = open(path, O_PATH);
attrfd = openat(fd, name, O_ALT);
close(fd);
read(attrfd, value, size);
close(attrfd);
}
and you'd still use getxattr() and friends as a shorthand (and for
POSIX compatibility), but internally in the kernel we'd have a
interface around that "xattrs are just file handles" model.
This doesn't work so well for setxattr(), which we want to be atomic.
This is a lot like a less nutty version of NTFS streams, whereas the /// idea is kind of like an extra-nutty version of NTFS streams.
I am personally not a fan of the in-band signaling implications of overloading /. For example, there is plenty of code out there that thinks that (a + “/“ + b) concatenates paths. With /// overloaded, this stops being true.
Since a////////b has known meaning, and lots of applications
play loose with '/', its really dangerous to treat the string as
special. We only get away with '.' and '..' because their behavior
was defined before many of y'all were born.
On Tue, Aug 11, 2020 at 9:17 AM Casey Schaufler [off-list ref] wrote:
This doesn't work so well for setxattr(), which we want to be atomic.
Well, it's not like the old interfaces could go away. But yes, doing
metadatafd = openat(fd, "metadataname", O_ALT | O_CREAT | O_EXCL)
to create a new xattr (and then write to it) would not act like
setxattr(). Even if you do it as one atomic write, a reader would see
that zero-sized xattr between the O_CREAT and the write.
Of course, we could just hide zero-sized xattrs from the legacy
interfaces and avoid things like that, but another option is to say
that only the legacy interfaces give that particular atomicity
guarantee.
Since a////////b has known meaning, and lots of applications
play loose with '/', its really dangerous to treat the string as
special. We only get away with '.' and '..' because their behavior
was defined before many of y'all were born.
Yeah, I really don't think it's a good idea to play with "//".
POSIX does allow special semantics for a pathname with "//" at the
*beginning*, but even that has been very questionable (and Linux has
never supported it).
Linus
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-11 16:39:16
On Tue, Aug 11, 2020 at 09:09:36AM -0700, Linus Torvalds wrote:
On Tue, Aug 11, 2020 at 9:05 AM Al Viro [off-list ref] wrote:
quoted
Except that you suddenly see non-directory dentries get children.
And a lot of dcache-related logics needs to be changed if that
becomes possible.
Yeah, I think you'd basically need to associate a (dynamic)
mount-point to that path when you start doing O_ALT. Or something.
Whee... That's going to be non-workable for xattrs - fgetxattr()
needs to work after unlink(). And you'd obviously need to prevent
crossing into that sucker on normal lookups, which would add quite
a few interesting twists around the automount points.
I'm not saying it's not doable, but it won't be anywhere near
straightforward. And API semantics questions are still there...
On Tue, Aug 11, 2020 at 6:05 PM Linus Torvalds
[off-list ref] wrote:
and then people do "$(srctree)/". If you haven't seen that kind of
pattern where the pathname has two (or sometimes more!) slashes in the
middle, you've led a very sheltered life.
Oh, I have. That's why I opted for triple slashes, since that should
work most of the time even in those concatenated cases. And yes, I
know, most is not always, and this might just be hiding bugs, etc...
I think the pragmatic approach would be to try this and see how many
triple slash hits a normal workload gets and if it's reasonably low,
then hopefully that together with warnings for O_ALT would be enough.
(b) even if the new user space were to think about that, and remove
those (hah! when have you ever seen user space do that?), as Al
mentioned, the user *filesystem* might have pathnames with double
slashes as part of symlinks.
So now we'd have to make sure that when we traverse symlinks, that
O_ALT gets cleared.
That's exactly what I implemented in the proof of concept patch.
Which means that it's not a unified namespace
after all, because you can't make symlinks point to metadata.
I don't think that's a great deal. Also I think other limitations
would make sense:
- no mounts allowed under ///
- no ./.. resolution after ///
- no hardlinks
- no special files, just regular and directory
- no seeking (regular or dir)
cat my-file.tar/inside/the/archive.c
or similar.
Al has convinced me it's a horrible idea (and there you have a
non-ambiguous marker: the slash at the end of a pathname that
otherwise looks and acts as a non-directory)
Umm, can you remind me what's so horrible about that? Yeah, hard
linked directories are a no-no. But it doesn't have to be implemented
in a way to actually be a problem with hard links.
Thanks,
Miklos
From: Christian Brauner <hidden> Date: 2020-08-11 19:39:25
On Tue, Aug 11, 2020 at 09:05:22AM -0700, Linus Torvalds wrote:
On Tue, Aug 11, 2020 at 8:30 AM Miklos Szeredi [off-list ref] wrote:
quoted
What's the disadvantage of doing it with a single lookup WITH an enabling flag?
It's definitely not going to break anything, so no backward
compatibility issues whatsoever.
No backwards compatibility issues for existing programs, no.
But your suggestion is fundamentally ambiguous, and you most
definitely *can* hit that if people start using this in new programs.
Where does that "unified" pathname come from? It will be generated
from "base filename + metadata name" in user space, and
(a) the base filename might have double or triple slashes in it for
whatever reasons.
This is not some "made-up gotcha" thing - I see double slashes *all*
the time when we have things like Makefiles doing
srctree=../../src/
and then people do "$(srctree)/". If you haven't seen that kind of
pattern where the pathname has two (or sometimes more!) slashes in the
middle, you've led a very sheltered life.
(b) even if the new user space were to think about that, and remove
those (hah! when have you ever seen user space do that?), as Al
mentioned, the user *filesystem* might have pathnames with double
slashes as part of symlinks.
So now we'd have to make sure that when we traverse symlinks, that
O_ALT gets cleared. Which means that it's not a unified namespace
after all, because you can't make symlinks point to metadata.
Or we'd retroactively change the semantics of a symlink, and that _is_
a backwards compatibility issue. Not with old software, no, but it
changes the meaning of old symlinks!
So no, I don't think a unified namespace ends up working.
And I say that as somebody who actually loves the concept. Ask Al: I
have a few times pushed for "let's allow directory behavior on regular
files", so that you could do things like a tar-filesystem, and access
the contents of a tar-file by just doing
cat my-file.tar/inside/the/archive.c
or similar.
Al has convinced me it's a horrible idea (and there you have a
non-ambiguous marker: the slash at the end of a pathname that
otherwise looks and acts as a non-directory)
Putting my kernel hat down, putting my userspace hat on.
I'm looking at this from a potential user of this interface.
I'm not a huge fan of the metadata fd approach I'd much rather have a
dedicated system call rather than opening a side-channel metadata fd
that I can read binary data from. Maybe I'm alone in this but I was
under the impression that other users including Ian, Lennart, and Karel
have said on-list in some form that they would prefer this approach.
There are even patches for systemd and libmount, I thought?
But if we want to go down a completely different route then I'd prefer
if this metadata fd with "special semantics" did not in any way alter
the meaning of regular paths. This has the potential to cause a lot of
churn for userspace. I think having to play concatenation games in
shared libraries for mount information is a bad plan in addition to all
the issues you raised here.
Christian
From: Lennart Poettering <hidden> Date: 2020-08-11 19:40:22
On Di, 11.08.20 20:49, Miklos Szeredi (miklos@szeredi.hu) wrote:
On Tue, Aug 11, 2020 at 6:05 PM Linus Torvalds
[off-list ref] wrote:
quoted
and then people do "$(srctree)/". If you haven't seen that kind of
pattern where the pathname has two (or sometimes more!) slashes in the
middle, you've led a very sheltered life.
Oh, I have. That's why I opted for triple slashes, since that should
work most of the time even in those concatenated cases. And yes, I
know, most is not always, and this might just be hiding bugs, etc...
I think the pragmatic approach would be to try this and see how many
triple slash hits a normal workload gets and if it's reasonably low,
then hopefully that together with warnings for O_ALT would be enough.
There's no point. Userspace relies on the current meaning of triple
slashes. It really does.
I know many places in systemd where we might end up with a triple
slash. Here's a real-life example: some code wants to access the
cgroup attribute 'cgroup.controllers' of the root cgroup. It thus
generates the right path in the fs for it, which is the concatenation of
"/sys/fs/cgroup/" (because that's where cgroupfs is mounted), of "/"
(i.e. for the root cgroup) and of "/cgroup.controllers" (as that's the
file the attribute is exposed under).
And there you go:
"/sys/fs/cgroup/" + "/" + "/cgroup.controllers" → "/sys/fs/cgroup///cgroup.controllers"
This is a real-life thing. Don't break this please.
Lennart
--
Lennart Poettering, Berlin
From: Christian Brauner <hidden> Date: 2020-08-11 19:50:38
On Tue, Aug 11, 2020 at 09:31:05PM +0200, Lennart Poettering wrote:
On Di, 11.08.20 20:49, Miklos Szeredi (miklos@szeredi.hu) wrote:
quoted
On Tue, Aug 11, 2020 at 6:05 PM Linus Torvalds
[off-list ref] wrote:
quoted
and then people do "$(srctree)/". If you haven't seen that kind of
pattern where the pathname has two (or sometimes more!) slashes in the
middle, you've led a very sheltered life.
Oh, I have. That's why I opted for triple slashes, since that should
work most of the time even in those concatenated cases. And yes, I
know, most is not always, and this might just be hiding bugs, etc...
I think the pragmatic approach would be to try this and see how many
triple slash hits a normal workload gets and if it's reasonably low,
then hopefully that together with warnings for O_ALT would be enough.
There's no point. Userspace relies on the current meaning of triple
slashes. It really does.
I know many places in systemd where we might end up with a triple
slash. Here's a real-life example: some code wants to access the
cgroup attribute 'cgroup.controllers' of the root cgroup. It thus
generates the right path in the fs for it, which is the concatenation of
"/sys/fs/cgroup/" (because that's where cgroupfs is mounted), of "/"
(i.e. for the root cgroup) and of "/cgroup.controllers" (as that's the
file the attribute is exposed under).
And there you go:
"/sys/fs/cgroup/" + "/" + "/cgroup.controllers" → "/sys/fs/cgroup///cgroup.controllers"
This is a real-life thing. Don't break this please.
Taken from a log from a container:
lxc f4 20200810105815.742 TRACE cgfsng - cgroups/cgfsng.c:cg_legacy_handle_cpuset_hierarchy:552 - "cgroup.clone_children" was already set to "1"
lxc f4 20200810105815.742 WARN cgfsng - cgroups/cgfsng.c:mkdir_eexist_on_last:1152 - File exists - Failed to create directory "/sys/fs/cgroup/cpuset///lxc.monitor.f4"
lxc f4 20200810105815.743 INFO cgfsng - cgroups/cgfsng.c:cgfsng_monitor_create:1366 - The monitor process uses "lxc.monitor.f4" as cgroup
lxc f4 20200810105815.743 DEBUG storage - storage/storage.c:get_storage_by_name:211 - Detected rootfs type "dir"
lxc f4 20200810105815.743 TRACE cgfsng - cgroups/cgfsng.c:cg_legacy_handle_cpuset_hierarchy:552 - "cgroup.clone_children" was already set to "1"
lxc f4 20200810105815.743 WARN cgfsng - cgroups/cgfsng.c:mkdir_eexist_on_last:1152 - File exists - Failed to create directory "/sys/fs/cgroup/cpuset///lxc.payload.f4"
lxc f4 20200810105815.743 INFO cgfsng - cgroups/cgfsng.c:cgfsng_payload_create:1469 - The container process uses "lxc.payload.f4" as cgroup
lxc f4 20200810105815.744 TRACE start - start.c:lxc_spawn:1731 - Spawned container directly into target cgroup via cgroup2 fd 17
Christian
On Tue, Aug 11, 2020 at 6:17 PM Casey Schaufler [off-list ref] wrote:
Since a////////b has known meaning, and lots of applications
play loose with '/', its really dangerous to treat the string as
special. We only get away with '.' and '..' because their behavior
was defined before many of y'all were born.
So the founding fathers have set things in stone and now we can't
change it. Right?
Well that's how it looks... but let's think a little; we have '/' and
'\0' that can't be used in filenames. Also '.' and '..' are
prohibited names. It's not a trivial limitation, so applications are
probably not used to dumping binary data into file names. And that
means it's probably possible to find a fairly short combination that
is never used in practice (probably containing the "/." sequence).
Why couldn't we reserve such a combination now?
I have no idea how to find such it, but other than that, I see no
theoretical problem with extending the list of reserved filenames.
Thanks,
Miklos
On Tue, Aug 11, 2020 at 10:29 PM Miklos Szeredi [off-list ref] wrote:
On Tue, Aug 11, 2020 at 6:17 PM Casey Schaufler [off-list ref] wrote:
quoted
Since a////////b has known meaning, and lots of applications
play loose with '/', its really dangerous to treat the string as
special. We only get away with '.' and '..' because their behavior
was defined before many of y'all were born.
So the founding fathers have set things in stone and now we can't
change it. Right?
Well that's how it looks... but let's think a little; we have '/' and
'\0' that can't be used in filenames. Also '.' and '..' are
prohibited names. It's not a trivial limitation, so applications are
probably not used to dumping binary data into file names. And that
means it's probably possible to find a fairly short combination that
is never used in practice (probably containing the "/." sequence).
Why couldn't we reserve such a combination now?
This isn't just about finding a string that "is never used in
practice". There is userspace software that performs security checks
based on the precise semantics that paths have nowadays, and those
security checks will sometimes happily let you use arbitrary binary
garbage in path components as long as there's no '\0' or '/' in there
and the name isn't "." or "..", because that's just how paths work on
Linux.
If you change the semantics of path strings, you'd have to be
confident that the new semantics fit nicely with all the path
validation routines that exist scattered across userspace, and don't
expose new interfaces through file server software and setuid binaries
and so on.
I really don't like this idea.
On Tue, Aug 11, 2020 at 10:37 PM Jann Horn [off-list ref] wrote:
If you change the semantics of path strings, you'd have to be
confident that the new semantics fit nicely with all the path
validation routines that exist scattered across userspace, and don't
expose new interfaces through file server software and setuid binaries
and so on.
So that's where O_ALT comes in. If the application is consenting,
then that should prevent exploits. Or?
Thanks,
Miklos
From: Andy Lutomirski <luto@kernel.org> Date: 2020-08-11 21:18:05
On Tue, Aug 11, 2020 at 1:56 PM Miklos Szeredi [off-list ref] wrote:
On Tue, Aug 11, 2020 at 10:37 PM Jann Horn [off-list ref] wrote:
quoted
If you change the semantics of path strings, you'd have to be
confident that the new semantics fit nicely with all the path
validation routines that exist scattered across userspace, and don't
expose new interfaces through file server software and setuid binaries
and so on.
So that's where O_ALT comes in. If the application is consenting,
then that should prevent exploits. Or?
We're going to be at risk from libraries that want to use the new
O_ALT mechanism but are invoked by old code that passes traditional
Linux paths. Each library will have to sanitize paths, and some will
screw it up.
I much prefer Linus' variant where the final part of the extended path
is passed as a separate parameter.
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-11 21:20:28
On Tue, Aug 11, 2020 at 10:28:31PM +0200, Miklos Szeredi wrote:
On Tue, Aug 11, 2020 at 6:17 PM Casey Schaufler [off-list ref] wrote:
quoted
Since a////////b has known meaning, and lots of applications
play loose with '/', its really dangerous to treat the string as
special. We only get away with '.' and '..' because their behavior
was defined before many of y'all were born.
So the founding fathers have set things in stone and now we can't
change it. Right?
Right.
Well that's how it looks... but let's think a little; we have '/' and
'\0' that can't be used in filenames. Also '.' and '..' are
prohibited names. It's not a trivial limitation, so applications are
probably not used to dumping binary data into file names. And that
means it's probably possible to find a fairly short combination that
is never used in practice (probably containing the "/." sequence).
No, it is not. Miklos, get real - you will end up with obscure
pathname produced once in a while by a script fragment from hell
spewed out by crusty piece of awk buried in a piece of shit
makefile from hell (and you are lucky if it won't be an automake
output, while we are at it). Exercised only when some shipped
turd needs to be regenerated. Have you _ever_ tried to debug e.g.
gcc build problems? I have, and it's extremely unpleasant. Failures
tend to be obscure as hell, backtracking them through the makefiles
is a massive PITA and figuring out why said piece of awk produces
what it does...
I know what I would've done if the likely 5 hours of cursing everything
would have ended up with discovery that some luser had assumed that
surely, no sane software would ever generate this sequence of characters
in anything used as a pathname, and that for this reason I'm looking
forward to several more hours of playing with utterly revolting crap
to convince it to stay away from that sequence...
Why couldn't we reserve such a combination now?
I have no idea how to find such it, but other than that, I see no
theoretical problem with extending the list of reserved filenames.
On Tue, Aug 11, 2020 at 1:56 PM Miklos Szeredi [off-list ref] wrote:
So that's where O_ALT comes in. If the application is consenting,
then that should prevent exploits. Or?
If the application is consenting AND GETS IT RIGHT it should prevent exploits.
But that's a big deal.
Why not just do it the way I suggested? Then you don't have any of these issues.
Linus
On Tue, Aug 11, 2020 at 6:17 PM Casey Schaufler [off-list ref] wrote:
quoted
Since a////////b has known meaning, and lots of applications
play loose with '/', its really dangerous to treat the string as
special. We only get away with '.' and '..' because their behavior
was defined before many of y'all were born.
So the founding fathers have set things in stone and now we can't
change it. Right?
The founders did lots of things that, in retrospect, weren't
such great ideas, but that we have to live with.
Well that's how it looks... but let's think a little; we have '/' and
'\0' that can't be used in filenames. Also '.' and '..' are
prohibited names. It's not a trivial limitation, so applications are
probably not used to dumping binary data into file names.
Hee Hee. Back in the early days of UNIX (the 1970s) there was command
dsw(1) "delete from switches" because files with untypeible names where
unfortunately common. I would question the assertion that "applications
are not used to dumping binary data into file names", based on how
often I've wished we still had dsw(1).
And that
means it's probably possible to find a fairly short combination that
is never used in practice (probably containing the "/." sequence).
You'd think, but you'd be wrong. In the UNIX days we tried everything
from "..." to ".NO_HID." and there always arose a problem or two. Not
the least of which is that a "magic" pathname generated on an old system,
then mounted on a new system will never give you the results you want.
Why couldn't we reserve such a combination now?
I have no idea how to find such it, but other than that, I see no
theoretical problem with extending the list of reserved filenames.
You need a sequence that is never used in any language, and
that has never been used as a magic shell sequence. If you want
a fun story to tell over beers, look up how using the "@" as the
erase character on a TTY33 lead to it being used in email addresses.
From: David Howells <dhowells@redhat.com> Date: 2020-08-12 00:05:48
Linus Torvalds [off-list ref] wrote:
[ I missed the beginning of this discussion, so maybe this was already
suggested ]
Well, the start of it was my proposal of an fsinfo() system call. That at its
simplest takes an object reference (eg. a path) and an integer attribute ID (it
could use a string instead, I suppose, but it would mean a bunch of strcmps
instead of integer comparisons) and returns the value of the attribute. But I
allow you to do slightly more interesting things than that too.
Miklós seems dead-set against adding a system call specifically for this -
though he's proposed extending open in various ways and also proposed an
additional syscall, readfile(), that does the open+read+close all in one step.
I think also at some point, he (or maybe James?) proposed adding a new magic
filesystem mounted somewhere on proc (reflecting an open fd) that then had a
bunch of symlinks to somewhere in sysfs (reflecting a mount). The idea being
that you did something like:
fd = open("/path/to/object", O_PATH);
sprintf(name, "/proc/self/fds/%u/attr1", fd);
attrfd = open(name, O_RDONLY);
read(attrfd, buf1, sizeof(buf1));
close(attrfd);
sprintf(name, "/proc/self/fds/%u/attr2", fd);
attrfd = open(name, O_RDONLY);
read(attrfd, buf2, sizeof(buf2));
close(attrfd);
or:
sprintf(name, "/proc/self/fds/%u/attr1", fd);
readfile(name, buf1, sizeof(buf1));
sprintf(name, "/proc/self/fds/%u/attr2", fd);
readfile(name, buf2, sizeof(buf2));
and then "/proc/self/fds/12/attr2" might then be a symlink to, say,
"/sys/mounts/615/mount_attr".
Miklós's justification for this was that it could then be operated from a shell
script without the need for a utility - except that bash, at least, can't do
O_PATH opens.
James has proposed making fsconfig() able to retrieve attributes (though I'd
prefer to give it a sibling syscall that does the retrieval rather than making
fsconfig() do that too).
Please don't go down this path. You're proposing five syscalls - including
creating two file descriptors - to do what fsinfo() does in one.
Do you have a particular objection to adding a syscall specifically for
retrieving filesystem/VFS information?
-~-
Anyway, in case you're interested in what I want to get out of this - which is
the reason for it being posted in the first place:
(*) The ability to retrieve various attributes of a filesystem/superblock,
including information on:
- Filesystem features: Does it support things like hard links, user
quotas, direct I/O.
- Filesystem limits: What's the maximum size of a file, an xattr, a
directory; how many files can it support.
- Supported API features: What FS_IOC_GETFLAGS does it support? Which
can be set? Does it have Windows file attributes available? What
statx attributes are supported? What do the timestamps support?
What sort of case handling is done on filenames?
Note that for a lot of cases, this stuff is fixed and can just be memcpy'd
from rodata. Some of this is variable, however, in things like ext4 and
xfs, depending on, say, mkfs configuration. The situation is even more
complex with network filesystems as this may depend on the server they're
talking to.
But note also that some of this stuff might change file-to-file, even
within a superblock.
(*) The ability to retrieve attributes of a mount point, including information
on the flags, propagation settings and child lists.
(*) The ability to quickly retrieve a list of accessible mount point IDs,
with change event counters to permit userspace (eg. systemd) to quickly
determine if anything changed in the even of an overrun.
(*) The ability to find mounts/superblocks by mount ID. Paths are not unique
identifiers for mountpoints. You can stack multiple mounts on the same
directory, but a path only sees the top one.
(*) The ability to look inside a different mount namespace - one to which you
have a reference fd. This would allow a container manager to look inside
the container it is managing.
(*) The ability to expose filesystem-specific attributes. Network filesystems
can expose lists of servers and server addresses, for instance.
(*) The ability to use the object referenced to determine the namespace
(particularly the network namespace) to look in. The problem with looking
in, say, /proc/net/... is that it looks at current's net namespace -
whether or not the object of interest is in the same one.
(*) The ability to query the context attached to the fd obtained from
fsopen(). Such a context may not have a superblock attached to it yet or
may not be mounted yet.
The aim is to allow a container manager to supervise a mount being made in
a container. It kind of pairs with fsconfig() in that respect.
(*) The ability to query mount and superblock event counters to help a
watching process handle overrun in the notifications queue.
What I've done with fsinfo() is:
(*) Provided a number of ways to refer to the object to be queried (path,
dirfd+path, fd, mount ID - with others planned).
(*) Made it so that attibutes are referenced by a numeric ID to keep search
time minimal. Numeric IDs must be declared in uapi/linux/fsinfo.h.
(*) Made it so that the core does most of the work. Filesystems are given an
in-kernel buffer to copy into and don't get to see any userspace pointers.
(*) Made it so that values are not, by and large, encoded as text if it can be
avoided. Backward and forward compatibility on binary structs is handled
by the core. The filesystem just fills in the values in the UAPI struct
in the buffer. The core will zero-pad or truncate the data to match what
userspace asked for.
The UAPI struct must be declared in uapi/linux/fsinfo.h.
(*) Made it so that, for some attributes, the core will fill in the data as
best it can from what's available in the superblock, mount struct or mount
namespace. The filesystem can then amend this if it wants to.
(*) Made it so that attributes are typed. The types are few: string, struct,
list of struct, opaque. Structs are extensible: the length is the
version, a new version is required to be a superset of the old version and
excess requestage is simply cleared by the kernel.
Information about the type of an attribute can be queried by fsinfo().
What I want to avoid:
(*) Adding another magic filesystem.
(*) Adding symlinks from proc to sysfs.
(*) Having to use open to get an attribute.
(*) Having to use multiple opens to get an attribute.
(*) Having to pathwalk to get to the attribute from the object being queried.
(*) Allocating another O_ open flag for this.
(*) Avoidable text encoding and decoding.
(*) Letting the filesystem access the userspace buffer.
Note that I'm not against splitting fsinfo() into a set of sibling syscalls if
that makes it more palatable, or even against using strings for the attribute
IDs, though I'd prefer to avoid the strcmps.
David
From: Ian Kent <raven@themaw.net> Date: 2020-08-12 00:53:57
On Tue, 2020-08-11 at 21:39 +0200, Christian Brauner wrote:
On Tue, Aug 11, 2020 at 09:05:22AM -0700, Linus Torvalds wrote:
quoted
On Tue, Aug 11, 2020 at 8:30 AM Miklos Szeredi [off-list ref]
wrote:
quoted
What's the disadvantage of doing it with a single lookup WITH an
enabling flag?
It's definitely not going to break anything, so no backward
compatibility issues whatsoever.
No backwards compatibility issues for existing programs, no.
But your suggestion is fundamentally ambiguous, and you most
definitely *can* hit that if people start using this in new
programs.
Where does that "unified" pathname come from? It will be generated
from "base filename + metadata name" in user space, and
(a) the base filename might have double or triple slashes in it
for
whatever reasons.
This is not some "made-up gotcha" thing - I see double slashes
*all*
the time when we have things like Makefiles doing
srctree=../../src/
and then people do "$(srctree)/". If you haven't seen that kind of
pattern where the pathname has two (or sometimes more!) slashes in
the
middle, you've led a very sheltered life.
(b) even if the new user space were to think about that, and
remove
those (hah! when have you ever seen user space do that?), as Al
mentioned, the user *filesystem* might have pathnames with double
slashes as part of symlinks.
So now we'd have to make sure that when we traverse symlinks, that
O_ALT gets cleared. Which means that it's not a unified namespace
after all, because you can't make symlinks point to metadata.
Or we'd retroactively change the semantics of a symlink, and that
_is_
a backwards compatibility issue. Not with old software, no, but it
changes the meaning of old symlinks!
So no, I don't think a unified namespace ends up working.
And I say that as somebody who actually loves the concept. Ask Al:
I
have a few times pushed for "let's allow directory behavior on
regular
files", so that you could do things like a tar-filesystem, and
access
the contents of a tar-file by just doing
cat my-file.tar/inside/the/archive.c
or similar.
Al has convinced me it's a horrible idea (and there you have a
non-ambiguous marker: the slash at the end of a pathname that
otherwise looks and acts as a non-directory)
Putting my kernel hat down, putting my userspace hat on.
I'm looking at this from a potential user of this interface.
I'm not a huge fan of the metadata fd approach I'd much rather have a
dedicated system call rather than opening a side-channel metadata fd
that I can read binary data from. Maybe I'm alone in this but I was
under the impression that other users including Ian, Lennart, and
Karel
have said on-list in some form that they would prefer this approach.
There are even patches for systemd and libmount, I thought?
Not quite sure what you mean here.
Karel (with some contributions by me) has implemented the interfaces
for David's mount notifications and fsinfo() call in libmount. We
still have a little more to do on that.
I also have a systemd implementation that uses these libmount features
for mount table handling that works quite well, with a couple more
things to do to complete it, that Lennart has done an initial review
for.
It's no secret that I don't like the proc file system in general
but it is really useful for many things, that's just the way it
is.
Ian
On Tue, Aug 11, 2020 at 11:19 PM Linus Torvalds
[off-list ref] wrote:
On Tue, Aug 11, 2020 at 1:56 PM Miklos Szeredi [off-list ref] wrote:
quoted
So that's where O_ALT comes in. If the application is consenting,
then that should prevent exploits. Or?
If the application is consenting AND GETS IT RIGHT it should prevent exploits.
But that's a big deal.
Why not just do it the way I suggested? Then you don't have any of these issues.
Will do.
I just want to understand the reasons why a unified namespace is
completely out of the question. And I won't accept "it's just fugly"
or "it's the way it's always been done, so don't change it". Those
are not good reasons.
Oh, I'm used to these "fights", had them all along. In hindsight I
should have accepted others' advice in some of the cases, but in
others that big argument turned out to be a complete non-issue. One
such being inode and dentry duplication in the overlayfs case vs.
in-built stacking in the union-mount case. There were a lot of issues
with overlayfs, that's true, but dcache/icache size has NEVER actually
been reported as a problem.
While Al has a lot of experience, it's hard to accept all that
anecdotal evidence just because he says it. Your worries are also
just those: worries. They may turn out to be an issue or they may
not.
Anyway, starting with just introducing the alt namespace without
unification seems to be a good first step. If that turns out to be
workable, we can revisit unification later.
Thanks,
Miklos
Please don't go down this path. You're proposing five syscalls - including
creating two file descriptors - to do what fsinfo() does in one.
So what? People argued against readfile() for exactly the opposite of
reasons, even though that's a lot less specialized than fsinfo().
Worried about performance? Io-uring will allow you to do all those
five syscalls (or many more) with just one I/O submission.
Thanks,
Miklos
From: David Howells <dhowells@redhat.com> Date: 2020-08-12 08:29:18
Miklos Szeredi [off-list ref] wrote:
Worried about performance? Io-uring will allow you to do all those
five syscalls (or many more) with just one I/O submission.
io_uring isn't going to help here. We're talking about synchronous reads.
AIUI, you're adding a couple more syscalls to the list and running stuff in a
side thread to save the effort of going in and out of the kernel five times.
But you still have to pay the set up/tear down costs on the fds and do the
pathwalks. io_uring doesn't magically make that cost disappear.
io_uring also requires resources such as a kernel accessible ring buffer to
make it work.
You're proposing making everything else more messy just to avoid a dedicated
syscall. Could you please set out your reasoning for that?
David
On Wed, Aug 12, 2020 at 10:29 AM David Howells [off-list ref] wrote:
Miklos Szeredi [off-list ref] wrote:
quoted
Worried about performance? Io-uring will allow you to do all those
five syscalls (or many more) with just one I/O submission.
io_uring isn't going to help here. We're talking about synchronous reads.
AIUI, you're adding a couple more syscalls to the list and running stuff in a
side thread to save the effort of going in and out of the kernel five times.
But you still have to pay the set up/tear down costs on the fds and do the
pathwalks. io_uring doesn't magically make that cost disappear.
io_uring also requires resources such as a kernel accessible ring buffer to
make it work.
You're proposing making everything else more messy just to avoid a dedicated
syscall. Could you please set out your reasoning for that?
a) A dedicated syscall with a complex binary API is a non-trivial
maintenance burden.
b) The awarded performance boost is not warranted for the use cases it
is designed for.
Thanks,
Miklos
From: Steven Whitehouse <hidden> Date: 2020-08-12 09:43:57
Hi,
On 12/08/2020 09:37, Miklos Szeredi wrote:
[snip]
b) The awarded performance boost is not warranted for the use cases it
is designed for.
Thanks,
Miklos
This is a key point. One of the main drivers for this work is the
efficiency improvement for large numbers of mounts. Ian and Karel have
already provided performance measurements showing a significant benefit
compared with what we have today. If you want to propose this
alternative interface then you need to show that it can sustain similar
levels of performance, otherwise it doesn't solve the problem. So
performance numbers here would be helpful.
Also - I may have missed this earlier in the discussion, what are the
atomicity guarantees with this proposal? This is the other key point for
the API, so it would be good to see that clearly stated (i.e. how does
one use it in combination with the notifications to provide an up to
date, consistent view of the kernel's mounts)
Steve.
On Wed, Aug 12, 2020 at 11:43 AM Steven Whitehouse [off-list ref] wrote:
Hi,
On 12/08/2020 09:37, Miklos Szeredi wrote:
[snip]
quoted
b) The awarded performance boost is not warranted for the use cases it
is designed for.
This is a key point. One of the main drivers for this work is the
efficiency improvement for large numbers of mounts. Ian and Karel have
already provided performance measurements showing a significant benefit
compared with what we have today. If you want to propose this
alternative interface then you need to show that it can sustain similar
levels of performance, otherwise it doesn't solve the problem. So
performance numbers here would be helpful.
Definitely. Will measure performance with the interface which Linus proposed.
I'm not worried, though; the problem with the previous interface was
that it resulted in the complete mount table being re-parsed on each
individual event resulting in quadratic behavior. This doesn't affect
any interface that can query individual mount/superblock objects.
Also - I may have missed this earlier in the discussion, what are the
atomicity guarantees with this proposal? This is the other key point for
the API, so it would be good to see that clearly stated (i.e. how does
one use it in combination with the notifications to provide an up to
date, consistent view of the kernel's mounts)
fsinfo(2) provides version counters on mount and superblock objects to
verify consistency of returned data, since not all data is returned in
a single call. Same method could be used with the open/read based
interface to verify consistency in case multiple attributes/attribute
groups need to be queried.
Thanks,
Miklos
From: Karel Zak <kzak@redhat.com> Date: 2020-08-12 10:14:22
On Tue, Aug 11, 2020 at 08:20:24AM -0700, Linus Torvalds wrote:
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
I have thought we want to replace mountinfo to reduce overhead. If I
understand your idea than we will need openat()+read()+close() for
each attribute? Sounds like a pretty expensive interface.
The question is also how consistent results you get if you will read
information about the same mountpoint by multiple openat()+read()+close()
calls.
For example, by fsinfo(FSINFO_ATTR_MOUNT_TOPOLOGY) you get all
mountpoint propagation setting and relations by one syscall, with
your idea you will read parent, slave and flags by multiple read() and
without any lock. Sounds like you can get a mess if someone moves or
reconfigure the mountpoint or so.
openat(O_ALT) seems elegant at first glance, but it will be necessary to
provide a richer (complex) answers by read() to reduce overhead and
to make it more consistent for userspace.
It would be also nice to avoid some strings formatting and separators
like we use in the current mountinfo.
I can imagine multiple values separated by binary header (like we already
have for watch_notification, inotify, etc):
fd = openat(fd, "mountinfo", O_ALT);
sz = read(fd, buf, BUFSZ);
p = buf;
while (sz) {
struct alt_metadata *alt = (struct alt_metadata *) p;
char *varname = alt->name;
char *data = alt->data;
int len = alt->len;
sz -= len;
p += len;
}
Karel
--
Karel Zak [off-list ref]
http://karelzak.blogspot.com
From: Karel Zak <kzak@redhat.com> Date: 2020-08-12 11:28:42
On Wed, Aug 12, 2020 at 12:04:14PM +0200, Miklos Szeredi wrote:
On Wed, Aug 12, 2020 at 11:43 AM Steven Whitehouse [off-list ref] wrote:
quoted
Hi,
On 12/08/2020 09:37, Miklos Szeredi wrote:
[snip]
quoted
b) The awarded performance boost is not warranted for the use cases it
is designed for.
quoted
This is a key point. One of the main drivers for this work is the
efficiency improvement for large numbers of mounts. Ian and Karel have
already provided performance measurements showing a significant benefit
compared with what we have today. If you want to propose this
alternative interface then you need to show that it can sustain similar
levels of performance, otherwise it doesn't solve the problem. So
performance numbers here would be helpful.
Definitely. Will measure performance with the interface which Linus proposed.
The proposal is based on paths and open(), how do you plan to deal
with mount IDs? David's fsinfo() allows to ask for mount info by mount
ID and it works well with mount notification where you get the ID. The
collaboration with notification interface is critical for our use-cases.
Karel
--
Karel Zak [off-list ref]
http://karelzak.blogspot.com
On Wed, Aug 12, 2020 at 1:28 PM Karel Zak [off-list ref] wrote:
The proposal is based on paths and open(), how do you plan to deal
with mount IDs? David's fsinfo() allows to ask for mount info by mount
ID and it works well with mount notification where you get the ID. The
collaboration with notification interface is critical for our use-cases.
One would use the notification to keep an up to date set of attributes
for each watched mount, right?
That presumably means the mount ID <-> mount path mapping already
exists, which means it's just possible to use the open(mount_path,
O_PATH) to obtain the base fd.
If that assumption is not true, we could add a new interface for
opening the root of the mount by ID. Fsinfo uses the dfd as a root
for checking connectivity and the filename as the mount ID + a special
flag indicating that it's not "dfd + path" we are dealing with but
"rootfd + mntid". That sort of semantic multiplexing is quite ugly
and I wouldn't suggest doing that with openat(2).
A new syscall that returns an fd pointing to the root of the mount
might be the best solution:
int open_mount(int root_fd, u64 mntid, int flags);
Yeah, yeah this is adding just another syscall interface, but notice how:
a) it does one simple thing, no multiplexing at all
b) is general purpose, and could be used for example in conjunction
with open_by_handle_at(2), that also requires an fd pointing to a
mount.
Thanks,
Miklos
From: David Howells <dhowells@redhat.com> Date: 2020-08-12 13:06:40
Miklos Szeredi [off-list ref] wrote:
That presumably means the mount ID <-> mount path mapping already
exists, which means it's just possible to use the open(mount_path,
O_PATH) to obtain the base fd.
No, you can't. A path more correspond to multiple mounts stacked on top of
each other, e.g.:
mount -t tmpfs none /mnt
mount -t tmpfs none /mnt
mount -t tmpfs none /mnt
Now you have three co-located mounts and you can't use the path to
differentiate them. I think this might be an issue in autofs, but Ian would
need to comment on that.
David
On Wed, Aug 12, 2020 at 12:14 PM Karel Zak [off-list ref] wrote:
For example, by fsinfo(FSINFO_ATTR_MOUNT_TOPOLOGY) you get all
mountpoint propagation setting and relations by one syscall,
That's just an arbitrary grouping of attributes.
You said yourself, that what's really needed is e.g. consistent
snapshot of a complete mount tree topology. And to get the complete
topology FSINFO_ATTR_MOUNT_TOPOLOGY and FSINFO_ATTR_MOUNT_CHILDREN are
needed for *each* individual mount. The topology can obviously change
between those calls.
So there's no fundamental difference between getting individual
attributes or getting attribute groups in this respect.
It would be also nice to avoid some strings formatting and separators
like we use in the current mountinfo.
I think quoting non-printable is okay.
I can imagine multiple values separated by binary header (like we already
have for watch_notification, inotify, etc):
Adding a few generic binary interfaces is okay. Adding many
specialized binary interfaces is a PITA.
Thanks,
Miklos
From: David Howells <dhowells@redhat.com> Date: 2020-08-12 13:33:27
Miklos Szeredi [off-list ref] wrote:
You said yourself, that what's really needed is e.g. consistent
snapshot of a complete mount tree topology. And to get the complete
topology FSINFO_ATTR_MOUNT_TOPOLOGY and FSINFO_ATTR_MOUNT_CHILDREN are
needed for *each* individual mount.
That's not entirely true.
FSINFO_ATTR_MOUNT_ALL can be used instead of FSINFO_ATTR_MOUNT_CHILDREN if you
want to scan an entire subtree in one go. It returns the same record type.
The result from ALL/CHILDREN includes sufficient information to build the
tree. That only requires the parent ID. All the rest of the information
TOPOLOGY exposes is to do with propagation.
Now, granted, I didn't include all of the topology info in the records
returned by ALL/CHILDREN because I don't expect it to change very often. But
you can check the event counter supplied with each record to see if it might
have changed - and then call TOPOLOGY on the ones that changed.
If it simplifies life, I could add the propagation info into ALL/CHILDREN so
that you only need to call ALL to scan everything. It requires larger
buffers, however.
Adding a few generic binary interfaces is okay. Adding many
specialized binary interfaces is a PITA.
Text interfaces are also a PITA, especially when you may get multiple pieces
of information returned in one buffer and especially when you throw in
character escaping. Of course, we can do it - and we do do it all over - but
that doesn't make it efficient.
David
From: David Howells <dhowells@redhat.com> Date: 2020-08-12 13:54:59
Linus Torvalds [off-list ref] wrote:
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
What is it going to walk through? You need to end up with an inode and dentry
from somewhere.
It sounds like this would have to open up a procfs-like magic filesystem, and
walk into it. But how would that actually work? Would you create a new
superblock each time you do this, labelled with the starting object (say the
dentry for "foo/bar" in this case), and then walk from the root?
An alternative, maybe, could be to make a new dentry type, say, and include it
in the superblock of the object being queried - and let the filesystems deal
with it. That would mean that non-dir dentries would then have virtual
children. You could then even use this to implement resource forks...
Another alternative would be to note O_ALT and then skip pathwalk entirely,
but just use the name as a key to the attribute, creating an anonfd to read
it. But then why use openat() at all? You could instead do:
metadatafd = openmeta(fd, "metadataname");
and save the page flag. You could even merge the two opens and do:
metadatafd = openmeta("foo/bar", "metadataname");
Why not even combine this with Miklos's readfile() idea:
readmeta(AT_FDCWD, "foo/bar", "metadataname", buf, sizeof(buf));
and we're now down to one syscall and no fds and you don't even need a magic
filesystem to make it work.
There's another consideration too: Paths are not unique handles to mounts.
It's entirely possible to have colocated mounts. We need to be able to query
all the mounts on a mountpoint.
David
On Wed, Aug 12, 2020 at 3:33 PM David Howells [off-list ref] wrote:
Miklos Szeredi [off-list ref] wrote:
quoted
You said yourself, that what's really needed is e.g. consistent
snapshot of a complete mount tree topology. And to get the complete
topology FSINFO_ATTR_MOUNT_TOPOLOGY and FSINFO_ATTR_MOUNT_CHILDREN are
needed for *each* individual mount.
That's not entirely true.
FSINFO_ATTR_MOUNT_ALL can be used instead of FSINFO_ATTR_MOUNT_CHILDREN if you
want to scan an entire subtree in one go. It returns the same record type.
The result from ALL/CHILDREN includes sufficient information to build the
tree. That only requires the parent ID. All the rest of the information
TOPOLOGY exposes is to do with propagation.
Now, granted, I didn't include all of the topology info in the records
returned by ALL/CHILDREN because I don't expect it to change very often. But
you can check the event counter supplied with each record to see if it might
have changed - and then call TOPOLOGY on the ones that changed.
IDGI, you have all these interfaces but how will they be used?
E.g. one wants to build a consistent topology together with
propagation and attributes. That would start with
FSINFO_ATTR_MOUNT_ALL, then iterate the given mounts calling
FSINFO_ATTR_MOUNT_INFO and FSINFO_ATTR_MOUNT_TOPOLOGY for each. Then
when done, check the subtree notification counter with
FSINFO_ATTR_MOUNT_INFO on the top one to see if anything has changed
in the meantime. If it has, the whole process needs to be restarted
to see which has been changed (unless notification is also enabled).
How does the atomicity of FSINFO_ATTR_MOUNT_ALL help with that? The
same could be done with just FSINFO_ATTR_MOUNT_CHILDREN.
And more importantly does level of consistency matter at all? There's
no such thing for directory trees, why are mount trees different in
this respect?
Text interfaces are also a PITA, especially when you may get multiple pieces
of information returned in one buffer and especially when you throw in
character escaping. Of course, we can do it - and we do do it all over - but
that doesn't make it efficient.
Agreed. The format of text interfaces matters very much.
Thanks,
Miklos
On Wed, Aug 12, 2020 at 3:54 PM David Howells [off-list ref] wrote:
Linus Torvalds [off-list ref] wrote:
quoted
IOW, if you do something more along the lines of
fd = open(""foo/bar", O_PATH);
metadatafd = openat(fd, "metadataname", O_ALT);
it might be workable.
What is it going to walk through? You need to end up with an inode and dentry
from somewhere.
It sounds like this would have to open up a procfs-like magic filesystem, and
walk into it. But how would that actually work? Would you create a new
superblock each time you do this, labelled with the starting object (say the
dentry for "foo/bar" in this case), and then walk from the root?
An alternative, maybe, could be to make a new dentry type, say, and include it
in the superblock of the object being queried - and let the filesystems deal
with it. That would mean that non-dir dentries would then have virtual
children. You could then even use this to implement resource forks...
Another alternative would be to note O_ALT and then skip pathwalk entirely,
but just use the name as a key to the attribute, creating an anonfd to read
it. But then why use openat() at all? You could instead do:
metadatafd = openmeta(fd, "metadataname");
and save the page flag. You could even merge the two opens and do:
metadatafd = openmeta("foo/bar", "metadataname");
Why not even combine this with Miklos's readfile() idea:
readmeta(AT_FDCWD, "foo/bar", "metadataname", buf, sizeof(buf));
And writemeta() and createmeta() and readdirmeta() and ...
The point is that generic operations already exist and no need to add
new, specialized ones to access metadata.
Thanks,
Miklos
From: David Howells <dhowells@redhat.com> Date: 2020-08-12 14:24:05
Miklos Szeredi [off-list ref] wrote:
The point is that generic operations already exist and no need to add
new, specialized ones to access metadata.
open and read already exist, yes, but the metadata isn't currently in
convenient inodes and dentries that you can just walk through. So you're
going to end up with a specialised filesystem instead, I suspect. Basically,
it's the same as your do-everything-through-/proc/self/fds/ approach.
And it's going to be heavier. I don't know if you're planning on creating a
superblock each time you do an O_ALT open, but you will end up creating some
inodes, dentries and a file - even before you get to the reading bit.
David
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-12 14:40:17
On Wed, Aug 12, 2020 at 09:23:23AM +0200, Miklos Szeredi wrote:
Anyway, starting with just introducing the alt namespace without
unification seems to be a good first step. If that turns out to be
workable, we can revisit unification later.
Start with coming up with answers to the questions on semantics
upthread. To spare you the joy of digging through the branches
of that thread, how's that for starters?
"Can those suckers be passed to
...at() as starting points? Can they be bound in namespace?
Can something be bound *on* them? What do they have for inodes
and what maintains their inumbers (and st_dev, while we are at
it)? Can _they_ have secondaries like that (sensu Swift)?
Is that a flat space, or can they be directories?"
On Wed, Aug 12, 2020 at 4:40 PM Al Viro [off-list ref] wrote:
On Wed, Aug 12, 2020 at 09:23:23AM +0200, Miklos Szeredi wrote:
quoted
Anyway, starting with just introducing the alt namespace without
unification seems to be a good first step. If that turns out to be
workable, we can revisit unification later.
Start with coming up with answers to the questions on semantics
upthread. To spare you the joy of digging through the branches
of that thread, how's that for starters?
"Can those suckers be passed to
...at() as starting points?
No.
Can they be bound in namespace?
No.
Can something be bound *on* them?
No.
What do they have for inodes
and what maintains their inumbers (and st_dev, while we are at
it)?
Irrelevant. Can be some anon dev + shared inode.
The only attribute of an attribute that I can think of that makes
sense would be st_size, but even that is probably unimportant.
Can _they_ have secondaries like that (sensu Swift)?
Reference?
Is that a flat space, or can they be directories?"
Yes it has a directory tree. But you can't mkdir, rename, link,
symlink, etc on anything in there.
Thanks,
Miklos
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-12 15:08:36
On Wed, Aug 12, 2020 at 04:46:20PM +0200, Miklos Szeredi wrote:
quoted
"Can those suckers be passed to
...at() as starting points?
No.
Lovely. And what of fchdir() to those? Are they all non-directories?
Because the starting point of ...at() can be simulated that way...
quoted
Can they be bound in namespace?
No.
quoted
Can something be bound *on* them?
No.
quoted
What do they have for inodes
and what maintains their inumbers (and st_dev, while we are at
it)?
Irrelevant. Can be some anon dev + shared inode.
The only attribute of an attribute that I can think of that makes
sense would be st_size, but even that is probably unimportant.
quoted
Can _they_ have secondaries like that (sensu Swift)?
Reference?
http://www.online-literature.com/swift/3515/
So, naturalists observe, a flea
Has smaller fleas that on him prey;
And these have smaller still to bite 'em,
And so proceed ad infinitum.
of course ;-)
IOW, can the things in those trees have secondary trees on them, etc.?
Not "will they have it in your originally intended use?" - "do we need
the architecture of the entire thing to be capable to deal with that?"
quoted
Is that a flat space, or can they be directories?"
Yes it has a directory tree. But you can't mkdir, rename, link,
symlink, etc on anything in there.
That kills the "shared inode" part - you'll get deadlocks from
hell that way. "Can't mkdir" doesn't save you from that. BTW,
what of unlink()? If the tree shape is not a hardwired constant,
you get to decide how it's initially populated...
Next: what will that tree be attached to? As in, "what's the parent
of its root"? And while we are at it, what will be the struct mount
used with those - same as the original file, something different
attached to it, something created on the fly for each pathwalk and
lazy-umounted? And see above re fchdir() - if they can be directories,
it's very much in the game.
On Wed, Aug 12, 2020 at 5:08 PM Al Viro [off-list ref] wrote:
On Wed, Aug 12, 2020 at 04:46:20PM +0200, Miklos Szeredi wrote:
quoted
quoted
"Can those suckers be passed to
...at() as starting points?
No.
Lovely. And what of fchdir() to those?
Not allowed.
Are they all non-directories?
Because the starting point of ...at() can be simulated that way...
quoted
quoted
Can they be bound in namespace?
No.
quoted
Can something be bound *on* them?
No.
quoted
What do they have for inodes
and what maintains their inumbers (and st_dev, while we are at
it)?
Irrelevant. Can be some anon dev + shared inode.
The only attribute of an attribute that I can think of that makes
sense would be st_size, but even that is probably unimportant.
quoted
Can _they_ have secondaries like that (sensu Swift)?
Reference?
http://www.online-literature.com/swift/3515/
So, naturalists observe, a flea
Has smaller fleas that on him prey;
And these have smaller still to bite 'em,
And so proceed ad infinitum.
of course ;-)
IOW, can the things in those trees have secondary trees on them, etc.?
Not "will they have it in your originally intended use?" - "do we need
the architecture of the entire thing to be capable to deal with that?"
No.
quoted
quoted
Is that a flat space, or can they be directories?"
Yes it has a directory tree. But you can't mkdir, rename, link,
symlink, etc on anything in there.
That kills the "shared inode" part - you'll get deadlocks from
hell that way.
No. The shared inode is not for lookup, just for the open file.
"Can't mkdir" doesn't save you from that. BTW,
what of unlink()? If the tree shape is not a hardwired constant,
you get to decide how it's initially populated...
Next: what will that tree be attached to? As in, "what's the parent
of its root"? And while we are at it, what will be the struct mount
used with those - same as the original file, something different
attached to it, something created on the fly for each pathwalk and
lazy-umounted? And see above re fchdir() - if they can be directories,
it's very much in the game.
Why does it have to have a struct mount? It does not have to use
dentry/mount based path lookup.
Thanks,
Miklos
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-12 16:34:07
On Wed, Aug 12, 2020 at 05:13:14PM +0200, Miklos Szeredi wrote:
quoted
Lovely. And what of fchdir() to those?
Not allowed.
Not allowed _how_? Existing check is "is it a directory"; what do you
propose? IIRC, you've mentioned using readdir() in that context, so
it's not that you only allow to open the leaves there.
quoted
quoted
quoted
Is that a flat space, or can they be directories?"
Yes it has a directory tree. But you can't mkdir, rename, link,
symlink, etc on anything in there.
That kills the "shared inode" part - you'll get deadlocks from
hell that way.
No. The shared inode is not for lookup, just for the open file.
Bloody hell... So what inodes are you using for lookups? And that
thing you would be passing to readdir() - what inode will _that_ have?
quoted
Next: what will that tree be attached to? As in, "what's the parent
of its root"? And while we are at it, what will be the struct mount
used with those - same as the original file, something different
attached to it, something created on the fly for each pathwalk and
lazy-umounted? And see above re fchdir() - if they can be directories,
it's very much in the game.
Why does it have to have a struct mount? It does not have to use
dentry/mount based path lookup.
What the fuck? So we suddenly get an additional class of objects
serving as kinda-sorta analogues of dentries *AND* now struct file
might refer to that instead of a dentry/mount pair - all on the VFS
level? And so do all the syscalls you want to allow for such "pathnames"?
Sure, that avoids all questions about dcache interactions - by growing
a replacement layer and making just about everything in fs/namei.c,
fs/open.c, etc. special-case the handling of that crap.
But yes, the syscall-level interface will be simple. Wonderful.
I really hope that's not what you have in mind, though.
On Wed, Aug 12, 2020 at 6:33 PM Al Viro [off-list ref] wrote:
On Wed, Aug 12, 2020 at 05:13:14PM +0200, Miklos Szeredi wrote:
quoted
Why does it have to have a struct mount? It does not have to use
dentry/mount based path lookup.
What the fuck? So we suddenly get an additional class of objects
serving as kinda-sorta analogues of dentries *AND* now struct file
might refer to that instead of a dentry/mount pair - all on the VFS
level? And so do all the syscalls you want to allow for such "pathnames"?
The only syscall I'd want to allow is open, everything else would be
on the open files themselves.
file->f_path can refer to an anon mount/inode, the real object is
referred to by file->private_data.
The change to namei.c would be on the order of ~10 lines. No other
parts of the VFS would be affected. Maybe I'm optimistic; we'll
see...
Now off to something completely different. Back on Tuesday.
Thanks,
Miklos
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-12 17:39:34
On Wed, Aug 12, 2020 at 07:16:37PM +0200, Miklos Szeredi wrote:
On Wed, Aug 12, 2020 at 6:33 PM Al Viro [off-list ref] wrote:
quoted
On Wed, Aug 12, 2020 at 05:13:14PM +0200, Miklos Szeredi wrote:
quoted
quoted
Why does it have to have a struct mount? It does not have to use
dentry/mount based path lookup.
What the fuck? So we suddenly get an additional class of objects
serving as kinda-sorta analogues of dentries *AND* now struct file
might refer to that instead of a dentry/mount pair - all on the VFS
level? And so do all the syscalls you want to allow for such "pathnames"?
The only syscall I'd want to allow is open, everything else would be
on the open files themselves.
file->f_path can refer to an anon mount/inode, the real object is
referred to by file->private_data.
The change to namei.c would be on the order of ~10 lines. No other
parts of the VFS would be affected.
If some of the things you open are directories (and you *have* said that
directories will be among those just upthread, and used references to
readdir() as argument in favour of your approach elsewhere in the thread),
you will have to do something about fchdir(). And that's the least of
the issues.
Maybe I'm optimistic; we'll
see...
Now off to something completely different. Back on Tuesday.
... after the window closes. You know, it's really starting to look
like rather nasty tactical games...
On Tue, Aug 11, 2020 at 5:05 PM David Howells [off-list ref] wrote:
Well, the start of it was my proposal of an fsinfo() system call.
Ugh. Ok, it's that thing.
This all seems *WAY* over-designed - both your fsinfo and Miklos' version.
What's wrong with fstatfs()? All the extra magic metadata seems to not
really be anything people really care about.
What people are actually asking for seems to be some unique mount ID,
and we have 16 bytes of spare information in 'struct statfs64'.
All the other fancy fsinfo stuff seems to be "just because", and like
complete overdesign.
Let's not add system calls just because we can.
Linus
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-12 18:33:45
On Wed, Aug 12, 2020 at 06:39:11PM +0100, Al Viro wrote:
On Wed, Aug 12, 2020 at 07:16:37PM +0200, Miklos Szeredi wrote:
quoted
On Wed, Aug 12, 2020 at 6:33 PM Al Viro [off-list ref] wrote:
quoted
On Wed, Aug 12, 2020 at 05:13:14PM +0200, Miklos Szeredi wrote:
quoted
quoted
Why does it have to have a struct mount? It does not have to use
dentry/mount based path lookup.
What the fuck? So we suddenly get an additional class of objects
serving as kinda-sorta analogues of dentries *AND* now struct file
might refer to that instead of a dentry/mount pair - all on the VFS
level? And so do all the syscalls you want to allow for such "pathnames"?
The only syscall I'd want to allow is open, everything else would be
on the open files themselves.
file->f_path can refer to an anon mount/inode, the real object is
referred to by file->private_data.
The change to namei.c would be on the order of ~10 lines. No other
parts of the VFS would be affected.
If some of the things you open are directories (and you *have* said that
directories will be among those just upthread, and used references to
readdir() as argument in favour of your approach elsewhere in the thread),
you will have to do something about fchdir(). And that's the least of
the issues.
BTW, what would such opened files look like from /proc/*/fd/* POV? And
what would happen if you walk _through_ that symlink, with e.g. ".."
following it? Or with names of those attributes, for that matter...
What about a normal open() of such a sucker? It won't know where to
look for your ->private_data...
FWIW, you keep refering to regularity of this stuff from the syscall
POV, but it looks like you have no real idea of what subset of the
things available for normal descriptors will be available for those.
From: Steven Whitehouse <hidden> Date: 2020-08-12 19:34:31
Hi,
On 12/08/2020 19:18, Linus Torvalds wrote:
On Tue, Aug 11, 2020 at 5:05 PM David Howells [off-list ref] wrote:
quoted
Well, the start of it was my proposal of an fsinfo() system call.
Ugh. Ok, it's that thing.
This all seems *WAY* over-designed - both your fsinfo and Miklos' version.
What's wrong with fstatfs()? All the extra magic metadata seems to not
really be anything people really care about.
What people are actually asking for seems to be some unique mount ID,
and we have 16 bytes of spare information in 'struct statfs64'.
All the other fancy fsinfo stuff seems to be "just because", and like
complete overdesign.
Let's not add system calls just because we can.
Linus
The point of this is to give us the ability to monitor mounts from
userspace. The original inspiration was rtnetlink, in that we need a
"dump" operation to give us a snapshot of the current mount state, plus
then a stream of events which allow us to keep that state updated. The
tricky question is what happens in case of overflow of the events queue,
and just like netlink, that needs a resync of the current state to fix
that, since we can't block mounts, of course.
The fsinfo syscall was designed to be the "dump" operation in this
system. David's other patch set provides the stream of events. So the
two are designed to work together. We had the discussion on using
netlink, of whatever form a while back, and there are a number of
reasons why that doesn't work (namespace being one).
I think fstatfs might also suffer from the issue of not being easy to
call on things for which you have no path (e.g. over-mounted mounts)
Plus we need to know which paths to query, which is why we need to
enumerate the mounts in the first place - how would we get the fds for
each mount? It might give you some sb info, but it doesn't tell you the
options that the sb is mounted with, and it doesn't tell you where it is
mounted either.
The overall aim is to solve some issues relating to scaling to large
numbers of mount in systemd and autofs, and also to provide a
generically useful interface that other tools may use to monitor mounts
in due course too. Currently parsing /proc/mounts is the only option,
and that tends to be slow and is certainly not atomic. Extension to
other sb related messages is a future goal, quota being one possible
application for the notifications.
If there is a simpler way to get to that goal, then thats all to the
good, and we should definitely consider it,
Steve.
On Wed, Aug 12, 2020 at 12:34 PM Steven Whitehouse [off-list ref] wrote:
The point of this is to give us the ability to monitor mounts from
userspace.
We haven't had that before, I don't see why it's suddenly such a big deal.
The notification side I understand. Polling /proc files is not the answer.
But the whole "let's design this crazy subsystem for it" seems way
overkill. I don't see anybody caring that deeply.
It really smells like "do it because we can, not because we must".
Who the hell cares about monitoring mounts at a kHz frequencies? If
this is for MIS use, you want a nice GUI and not wasting CPU time
polling.
I'm starting to ignore the pull requests from David Howells, because
by now they have had the same pattern for a couple of years now:
esoteric new interfaces that seem overdesigned for corner-cases that
I'm not seeing people clamoring for.
I need (a) proof this is actualyl something real users care about and
(b) way more open discussion and implementation from multiple parties.
Because right now it looks like a small in-cabal of a couple of people
who have wild ideas but I'm not seeing the wider use of it.
Convince me otherwise. AGAIN. This is the exact same issue I had with
the notification queues that I really wanted actual use-cases for, and
feedback from actual outside users.
I really think this is engineering for its own sake, rather than
responding to actual user concerns.
Linus
From: Al Viro <viro@zeniv.linux.org.uk> Date: 2020-08-12 21:31:02
On Wed, Aug 12, 2020 at 07:33:26PM +0100, Al Viro wrote:
BTW, what would such opened files look like from /proc/*/fd/* POV? And
what would happen if you walk _through_ that symlink, with e.g. ".."
following it? Or with names of those attributes, for that matter...
What about a normal open() of such a sucker? It won't know where to
look for your ->private_data...
FWIW, you keep refering to regularity of this stuff from the syscall
POV, but it looks like you have no real idea of what subset of the
things available for normal descriptors will be available for those.
Another question: what should happen with that sucker on umount of
the filesystem holding the underlying object? Should it be counted
as pinning that fs?
Who controls what's in that tree? If we plan to have xattrs there,
will they be in a flat tree, or should it mirror the hierarchy of
xattrs? When is it populated? open() time? What happens if we
add/remove an xattr after that point?
If we open the same file several times, what should we get? A full
copy of the tree every time, with all coherency being up to whatever's
putting attributes there?
What are the permissions needed to do lookups in that thing?
All of that is about semantics and the answers are needed before we
start looking into implementations. "Whatever my implementation
does" is _not_ a good way to go, especially since that'll be cast
in stone as soon as API becomes exposed to userland...
From: Ian Kent <raven@themaw.net> Date: 2020-08-13 01:01:50
On Wed, 2020-08-12 at 14:06 +0100, David Howells wrote:
Miklos Szeredi [off-list ref] wrote:
quoted
That presumably means the mount ID <-> mount path mapping already
exists, which means it's just possible to use the open(mount_path,
O_PATH) to obtain the base fd.
No, you can't. A path more correspond to multiple mounts stacked on
top of
each other, e.g.:
mount -t tmpfs none /mnt
mount -t tmpfs none /mnt
mount -t tmpfs none /mnt
Now you have three co-located mounts and you can't use the path to
differentiate them. I think this might be an issue in autofs, but
Ian would
need to comment on that.
It is a problem for autofs, direct mounts in particular, but also
for mount ordering at times when umounting a tree of mounts where
mounts are covered or at shutdown.
Ian
From: Ian Kent <raven@themaw.net> Date: 2020-08-13 03:45:14
On Wed, 2020-08-12 at 12:50 -0700, Linus Torvalds wrote:
On Wed, Aug 12, 2020 at 12:34 PM Steven Whitehouse <
swhiteho@redhat.com> wrote:
quoted
The point of this is to give us the ability to monitor mounts from
userspace.
We haven't had that before, I don't see why it's suddenly such a big
deal.
Because there's a trend occurring in user space where there are
frequent and persistent mount changes that cause high overhead.
I've seen the number of problems building up over the last few months
that are essentially the same problem that I wanted to resolve. And
that's related to side effects of autofs using a large number of
mounts.
The problems are real.
The notification side I understand. Polling /proc files is not the
answer.
Yep, that's one aspect, getting the information about a mount without
reading the entire mount table seems like the sensible thing to do to
allow for a more efficient notification mechanism.
But the whole "let's design this crazy subsystem for it" seems way
overkill. I don't see anybody caring that deeply.
It really smells like "do it because we can, not because we must".
Who the hell cares about monitoring mounts at a kHz frequencies? If
this is for MIS use, you want a nice GUI and not wasting CPU time
polling.
That part of the problem still remains.
The kernel sending a continuous stream of wake ups under load does
also introduce a resource problem but that's probably something to
handle in user space.
I'm starting to ignore the pull requests from David Howells, because
by now they have had the same pattern for a couple of years now:
esoteric new interfaces that seem overdesigned for corner-cases that
I'm not seeing people clamoring for.
I need (a) proof this is actualyl something real users care about and
(b) way more open discussion and implementation from multiple
parties.
Because right now it looks like a small in-cabal of a couple of
people
who have wild ideas but I'm not seeing the wider use of it.
Convince me otherwise. AGAIN. This is the exact same issue I had with
the notification queues that I really wanted actual use-cases for,
and
feedback from actual outside users.
I really think this is engineering for its own sake, rather than
responding to actual user concerns.
Linus
From: Jeffrey E Altman <hidden> Date: 2020-08-13 03:56:59
On 8/12/2020 2:18 PM, Linus Torvalds (torvalds@linux-foundation.org) wrote:
What's wrong with fstatfs()? All the extra magic metadata seems to not
really be anything people really care about.
What people are actually asking for seems to be some unique mount ID,
and we have 16 bytes of spare information in 'struct statfs64'.
All the other fancy fsinfo stuff seems to be "just because", and like
complete overdesign.
Hi Linus,
Is there any existing method by which userland applications can
determine the properties of the filesystem in which a directory or file
is stored in a filesystem agnostic manner?
Over the past year I've observed the opendev/openstack community
struggle with performance issues caused by rsync's inability to
determine if the source and destination object's last update time have
the same resolution and valid time range. If the source file system
supports 100 nanosecond granularity and the destination file system
supports one second granularity, any source file with a non-zero
fractional seconds timestamp will appear to have changed compared to the
copy in the destination filesystem which discarded the fractional
seconds during the last sync. Sure, the end user could use the
--modify-window=1 option to inform rsync to add fuzz to the comparisons,
but that introduces the possibility that a file updated a fraction of a
second after an rsync execution would not synchronize the file on the
next run when both source and target have fine grained timestamps. If
the userland sync processes have access to the source and destination
filesystem time capabilities, they can make more intelligent decisions
without explicit user input. At a minimum, the timestamp properties
that are important to know include the range of valid timestamps and the
resolution. Some filesystems support unsigned 32-bit time starting with
UNIX epoch. Others signed 32-bit time with UNIX epoch. Still others
FAT, NTFS, etc use alternative epochs and range and resolutions.
Another case where lack of filesystem properties is problematic is "df
--local" which currently relies upon string comparisons of file system
name strings to determine if the underlying file system is local or
remote. This requires that the gnulib maintainers have knowledge of all
file systems implementations, their published names, and which category
they belong to. Patches have been accepted in the past year to add
"smb3", "afs", and "gpfs" to the list of remote file systems. There are
many more remote filesystems that have yet to be added including
"cephfs", "lustre", "gluster", etc.
In many cases, the filesystem properties cannot be inferred from the
filesystem name. For network file systems, these properties might
depend upon the remote server capabilities or even the properties
associated with a particular volume or share. Consider the case of a
remote file server that supports 64-bit 100ns time but which for
backward compatibility exports certain volumes or shares with more
restrictive capabilities. Or the case of a network file system protocol
that has evolved over time and gained new capabilities.
For the AFS community, fsinfo offers a method of exposing some server
and volume properties that are obtained via "path ioctls" in OpenAFS and
AuriStorFS. Some example of properties that might be exposed include
answers to questions such as:
* what is the volume cell id? perhaps a uuid.
* what is the volume id in the cell? unsigned 64-bit integer
* where is a mounted volume hosted? which fileservers, named by uuid
* what is the block size? 1K, 4K, ...
* how many blocks are in use or available?
* what is the quota (thin provisioning), if any?
* what is the reserved space (fat provisioning), if any?
* how many vnodes are present?
* what is the vnode count limit, if any?
* when was the volume created and last updated?
* what is the file size limit?
* are byte range locks supported?
* are mandatory locks supported?
* how many entries can be created within a directory?
* are cross-directory hard links supported?
* are directories just-send-8, case-sensitive, case-preserving, or
case-insensitive?
* if not just-send-8, what character set is used?
* if Unicode, what normalization rules? etc.
* are per-object acls supported?
* what volume maximum acl is assigned, if any?
* what volume security policy (authn, integ, priv) is assigned, if any?
* what is the replication policy, if any?
* what is the volume encryption policy, if any?
* what is the volume compression policy, if any?
* are server-to-server copies supported?
* which of atime, ctime and mtime does the volume support?
* what is the permitted timestamp range and resolution?
* are xattrs supported?
* what is the xattr maximum name length?
* what is the xattr maximum object size?
* is the volume currently reachable?
* is the volume immutable?
* etc ...
Its true that there isn't widespread use of these filesystem properties
by today's userland applications but that might be due to the lack of
standard interfaces necessary to acquire the information. For example,
userland frameworks for parallel i/o HPC applications such as HDF5,
PnetCDF and ROMIO require each supported filesystem to provide its own
proprietary "driver" which does little more than expose the filesystem
properties necessary to optimize the layout of file stream data
structures. With something like "fsinfo" it would be much easier to
develop these HPC frameworks in a filesystem agnostic manner. This
would permit applications built upon these frameworks to use the best
Linux filesystem available for the workload and not simply the ones for
which proprietary "drivers" have been published.
Although I am sympathetic to the voices in the community that would
prefer to start over with a different architectural approach, David's
fsinfo has been under development for more than two years. It has not
been developed in a vacuum but in parallel with other kernel components
that have been merged during that time frame. From my reading of this
thread and those that preceded it, fsinfo has also been developed with
input from significant userland development communities that intend to
leverage the syscall interface as soon as it becomes available. The
March 2020 discussion of fsinfo received positive feedback not only from
within Red Hat but from other parties as well.
Since no one stepped up to provide an alternative approach in the last
five months, how long should those that desire access to the
functionality be expected to wait for it?
What is the likelihood that an alternative robust solution will be
available in the next merge window or two?
Is the design so horrid that it is better to go without the
functionality than to live with the imperfections?
I for one would like to see this functionality be made available sooner
rather than later. I know my end users would benefit from the
availability of fsinfo.
Thank you for listening. Stay healthy and safe, and please wear a mask.
Jeffrey Altman
From: Karel Zak <kzak@redhat.com> Date: 2020-08-13 08:53:09
On Wed, Aug 12, 2020 at 02:43:32PM +0200, Miklos Szeredi wrote:
On Wed, Aug 12, 2020 at 1:28 PM Karel Zak [off-list ref] wrote:
quoted
The proposal is based on paths and open(), how do you plan to deal
with mount IDs? David's fsinfo() allows to ask for mount info by mount
ID and it works well with mount notification where you get the ID. The
collaboration with notification interface is critical for our use-cases.
One would use the notification to keep an up to date set of attributes
for each watched mount, right?
That presumably means the mount ID <-> mount path mapping already
exists, which means it's just possible to use the open(mount_path,
O_PATH) to obtain the base fd.
The notification also reports new mount nodes, so we have no mount ID
<-> mount path mapping in userspace yet.
The another problem is that open(path) cannot be used if you have multiple
filesystems on the same mount point -- in this case (at least in theory)
you can get ID for by-path inaccessible filesystem.
A new syscall that returns an fd pointing to the root of the mount
might be the best solution:
int open_mount(int root_fd, u64 mntid, int flags);
Yes, something like this is necessary. You do not want to depend
on paths if you want to read information about mountpoints.
Karel
--
Karel Zak [off-list ref]
http://karelzak.blogspot.com
From: Karel Zak <kzak@redhat.com> Date: 2020-08-13 10:36:58
On Wed, Aug 12, 2020 at 12:50:28PM -0700, Linus Torvalds wrote:
Convince me otherwise. AGAIN. This is the exact same issue I had with
the notification queues that I really wanted actual use-cases for, and
feedback from actual outside users.
I thought (in last 10 years) we all agree that /proc/self/mountinfo is
the expensive, ineffective and fragile way how to deliver information to
userspace.
We have systems with thousands of mountpoints and compose mountinfo in
kernel and again parse it in userspace takes time and it's strange if
you need info about just one mountpoint.
Unfortunately, the same systems modify the huge mount table extremely
often, because it starts/stops large number of containers and every
container means a mount operation(s).
In this crazy environment, we have userspace tools like systemd or udisk
which react to VFS changes and there is no elegant way how to get
details about a modified mount node from kernel.
And of course we already have negative feedback from users who
maintain large systems -- mountinfo returns inconsistent data if you
read it by more read() calls (hopefully fixed by recent Miklos'
mountinfo cursors); system is pretty busy to compose+parse mountinfo,
etc.
I really think this is engineering for its own sake, rather than
responding to actual user concerns.
We're too old and too lazy for "engineering for its own sake" :-)
there is pressure from users ...
Maybe David's fsinfo() sucks, but it does not mean that
/proc/self/mountinfo is something cool. Right?
We have to dig deep grave for /proc/self/mountinfo ...
Karel
--
Karel Zak [off-list ref]
http://karelzak.blogspot.com
From: Lennart Poettering <hidden> Date: 2020-08-14 07:58:48
On Mi, 12.08.20 12:50, Linus Torvalds (torvalds@linux-foundation.org) wrote:
On Wed, Aug 12, 2020 at 12:34 PM Steven Whitehouse [off-list ref] wrote:
quoted
The point of this is to give us the ability to monitor mounts from
userspace.
We haven't had that before, I don't see why it's suddenly such a big deal.
The notification side I understand. Polling /proc files is not the answer.
But the whole "let's design this crazy subsystem for it" seems way
overkill. I don't see anybody caring that deeply.
It really smells like "do it because we can, not because we must".
With my systemd maintainer hat on (and of other userspace stuff),
there's a couple of things I really want from the kernel because it
would fix real problems for us:
1. we want mount notifications that don't require to scan
/proc/self/mountinfo entirely again every time things change, over
and over again, simply because that doesn't scale. We have various
bugs open about this performance bottleneck, I could point you to,
but I figure it's easy to see why this currently doesn't scale...
2. We want an unpriv API to query (and maybe set) the fs UUID, like we
have nowadays for the fs label FS_IOC_[GS]ETFSLABEL
3. We want an API to query time granularity of file systems
timestamps. Otherwise it's so hard in userspace to reproducibly
re-generate directory trees. We need to know for example that some
fs only has 2s granularity (like fat).
4. Similar, we want to know if an fs is case-sensitive for file
names. Or case-preserving. And which charset it accepts for filenames.
5. We want to know if a file system supports access modes, xattrs,
file ownership, device nodes, symlinks, hardlinks, fifos, atimes,
btimes, ACLs and so on. All these things currently can only be
figured out by changing things and reading back if it worked. Which
sucks hard of course.
6. We'd like to know the max file size on a file system.
7. Right now it's hard to figure out mount options used for the fs
backing some file: you can now statx() the file, determine the
mnt_id by that, and then search that in /proc/self/mountinfo, but
it's slow, because again we need to scan the whole file until we
find the entry we need. And that can be huge IRL.
8. Similar: we quite often want to know submounts of a mount. It would
be great if for that kind of information (i.e. list of mnt_ids
below some other mnt_id) we wouldn't have to scan the whole of
/p/s/mi again. In many cases in our code we operate recursively,
and want to know the mounts below some specific dir, but currently
pay performance price for it if the number of file systems on the
host is huge. This doesn't sound like a biggie, but actually is a
biggie. In systemd we spend a lot of time scaninng /p/s/mi...
9. How are file locks implemented on this fs? Are they local only, and
orthogonal to remote locks? Are POSIX and BSD locks possibly merged
at the backend? Do they work at all?
I don't really care too much how an API for this looks like, but let
me just say that I am not a fan of APIs that require allocating an fd
for querying info about an fd. This 'feels' a bit too recursive: if
you expose information about some fd in some magic procfs subdir, or
even in some virtual pseudo-file below the file's path then this means
we have to allocate a new fd to figure out things or the first fd, and
if we'd know the same info for that, we'd theoretically recurse
down. Now of course, most likely IRL we wouldn't actually recurse down,
but it is still smelly. In particular if fd limits are tight. I mean,
I really don't care if you expose non-file-system stuff via the fs, if
that's what you want, but I think exposing *fs* metainfo in the *fs*,
it's just ugly.
I generally detest APIs that have no chance to ever returning multiple
bits of information atomically. Splitting up querying of multiple
attributes into multiple system calls means they couldn't possibly be
determined in a congruent way. I much prefer APIs where we provide a
struct to fill in and do a single syscall, and at least for some
fields we'd know afterwards that the fields were filled in together
and are congruent with each other.
I am a fan of the statx() system call I must say. If we had something
like this for the file system itself I'd be quite happy, it could tick
off many of the requests I list above.
Hope this is useful,
Lennart
--
Lennart Poettering, Berlin
From: Lennart Poettering <hidden> Date: 2020-08-14 08:06:18
On Mi, 12.08.20 11:18, Linus Torvalds (torvalds@linux-foundation.org) wrote:
On Tue, Aug 11, 2020 at 5:05 PM David Howells [off-list ref] wrote:
quoted
Well, the start of it was my proposal of an fsinfo() system call.
Ugh. Ok, it's that thing.
This all seems *WAY* over-designed - both your fsinfo and Miklos' version.
What's wrong with fstatfs()? All the extra magic metadata seems to not
really be anything people really care about.
What people are actually asking for seems to be some unique mount ID,
and we have 16 bytes of spare information in 'struct statfs64'.
statx() exposes a `stx_mnt_id` field nowadays. So that's easy and
quick to get nowadays. It's just so inefficient matching that up with
/proc/self/mountinfo then. And it still won't give you any of the fs
capability bits (time granularity, max file size, features, …),
because the kernel doesn't expose that at all right now.
OTOH I'd already be quite happy if struct statfs64 would expose
f_features, f_max_fsize, f_time_granularity, f_charset_case_handling
fields or so.
Lennart
--
Lennart Poettering, Berlin