Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Tue, 19 Sep 2023 14:50:28 +0200
From:   Christian Brauner <brauner@kernel.org>
To:     Andreas Dilger <adilger@dilger.ca>
Cc:     Miklos Szeredi <miklos@szeredi.hu>,
        Miklos Szeredi <mszeredi@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-api@vger.kernel.org, linux-man@vger.kernel.org,
        linux-security-module@vger.kernel.org, Karel Zak <kzak@redhat.com>,
        Ian Kent <raven@themaw.net>,
        David Howells <dhowells@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <christian@brauner.io>,
        Amir Goldstein <amir73il@gmail.com>
Subject: Re: [RFC PATCH 2/3] add statmnt(2) syscall
Message-ID: <20230919-gewusel-hingabe-714c000cef8f@brauner>
References: <20230913152238.905247-1-mszeredi@redhat.com>
 <20230913152238.905247-3-mszeredi@redhat.com>
 <20230914-salzig-manifest-f6c3adb1b7b4@brauner>
 <CAJfpegs-sDk0++FjSZ_RuW5m-z3BTBQdu4T9QPtWwmSZ1_4Yvw@mail.gmail.com>
 <20230914-lockmittel-verknallen-d1a18d76ba44@brauner>
 <CAJfpegt-VPZP3ou-TMQFs1Xupj_iWA5ttC2UUFKh3E43EyCOQQ@mail.gmail.com>
 <20230918-grafik-zutreffen-995b321017ae@brauner>
 <59DA5D4F-8242-4BD4-AE1C-FC5A6464E377@dilger.ca>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <59DA5D4F-8242-4BD4-AE1C-FC5A6464E377@dilger.ca>
Precedence: bulk

On Mon, Sep 18, 2023 at 02:58:00PM -0600, Andreas Dilger wrote:
> On Sep 18, 2023, at 7:51 AM, Christian Brauner <brauner@kernel.org> wrote:
> > 
> > 
> >> The type and subtype are naturally limited to sane sizes, those are
> >> not an issue.
> > 
> > What's the limit for fstype actually? I don't think there is one.
> > There's one by chance but not by design afaict?
> > 
> > Maybe crazy idea:
> > That magic number thing that we do in include/uapi/linux/magic.h
> > is there a good reason for this or why don't we just add a proper,
> > simple enum:
> > 
> > enum {
> > 	FS_TYPE_ADFS        1
> > 	FS_TYPE_AFFS        2
> > 	FS_TYPE_AFS         3
> > 	FS_TYPE_AUTOFS      4
> > 	FS_TYPE_EXT2	    5
> > 	FS_TYPE_EXT3	    6
> > 	FS_TYPE_EXT4	    7
> > 	.
> > 	.
> > 	.
> > 	FS_TYPE_MAX
> > }
> > 
> > that we start returning from statmount(). We can still return both the
> > old and the new fstype? It always felt a bit odd that fs developers to
> > just select a magic number.
> 
> Yes, there is a very good reason that there isn't an enum for filesystem

I think this isn't all that relevant to the patchset so I'm not going to
spend a lot of time on this discussion but I'm curious.

> type, which is because this API would be broken if it encounters any
> filesystem that is not listed there.  Often a single filesystem driver in
> the kernel will have multiple different magic numbers to handle versions,
> endianness, etc.

Why isn't this a problem for magically chosen numbers?

> 
> Having a 32-bit magic number allows decentralized development with low
> chance of collision, and using new filesystems without having to patch
> every kernel for this new API to work with that filesystem.  Also,

We don't care about out of tree filesystems.

> filesystems come and go (though more slowly) over time, and keeping the

Even if we did ever remove a filesystem we'd obviously leave the enum in
place. Same thig we do for deprecated flags, same thing we'd do for
magic numbers.

> full list of every filesystem ever developed in the kernel enum would be
> a headache.

I really don't follow this argument.

> 
> The field in the statmnt() call would need to be at a fixed-size 32-bit
> value in any case, so having it return the existing magic will "just work"
> because userspace tools already know and understand these magic values,
> while introducing an in-kernel enum would be broken for multiple reasons.

We already do expose the magic number in statmount() but it can't
differentiate between ext2, ext3, and ext4 for example which is why I
asked.

Afaict, none of the points you mention are show stoppers and none of
them are unique to an enum.