Re: [GIT PULL] vfs pidfd

From: Christian Brauner <brauner@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] vfs pidfd
Date: Tue, 12 Mar 2024 21:09:26 +0100	[thread overview]
Message-ID: <20240312-pflug-sandalen-0675311c1ec5@brauner> (raw)
In-Reply-To: <CAHk-=wgsjaakq1FFOXEKAdZKrkTgGafW9BedmWMP2NNka4bU-w@mail.gmail.com>

On Tue, Mar 12, 2024 at 09:23:55AM -0700, Linus Torvalds wrote:
> On Tue, 12 Mar 2024 at 07:16, Christian Brauner <brauner@kernel.org> wrote:
> >
> > No, the size of struct pid was the main reason but I don't think it
> > matters. A side-effect was that we could easily enforce 64bit inode
> > numbers. But realistically it's trivial enough to workaround. Here's a
> > patch for what I think is pretty simple appended. Does that work?
> 
> This looks eminently sane to me. Not that I actually _tested_it, but
> since my testing would have compared it to my current setup (64-bit
> and CONFIG_FS_PID=y) any testing would have been pointless because
> that case didn't change.
> 
> Looking at the patch, I do wonder how much we even care about 64-bit
> inodes. I'd like to point out how 'path_from_stashed()' only takes a
> 'unsigned long ino' anyway, and I don't think anything really cares
> about either the high bits *or* the uniqueness of that inode number..
> 
> And similarly, i_ino isn't actually *used* for anything but naming to
> user space.

It's used to compare pidfs and someone actually already sent a pull
request for this to another project iirc. So it'd be good to keep that
property.

But if your point is that we don't care about this for 32bit then I do
agree. We could do away with the checks completely and just accept the
truncation for 32bit. If that's your point feel free to just remove the
32bit handling in the patch and apply it. Let me know. Maybe I
misunderstood.

> 
> So I'm not at all sure the whole 64-bit checks are worth it. Am I
> missing something else?