On Tue, 30 Jan 2024 23:26:21 +0000
Al Viro <v...@zeniv.linux.org.uk> wrote:
> On Tue, Jan 30, 2024 at 11:03:52AM -0800, Linus Torvalds wrote:
> > The dentry lookup for eventfs files was very broken, and had lots of
> > signs of the old situation where the filesystem names were all created
> > statically in the dentry tree, rather than being looked up dynamically
> > based on the eventfs data structures.
> >
> > You could see it in the naming - how it claimed to "create" dentries
> > rather than just look up the dentries that were given it.
> >
> > You could see it in various nonsensical and very incorrect operations,
> > like using "simple_lookup()" on the dentries that were passed in, which
> > only results in those dentries becoming negative dentries. Which meant
> > that any other lookup would possibly return ENOENT if it saw that
> > negative dentry before the data rwas then later filled in.
> >
> > You could see it in the immesnse amount of nonsensical code that didn't
> > actually just do lookups.
>
> > -static struct dentry *create_file(const char *name, umode_t mode,
> > +static struct dentry *lookup_file(struct dentry *dentry,
> > + umode_t mode,
> > struct eventfs_attr *attr,
> > - struct dentry *parent, void *data,
> > + void *data,
> > const struct file_operations *fop)
> > {
> > struct tracefs_inode *ti;
> > - struct dentry *dentry;
> > struct inode *inode;
> >
> > if (!(mode & S_IFMT))
> > @@ -307,12 +304,6 @@ static struct dentry *create_file(const char *name,
> > umode_t mode,
> > if (WARN_ON_ONCE(!S_ISREG(mode)))
> > return NULL;
> >
> > - WARN_ON_ONCE(!parent);
> > - dentry = eventfs_start_creating(name, parent);
>
> Used to lock the inode of parent.
Actually it's the tracefs_start_creating() locks the inode, the
eventfs_start_creating() doesn't.
-- Steve
>
> > if (unlikely(!inode))
> > return eventfs_failed_creating(dentry);
>
> ... and that still unlocks it.
>
> > @@ -331,29 +322,25 @@ static struct dentry *create_file(const char *name,
> > umode_t mode,
> > ti->flags = TRACEFS_EVENT_INODE;
> > ti->private = NULL; // Directories have 'ei', files
> > not
> >
> > - d_instantiate(dentry, inode);
> > + d_add(dentry, inode);
> > fsnotify_create(dentry->d_parent->d_inode, dentry);
> > return eventfs_end_creating(dentry);
>
> ... and so does this.
>
> > };
>
> Where has that inode_lock() gone and how could that possibly work?
