On Fri, May 20, 2022 at 07:45:39PM -0400, Theodore Ts'o wrote:

On Fri, May 20, 2022 at 12:16:52PM -0400, Kent Overstreet wrote:

quoted

Where the lack of real namespacing bites us more is when ioctls get promoted
from filesystem or driver on up. Ted had a good example of an ext2 ioctl getting
promoted to the VFS when it really shouldn't have, because it was exposing ext2
specific data structures.

But because this is as simple as changing a #define EXT2_IOC to #define FS_IOC,
it's really easy to do without adequate review - you don't have to change the
ioctl number and break userspace, so why would you?

Introducing real namespacing would mean that promoting an ioctl to the VFS level
would really have to be a new ioctl, and it'll get people to think more about
what the new ioctl would be.

It's not clear that making harder not to break userspace is a
*feature*.  If existing programs are using a particular ioctl
namespace, being able to have other file systems adopt it has
historically been considered a *feature* not a *bug*.

At the time, we had working utilities, chattr and lsattr, which were
deployed on all Linux distributions, and newer file systems, such as
xfs, reiserfs, btrfs, etc., decided they wanted to piggy-back on those
existing utilities.  Forcing folks to deploy new utilities just
because it's the best way to force "adequate review" might be swinging
the pendulum too far in the straight-jacket direction.

But back in those days, users updating those core utilities was a much bigger
hassle. That's changing, we don't really have users compiling from source
anymore, and we're slowly but steadily moving towards the rolling releases
world: slackware -> debian -> nixos.

And on the developer side of things, historically developers have worked on a
few packages where they were comfortable with the process, and packages tended
more towards giant monorepos, but the younger generation is more used to working
across multiple packages/repositories as necessary (this is where github has
been emphatically a good thing, despite being proprietary; it's standardized a
lot of the friction-y "how do I submit to this repo" stuff).

So I understand where you're coming from but I think this is worth rethinking.

Additionally, bikeshedding gets really painful when people are trying to plan
for all eventualities and think too far ahead. Having multiple stages of review
is _helpful_ with this. If an ioctl is filesystem-private, it's perfectly fine
for it embed filesystem specific data structures - if we can ensure that that
won't get lifted to the VFS layer without anyone noticing!

...

In the case of extended attributes, we had perfectly working userspace
tools that would have ***broken*** if we adhered to a doctrinaire,
when you promote an interface, we break the userspace interface Just
Because it's the Good Computer Science Thing To Do.

Not broken, though - it just would've needed updating to support additional
filesystems, and when the ioctls don't need changing the patches would be
trivial:

ret = ioctl_xfs_goingdown(..);
becomes
ret = ioctl_fs_goingdown(...) ?:
      ioctl_xfs_goingdown(...);

(I'm the only one I know who does that chaining syntax in C, but I like it :)

So this approach requires that someone has to actually implement the
wrapper library.  Who will that be?  The answer could be, "let libc do
it", but then we need to worry about all the C libraries out there
actually adopting the new ioctl, which takes a long time, and
historically, some C library maintainers have had.... opinionated
points of view about the sort of "value add that should be done at the
C Library level".

Not libc, and we definitely don't want to have to update that library for every
new ioctl - I'm imagining that library just being responsible for the "query
kernel for ioctl numbers" part, the ioctl definitions themselves will still come
from kernel headers.

For example, I have an ext2fs library function
ext2fs_get_device_size2(), which wraps not only the BLKGETSIZE and
BLKGETSIZE64 ioctls, but also the equivalents for Apple Darwin
(DKIOCGETBLOCKCOUNT), FreeBSD/NetBSD (DIOCGDINFO and later
DIOCGMEDIASIZE), and the Window's DeviceIoControl()'s
IOCTL_DISK_GET_DRIVE_GEOMETRY call.  The point is that wrapper
functions are very much orthogonal to the ioctl interface; we're all
going to have wrapper functions, and we'll create them where they are
needed.

This seems unrelated to the ioctl v2 discussion, but - it would be _great_ if we
could get that in a separate repository where others could make use of it :)