Message-id: <176247363419.634289.473957828516111884@noble.neil.brown.name>
Date: Fri, 07 Nov 2025 11:00:34 +1100
From: NeilBrown <neilb@...mail.net>
To: "Jeff Layton" <jlayton@...nel.org>
Cc: "Eric Van Hensbergen" <ericvh@...nel.org>,
 "Latchesar Ionkov" <lucho@...kov.net>,
 "Dominique Martinet" <asmadeus@...ewreck.org>,
 "Christian Schoenebeck" <linux_oss@...debyte.com>,
 "David Sterba" <dsterba@...e.com>, "David Howells" <dhowells@...hat.com>,
 "Marc Dionne" <marc.dionne@...istor.com>,
 "Alexander Viro" <viro@...iv.linux.org.uk>,
 "Christian Brauner" <brauner@...nel.org>, "Jan Kara" <jack@...e.cz>,
 "Tigran A. Aivazian" <aivazian.tigran@...il.com>,
 "Chris Mason" <clm@...com>, "Xiubo Li" <xiubli@...hat.com>,
 "Ilya Dryomov" <idryomov@...il.com>, "Jan Harkes" <jaharkes@...cmu.edu>,
 coda@...cmu.edu, "Tyler Hicks" <code@...icks.com>,
 "Jeremy Kerr" <jk@...abs.org>, "Ard Biesheuvel" <ardb@...nel.org>,
 "Namjae Jeon" <linkinjeon@...nel.org>,
 "Sungjong Seo" <sj1557.seo@...sung.com>,
 "Yuezhang Mo" <yuezhang.mo@...y.com>, "Theodore Ts'o" <tytso@....edu>,
 "Andreas Dilger" <adilger.kernel@...ger.ca>,
 "Jaegeuk Kim" <jaegeuk@...nel.org>, "Chao Yu" <chao@...nel.org>,
 "OGAWA Hirofumi" <hirofumi@...l.parknet.co.jp>,
 "Miklos Szeredi" <miklos@...redi.hu>,
 "Andreas Gruenbacher" <agruenba@...hat.com>,
 "Viacheslav Dubeyko" <slava@...eyko.com>,
 "John Paul Adrian Glaubitz" <glaubitz@...sik.fu-berlin.de>,
 "Yangtao Li" <frank.li@...o.com>, "Richard Weinberger" <richard@....at>,
 "Anton Ivanov" <anton.ivanov@...bridgegreys.com>,
 "Johannes Berg" <johannes@...solutions.net>,
 "Mikulas Patocka" <mikulas@...ax.karlin.mff.cuni.cz>,
 "Muchun Song" <muchun.song@...ux.dev>,
 "Oscar Salvador" <osalvador@...e.de>,
 "David Hildenbrand" <david@...hat.com>,
 "David Woodhouse" <dwmw2@...radead.org>,
 "Dave Kleikamp" <shaggy@...nel.org>,
 "Trond Myklebust" <trondmy@...nel.org>,
 "Anna Schumaker" <anna@...nel.org>,
 "Ryusuke Konishi" <konishi.ryusuke@...il.com>,
 "Konstantin Komarov" <almaz.alexandrovich@...agon-software.com>,
 "Mark Fasheh" <mark@...heh.com>, "Joel Becker" <jlbec@...lplan.org>,
 "Joseph Qi" <joseph.qi@...ux.alibaba.com>,
 "Bob Copeland" <me@...copeland.com>,
 "Mike Marshall" <hubcap@...ibond.com>,
 "Martin Brandenburg" <martin@...ibond.com>,
 "Amir Goldstein" <amir73il@...il.com>,
 "Steve French" <sfrench@...ba.org>, "Paulo Alcantara" <pc@...guebit.org>,
 "Ronnie Sahlberg" <ronniesahlberg@...il.com>,
 "Shyam Prasad N" <sprasad@...rosoft.com>, "Tom Talpey" <tom@...pey.com>,
 "Bharath SM" <bharathsm@...rosoft.com>,
 "Zhihao Cheng" <chengzhihao1@...wei.com>,
 "Hans de Goede" <hansg@...nel.org>, "Carlos Maiolino" <cem@...nel.org>,
 "Hugh Dickins" <hughd@...gle.com>,
 "Baolin Wang" <baolin.wang@...ux.alibaba.com>,
 "Andrew Morton" <akpm@...ux-foundation.org>,
 "Kees Cook" <kees@...nel.org>,
 "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 linux-kernel@...r.kernel.org, v9fs@...ts.linux.dev,
 linux-fsdevel@...r.kernel.org, linux-afs@...ts.infradead.org,
 linux-btrfs@...r.kernel.org, ceph-devel@...r.kernel.org,
 codalist@...a.cs.cmu.edu, ecryptfs@...r.kernel.org,
 linux-efi@...r.kernel.org, linux-ext4@...r.kernel.org,
 linux-f2fs-devel@...ts.sourceforge.net, gfs2@...ts.linux.dev,
 linux-um@...ts.infradead.org, linux-mm@...ck.org,
 linux-mtd@...ts.infradead.org, jfs-discussion@...ts.sourceforge.net,
 linux-nfs@...r.kernel.org, linux-nilfs@...r.kernel.org,
 ntfs3@...ts.linux.dev, ocfs2-devel@...ts.linux.dev,
 linux-karma-devel@...ts.sourceforge.net, devel@...ts.orangefs.org,
 linux-unionfs@...r.kernel.org, linux-cifs@...r.kernel.org,
 samba-technical@...ts.samba.org, linux-xfs@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject:
 Re: [PATCH] vfs: remove the excl argument from the ->create() inode_operation

On Fri, 07 Nov 2025, Jeff Layton wrote:
> 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.

Thanks, that is a substantial improvement.  I see your point now and I
think this is a really nice cleanup to make - thanks.

I think the commit message could be improved further by leading with the
detail that is central - that most ->create function ignore 'excl'.

 With two exceptions, ->create() methods provided by filesystems ignore
 the "excl" flag.  Those exception are NFS and GFS2 which both also
 provide ->atomic_open.

 excl is always true when ->create is called from vfs_create() (since
 commit......) so the only time it can be false is when it is called by
 lookup_open() for filesystems that do not provide ->atomic_open.

 So the excl flag to ->create is either ignored or true.  So we can
 remove it and change NFS and GFS2 to acts as though it were true.

> 
> Maybe we also need some comments or updates to Documentation/ to make
> it clear that ->create() always implies O_EXCL semantics?

Definitely, something in porting.rst and something in vfs.rst.

I would be worth saying somewhere that if the fs needs to mediate
non-exclusive creation, it must provide atomic_open().

Thanks,
NeilBrown


> -- 
> Jeff Layton <jlayton@...nel.org>
> 

Powered by blists - more mailing lists