[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 RESEND] fcntl: Add 32bit filesystem mode
From: |
Arnd Bergmann |
Subject: |
Re: [PATCH v3 RESEND] fcntl: Add 32bit filesystem mode |
Date: |
Wed, 18 Nov 2020 10:00:19 +0100 |
On Wed, Nov 18, 2020 at 12:38 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Oct 13, 2020 at 11:22 AM Dave Martin <Dave.Martin@arm.com> wrote:
>
> > > case F_SETFD:
> > > err = 0;
> > > set_close_on_exec(fd, arg & FD_CLOEXEC);
> > > + if (arg & FD_32BIT_MODE)
> > > + filp->f_mode |= FMODE_32BITHASH;
> > > + else
> > > + filp->f_mode &= ~FMODE_32BITHASH;
> >
> > This seems inconsistent? F_SETFD is for setting flags on a file
> > descriptor. Won't setting a flag on filp here instead cause the
> > behaviour to change for all file descriptors across the system that are
> > open on this struct file? Compare set_close_on_exec().
> >
> > I don't see any discussion on whether this should be an F_SETFL or an
> > F_SETFD, though I see F_SETFD was Ted's suggestion originally.
>
> I cannot honestly say I know the semantic difference.
>
> I would ask the QEMU people how a user program would expect
> the flag to behave.
I agree it should either use F_SETFD to set a bit in the fdtable structure
like set_close_on_exec() or it should use F_SETFL to set a bit in
filp->f_mode.
It appears the reason FMODE_32BITHASH is part of filp->f_mode
is that the only user today is nfsd, which does not have a file
descriptor but only has a struct file. Similarly, the only code that
understands the difference (ext4_readdir()) has no reference to
the file descriptor.
If this becomes an O_DIR32BITHASH flag for F_SETFL,
I suppose it should also be supported by openat2().
Arnd