[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGudoHHnpOp5JL-wUnVp+X=dt+pRtX2o-dbfuqQamjWhxJei-A@mail.gmail.com>
Date: Mon, 29 Jul 2024 12:56:55 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Florian Weimer <fweimer@...hat.com>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-api@...r.kernel.org, Dave Chinner <dchinner@...hat.com>
Subject: Re: Testing if two open descriptors refer to the same inode
On Mon, Jul 29, 2024 at 12:50 PM Mateusz Guzik <mjguzik@...il.com> wrote:
>
> On Mon, Jul 29, 2024 at 12:40:35PM +0200, Florian Weimer wrote:
> > * Mateusz Guzik:
> >
> > > On Mon, Jul 29, 2024 at 08:55:46AM +0200, Florian Weimer wrote:
> > >> It was pointed out to me that inode numbers on Linux are no longer
> > >> expected to be unique per file system, even for local file systems.
> > >
> > > I don't know if I'm parsing this correctly.
> > >
> > > Are you claiming on-disk inode numbers are not guaranteed unique per
> > > filesystem? It sounds like utter breakage, with capital 'f'.
> >
> > Yes, POSIX semantics and traditional Linux semantics for POSIX-like
> > local file systems are different.
> >
>
> Can you link me some threads about this?
>
> > > While the above is not what's needed here, I guess it sets a precedent
> > > for F_DUPINODE_QUERY (or whatever other name) to be added to handily
> > > compare inode pointers. It may be worthwhile regardless of the above.
> > > (or maybe kcmp could be extended?)
> >
> > I looked at kcmp as well, but I think it's dependent on
> > checkpoint/restore. File sameness checks are much more basic than that.
> >
>
> I had this in mind (untested modulo compilation):
>
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 300e5d9ad913..5723c3e82eac 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -343,6 +343,13 @@ static long f_dupfd_query(int fd, struct file *filp)
> return f.file == filp;
> }
>
> +static long f_dupfd_query_inode(int fd, struct file *filp)
> +{
> + CLASS(fd_raw, f)(fd);
> +
> + return f.file->f_inode == filp->f_inode;
> +}
> +
> static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
> struct file *filp)
> {
> @@ -361,6 +368,9 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
> case F_DUPFD_QUERY:
> err = f_dupfd_query(argi, filp);
> break;
> + case F_DUPFD_QUERY_INODE:
> + err = f_dupfd_query_inode(argi, filp);
> + break;
> case F_GETFD:
> err = get_close_on_exec(fd) ? FD_CLOEXEC : 0;
> break;
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index c0bcc185fa48..2e93dbdd8fd2 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -16,6 +16,8 @@
>
> #define F_DUPFD_QUERY (F_LINUX_SPECIFIC_BASE + 3)
>
> +#define F_DUPFD_QUERY_INODE (F_LINUX_SPECIFIC_BASE + 4)
> +
> /*
> * Cancel a blocking posix lock; internal use only until we expose an
> * asynchronous lock api to userspace:
To clarify, if indeed the dev + ino combo is no longer sufficient to
conclude it's the same thing, an explicit & handy to use way should be
provided.
For a case where you got both inodes open something like the above
will do the trick, unless I'm missing something. I can do a proper
patch posting later after runtime testing and whatnot if you confirm
this sorts out your problem.
--
Mateusz Guzik <mjguzik gmail.com>
Powered by blists - more mailing lists