On Wed, Nov 18, 2020 at 12:38 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > On Tue, Oct 13, 2020 at 11:22 AM Dave Martin <Dave.Martin@xxxxxxx> 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