Re: [PATCH] vfs: remove the excl argument from the ->create() inode_operation

On Thu, 2025-11-06 at 07:07 -0500, Jeff Layton wrote:
> On Thu, 2025-11-06 at 08:23 +1100, NeilBrown wrote:
> > On Thu, 06 Nov 2025, Jeff Layton wrote:
> > > Since ce8644fcadc5 ("lookup_open(): expand the call of vfs_create()"),
> > > the "excl" argument to the ->create() inode_operation is always set to
> > > true. Remove it, and fix up all of the create implementations.
> > 
> > nonono
> > 
> > 
> > > @@ -3802,7 +3802,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> > >  		}
> > >  
> > >  		error = dir_inode->i_op->create(idmap, dir_inode, dentry,
> > > -						mode, open_flag & O_EXCL);
> > > +						mode);
> > 
> > "open_flag & O_EXCL" is not the same as "true".
> > 
> > It is true that "all calls to vfs_create() pass true for 'excl'"
> > The same is NOT true for inode_operations.create.
> > 
> 
> I don't think this is a problem, actually:
> 
> Almost all of the existing ->create() operations ignore the "excl"
> bool. There are only two that I found that do not: NFS and GFS2. Both
> of those have an ->atomic_open() operation though, so lookup_open()
> will never call ->create() for those filesystems. This means that -
> > create() _is_ always called with excl == true.

How about this for a revised changelog, which makes the above clear:

    vfs: remove the excl argument from the ->create() inode_operation
    
    Since ce8644fcadc5 ("lookup_open(): expand the call of vfs_create()"),
    the "excl" argument to the ->create() inode_operation is always set to
    true in vfs_create().
    
    There is another call to ->create() in lookup_open() that can set it to
    either true or false. All of the ->create() operations in the kernel
    ignore the excl argument, except for NFS and GFS2. Both NFS and GFS2
    have an ->atomic_open() operation, however so lookup_open() will never
    call ->create() on those filesystems.
    
    Remove the "excl" argument from the ->create() operation, and fix up the
    filesystems accordingly.

Maybe we also need some comments or updates to Documentation/ to make
it clear that ->create() always implies O_EXCL semantics?
-- 
Jeff Layton <[email protected]>