diff mbox series

[2/2] afs: Make /afs/@cell and /afs/.@cell symlinks

Message ID 20250107142513.527300-3-dhowells@redhat.com (mailing list archive)
State New
Headers show
Series afs: Dynamic root improvements | expand

Commit Message

David Howells Jan. 7, 2025, 2:25 p.m. UTC
Make /afs/@cell a symlink in the /afs dynamic root to match what other AFS
clients do rather than doing a substitution in the dentry name.  This has
the bonus of being tab-expandable also.

Further, provide a /afs/.@cell symlink to point to the dotted cell share.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 fs/afs/dynroot.c           | 140 +++++++++++++++++++++++++++++--------
 include/trace/events/afs.h |   2 +
 2 files changed, 112 insertions(+), 30 deletions(-)

Comments

Al Viro Jan. 7, 2025, 3:20 p.m. UTC | #1
On Tue, Jan 07, 2025 at 02:25:09PM +0000, David Howells wrote:

> @@ -247,9 +285,13 @@ static struct dentry *afs_dynroot_lookup(struct inode *dir, struct dentry *dentr
>  		return ERR_PTR(-ENAMETOOLONG);
>  	}
>  
> -	if (dentry->d_name.len == 5 &&
> -	    memcmp(dentry->d_name.name, "@cell", 5) == 0)
> -		return afs_lookup_atcell(dentry);
> +	if (dentry->d_name.len == sizeof(afs_atcell) - 1 &&
> +	    memcmp(dentry->d_name.name, afs_atcell, sizeof(afs_atcell) - 1) == 0)
> +		return afs_lookup_atcell(dentry, false);
> +
> +	if (dentry->d_name.len == sizeof(afs_dotatcell) - 1 &&
> +	    memcmp(dentry->d_name.name, afs_dotatcell, sizeof(afs_dotatcell) - 1) == 0)
> +		return afs_lookup_atcell(dentry, true);

Ow...  That looks just painful.

>  	return d_splice_alias(afs_try_auto_mntpt(dentry, dir), dentry);
>  }
> @@ -343,6 +385,40 @@ void afs_dynroot_rmdir(struct afs_net *net, struct afs_cell *cell)

