On Mon 23-06-25 15:00:43, Christian Brauner wrote: > On Mon, Jun 23, 2025 at 02:54:00PM +0200, Amir Goldstein wrote: > > On Mon, Jun 23, 2025 at 2:25 PM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > > > > > On Mon, Jun 23, 2025 at 02:06:43PM +0200, Jan Kara wrote: > > > > On Mon 23-06-25 11:01:30, Christian Brauner wrote: > > > > > Various filesystems such as pidfs (and likely drm in the future) have a > > > > > use-case to support opening files purely based on the handle without > > > > > having to require a file descriptor to another object. That's especially > > > > > the case for filesystems that don't do any lookup whatsoever and there's > > > > > zero relationship between the objects. Such filesystems are also > > > > > singletons that stay around for the lifetime of the system meaning that > > > > > they can be uniquely identified and accessed purely based on the file > > > > > handle type. Enable that so that userspace doesn't have to allocate an > > > > > object needlessly especially if they can't do that for whatever reason. > > > > > > > > > > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx> > > > > > > > > Hmm, maybe we should predefine some invalid fd value userspace should pass > > > > when it wants to "autopick" fs root? Otherwise defining more special fd > > > > values like AT_FDCWD would become difficult in the future. Or we could just > > > > > > Fwiw, I already did that with: > > > > > > #define PIDFD_SELF_THREAD -10000 /* Current thread. */ > > > #define PIDFD_SELF_THREAD_GROUP -20000 /* Current thread group leader. */ > > > > > > I think the correct thing to do would have been to say anything below > > > > > > #define AT_FDCWD -100 /* Special value for dirfd used to > > > > > > is reserved for the kernel. But we can probably easily do this and say > > > anything from -10000 to -40000 is reserved for the kernel. > > > > > > I would then change: > > > > > > #define PIDFD_SELF_THREAD -10000 /* Current thread. */ > > > #define PIDFD_SELF_THREAD_GROUP -10001 /* Current thread group leader. */ > > > > > > since that's very very new and then move > > > PIDFD_SELF_THREAD/PIDFD_SELF_THREAD_GROUP to include/uapi/linux/fcntl.h > > > > > > and add that comment about the reserved range in there. > > > > > > The thing is that we'd need to enforce this on the system call level. > > > > > > Thoughts? > > > > > > > define that FILEID_PIDFS file handles *always* ignore the fd value and > > > > auto-pick the root. > > > > > > I see the issue I don't think it's a big deal but I'm open to adding: > > > > > > #define AT_EBADF -10009 /* -10000 - EBADF */ > > > > > > and document that as a stand-in for a handle that can't be resolved. > > > > > > Thoughts? > > > > I think the AT prefix of AT_FDCWD may have been a mistake > > because it is quite easy to confuse this value with the completely > > unrelated namespace of AT_ flags. > > > > This is a null dirfd value. Is it not? > > Not necessarily dirfd. We do allow direct operations of file descriptor > of any type. For example, in the mount api where you can mount files > onto other files. > > > > > FD_NULL, FD_NONE? > > FD_INVALID, I think. > > > > > You could envision that an *at() syscalls could in theory accept > > (FD_NONE , "/an/absolute/path/only", ... > > > > or MOUNTFD_NONE if we want to define a constant specifically for > > this open_by_handle_at() extension. > > I think this is useful beyond open_by_handle_at(). Yes, defining FD_INVALID seems like useful addition that may become useful in other cases in the future as well. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
