Re: [PATCH] tracefs/eventfs: Use root and instance inodes as default ownership

On Thu, 4 Jan 2024 01:59:10 +0000
Al Viro [off-list ref] wrote:

On Wed, Jan 03, 2024 at 08:32:46PM -0500, Steven Rostedt wrote:

quoted

+static struct inode *instance_inode(struct dentry *parent, struct inode *inode)
+{
+	struct tracefs_inode *ti;
+	struct inode *root_inode;
+
+	root_inode = d_inode(inode->i_sb->s_root);
+
+	/* If parent is NULL then use root inode */
+	if (!parent)
+		return root_inode;
+
+	/* Find the inode that is flagged as an instance or the root inode */
+	do {
+		inode = d_inode(parent);
+		if (inode == root_inode)
+			return root_inode;
+
+		ti = get_tracefs(inode);
+
+		if (ti->flags & TRACEFS_INSTANCE_INODE)
+			return inode;
+	} while ((parent = parent->d_parent));  

*blink*

This is equivalent to
		...
		parent = parent->d_parent;
	} while (true);

Yeah, that loop went through a few iterations as I first thought that root
was a tracefs_inode and the get_tracefs() would work on it. No, it was not,
and it caused a cash. But I didn't rewrite the loop well after fixing that.

I was also not sure if parent could be NULL, and wanted to protect against it.

->d_parent is *never* NULL.  And what the hell is that loop supposed to do,  
anyway?  Find the nearest ancestor tagged with TRACEFS_INSTANCE_INODE?

If root is not marked that way, I would suggest
	if (!parent)
		parent = inode->i_sb->s_root;
	while (!IS_ROOT(parent)) {
		struct tracefs_inode *ti = get_tracefs(parent->d_inode);
		if (ti->flags & TRACEFS_INSTANCE_INODE)
			break;
		parent = parent->d_parent;
	}
	return parent->d_inode;

Sure, I could rewrite it that way too.

Thanks,

-- Steve