Re: [PATCH 3/6] tracefs: dentry lookup crapectomy
From: Al Viro <viro@zeniv.linux.org.uk>
Date: 2024-01-30 23:26:26
Also in:
lkml
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.
quoted hunk ↗ jump to hunk
-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.
if (unlikely(!inode)) return eventfs_failed_creating(dentry);
... and that still unlocks it.
quoted hunk ↗ jump to hunk
@@ -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?