> +static int afs_dynroot_symlink(struct afs_net *net)
> +{
> +	struct super_block *sb = net->dynroot_sb;
> +	struct dentry *root, *symlink, *dsymlink;
> +	int ret;
> +
> +	/* Let the ->lookup op do the creation */
> +	root = sb->s_root;
> +	inode_lock(root->d_inode);
> +	symlink = lookup_one_len(afs_atcell, root, sizeof(afs_atcell) - 1);
> +	if (IS_ERR(symlink)) {
> +		ret = PTR_ERR(symlink);
> +		goto unlock;
> +	}
> +
> +	dsymlink = lookup_one_len(afs_dotatcell, root, sizeof(afs_dotatcell) - 1);
> +	if (IS_ERR(dsymlink)) {
> +		ret = PTR_ERR(dsymlink);
> +		dput(symlink);
> +		goto unlock;
> +	}

Just allocate those child dentries and call your afs_lookup_atcell() for them.
No need to keep that mess in ->lookup() - you are keeping those suckers cached
now, so...
David Howells Jan. 7, 2025, 3:36 p.m. UTC | #2
Al Viro <viro@zeniv.linux.org.uk> wrote:

> > +	dsymlink = lookup_one_len(afs_dotatcell, root, sizeof(afs_dotatcell) - 1);
> > +	if (IS_ERR(dsymlink)) {
> > +		ret = PTR_ERR(dsymlink);
> > +		dput(symlink);
> > +		goto unlock;
> > +	}
> 
> Just allocate those child dentries and call your afs_lookup_atcell() for them.
> No need to keep that mess in ->lookup() - you are keeping those suckers cached
> now, so...

Good point.  I need to do that for the cell mountpoints because someone can
arbitrarily create one by triggering a lookup of /afs/<cell>/, but for this
pair of symlinks, they're created before the sb goes live.

David
David Howells Jan. 7, 2025, 5:56 p.m. UTC | #3
Al Viro <viro@zeniv.linux.org.uk> wrote:

> Just allocate those child dentries and call your afs_lookup_atcell() for them.
> No need to keep that mess in ->lookup() - you are keeping those suckers cached
> now, so...

Actually, I wonder if creating the inodes and dentries for cells and the @cell
symlinks at mount of the dynamic root or when the cells are created is
actually the best way.

It might be better to list all the cells and symlinks in readdir and only
create them on demand in ->lookup().  The cells are kept in their own list on
the network namespace anyway, even if the dynamic root isn't mounted.

David
diff mbox series

Patch

diff --git a/fs/afs/dynroot.c b/fs/afs/dynroot.c
index f80a4244b9d2..5a53631239f1 100644
--- a/fs/afs/dynroot.c
+++ b/fs/afs/dynroot.c
@@ -11,6 +11,8 @@ 
 #include "internal.h"
 
 static atomic_t afs_autocell_ino;
+static const char afs_atcell[] = "@cell";
+static const char afs_dotatcell[] = ".@cell";
 
 /*
  * iget5() comparator for inode created by autocell operations
@@ -185,48 +187,84 @@  struct inode *afs_try_auto_mntpt(struct dentry *dentry, struct inode *dir)
 	return ret == -ENOENT ? NULL : ERR_PTR(ret);
 }
 
+static void afs_atcell_delayed_put_cell(void *arg)
+{
+	struct afs_cell *cell = arg;
+
+	afs_put_cell(cell, afs_cell_trace_put_atcell);
+}
+
 /*
- * Look up @cell in a dynroot directory.  This is a substitution for the
- * local cell name for the net namespace.
+ * Read @cell or .@cell symlinks.
  */
-static struct dentry *afs_lookup_atcell(struct dentry *dentry)
+static const char *afs_atcell_get_link(struct dentry *dentry, struct inode *inode,
+				       struct delayed_call *done)
 {
+	struct afs_vnode *vnode = AFS_FS_I(inode);
 	struct afs_cell *cell;
-	struct afs_net *net = afs_d2net(dentry);
-	struct dentry *ret;
-	char *name;
-	int len;
+	struct afs_net *net = afs_i2net(inode);
+	const char *name;
+	bool dotted = vnode->fid.vnode == 3;
 
 	if (!net->ws_cell)
 		return ERR_PTR(-ENOENT);
 
-	ret = ERR_PTR(-ENOMEM);
-	name = kmalloc(AFS_MAXCELLNAME + 1, GFP_KERNEL);
-	if (!name)
-		goto out_p;
-
 	down_read(&net->cells_lock);
+
 	cell = net->ws_cell;
-	if (cell) {
-		len = cell->name_len;
-		memcpy(name, cell->name, len + 1);
-	}
+	if (dotted)
+		name = cell->name - 1;
+	else
+		name = cell->name;
+	afs_get_cell(cell, afs_cell_trace_get_atcell);
+	set_delayed_call(done, afs_atcell_delayed_put_cell, cell);
+
 	up_read(&net->cells_lock);
+	return name;
+}
 
-	ret = ERR_PTR(-ENOENT);
-	if (!cell)
-		goto out_n;
+static const struct inode_operations afs_atcell_inode_operations = {
+	.get_link	= afs_atcell_get_link,
+};
 
-	ret = lookup_one_len(name, dentry->d_parent, len);
+/*
+ * Look up @cell or .@cell in a dynroot directory.  This is a substitution for
+ * the local cell name for the net namespace.
+ */
+static struct dentry *afs_lookup_atcell(struct dentry *dentry, bool dotted)
+{
+	struct afs_vnode *vnode;
+	struct afs_fid fid = { .vnode = 2, .unique = 1, };
+	struct inode *inode;
 
-	/* We don't want to d_add() the @cell dentry here as we don't want to
-	 * the cached dentry to hide changes to the local cell name.
-	 */
+	if (dotted)
+		fid.vnode = 3;
 
-out_n:
-	kfree(name);
-out_p:
-	return ret;
+
+	inode = iget5_locked(dentry->d_sb, fid.vnode,
+			     afs_iget5_pseudo_test, afs_iget5_pseudo_set, &fid);
+	if (!inode)
+		return ERR_PTR(-ENOMEM);
+
+	vnode = AFS_FS_I(inode);
+
+	/* there shouldn't be an existing inode */
+	BUG_ON(!(inode->i_state & I_NEW));
+
+	netfs_inode_init(&vnode->netfs, NULL, false);
+	simple_inode_init_ts(inode);
+	set_nlink(inode, 1);
+	inode->i_size		= 0;
+	inode->i_mode		= S_IFLNK | S_IRUGO | S_IXUGO;
+	inode->i_op		= &afs_atcell_inode_operations;
+	inode->i_uid		= GLOBAL_ROOT_UID;
+	inode->i_gid		= GLOBAL_ROOT_GID;
+	inode->i_blocks		= 0;
+	inode->i_generation	= 0;
+	inode->i_flags		|= S_NOATIME;
+
+	unlock_new_inode(inode);
+	return d_splice_alias(inode, dentry);
 }
 
 /*
@@ -247,9 +285,13 @@  static struct dentry *afs_dynroot_lookup(struct inode *dir, struct dentry *dentr
 		return ERR_PTR(-ENAMETOOLONG);
 	}
 
-	if (dentry->d_name.len == 5 &&
-	    memcmp(dentry->d_name.name, "@cell", 5) == 0)
-		return afs_lookup_atcell(dentry);
+	if (dentry->d_name.len == sizeof(afs_atcell) - 1 &&
+	    memcmp(dentry->d_name.name, afs_atcell, sizeof(afs_atcell) - 1) == 0)
+		return afs_lookup_atcell(dentry, false);
+
+	if (dentry->d_name.len == sizeof(afs_dotatcell) - 1 &&
+	    memcmp(dentry->d_name.name, afs_dotatcell, sizeof(afs_dotatcell) - 1) == 0)
+		return afs_lookup_atcell(dentry, true);
 
 	return d_splice_alias(afs_try_auto_mntpt(dentry, dir), dentry);
 }
@@ -343,6 +385,40 @@  void afs_dynroot_rmdir(struct afs_net *net, struct afs_cell *cell)
 	_leave("");
 }
 
+/*
+ * Create @cell and .@cell symlinks.
+ */
+static int afs_dynroot_symlink(struct afs_net *net)
+{
+	struct super_block *sb = net->dynroot_sb;
+	struct dentry *root, *symlink, *dsymlink;
+	int ret;
+
+	/* Let the ->lookup op do the creation */
+	root = sb->s_root;
+	inode_lock(root->d_inode);
+	symlink = lookup_one_len(afs_atcell, root, sizeof(afs_atcell) - 1);
+	if (IS_ERR(symlink)) {
+		ret = PTR_ERR(symlink);
+		goto unlock;
+	}
+
+	dsymlink = lookup_one_len(afs_dotatcell, root, sizeof(afs_dotatcell) - 1);
+	if (IS_ERR(dsymlink)) {
+		ret = PTR_ERR(dsymlink);
+		dput(symlink);
+		goto unlock;
+	}
+
+	/* Note that we're retaining extra refs on the dentries. */
+	symlink->d_fsdata = (void *)1UL;
+	dsymlink->d_fsdata = (void *)1UL;
+	ret = 0;
+unlock:
+	inode_unlock(root->d_inode);
+	return ret;
+}
+
 /*
  * Populate a newly created dynamic root with cell names.
  */
@@ -355,6 +431,10 @@  int afs_dynroot_populate(struct super_block *sb)
 	mutex_lock(&net->proc_cells_lock);
 
 	net->dynroot_sb = sb;
+	ret = afs_dynroot_symlink(net);
+	if (ret < 0)
+		goto error;
+
 	hlist_for_each_entry(cell, &net->proc_cells, proc_link) {
 		ret = afs_dynroot_mkdir(net, cell);
 		if (ret < 0)
diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h
index a0aed1a428a1..de0e2301a037 100644
--- a/include/trace/events/afs.h
+++ b/include/trace/events/afs.h
@@ -168,12 +168,14 @@  enum yfs_cm_operation {
 #define afs_cell_traces \
 	EM(afs_cell_trace_alloc,		"ALLOC     ") \
 	EM(afs_cell_trace_free,			"FREE      ") \
+	EM(afs_cell_trace_get_atcell,		"GET atcell") \
 	EM(afs_cell_trace_get_queue_dns,	"GET q-dns ") \
 	EM(afs_cell_trace_get_queue_manage,	"GET q-mng ") \
 	EM(afs_cell_trace_get_queue_new,	"GET q-new ") \
 	EM(afs_cell_trace_get_vol,		"GET vol   ") \
 	EM(afs_cell_trace_insert,		"INSERT    ") \
 	EM(afs_cell_trace_manage,		"MANAGE    ") \
+	EM(afs_cell_trace_put_atcell,		"PUT atcell") \
 	EM(afs_cell_trace_put_candidate,	"PUT candid") \
 	EM(afs_cell_trace_put_destroy,		"PUT destry") \
 	EM(afs_cell_trace_put_queue_work,	"PUT q-work") \