On Wed, Jul 11, 2018 at 04:25:55PM +0100, Al Viro wrote:
> FWIW, looking at the ->f_flags handling, I'm seriously tempted to do
> alloc_file_pseudo(inode, mnt, name, f_flags, ops).
>
> Reason: right now all but two callers of alloc_file_pseudo() are followed
> by setting ->f_flags and for all those callers the mode argument passed
> to alloc_file_pseudo() is equal to OPEN_FMODE(->f_flags value to be).
>
> The exceptions are __shmem_file_setup() and hugetlb_file_setup(). Both
> end up with FMODE_READ | FMODE_WRITE combined with 0 (i.e. O_RDONLY) in
> f_flags. Which smells like a bug in making, at the very least.
>
> Unless somebody has a good reason why those shouldn't have O_RDWR,
> I want to add (and fold back into individual "convert to alloc_file_pseudo"
> commits) the following:
[snip]
> Objections? Another odd beastie is do_shmat() - there we end up with
> f_mode not matching f_flags, same way as in shmem and hugetlb. If we
> could rectify that one as well, we'd be able to switch alloc_file_clone()
> to flags as well. I would obviously prefer that kind of change to happen
> before these helpers go into mainline...
Actually, looking at the entire thing, I'm rather tempted to go for
alloc_empty_file(f_flags, cred)
setting both f_flags and f_flags-derived part of f_mode, making
alloc_file_pseudo(inode, mnt, name, f_flags, ops)
alloc_file_clone(base, f_flags, ops)
do the same automatically as they call alloc_empty_file().
do_dentry_open() would, instead of
f->f_mode |= OPEN_FMODE(f->f_flags) | FMODE_LSEEK |
FMODE_PREAD | FMODE_PWRITE;
do just
f->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
just before calling ->open(), with comment along the lines of
"usually we want those set; let ->open() remove the wrong ones
if it wants to".
