[PATCH 0/2] afs: Dynamic root improvements

[PATCH 0/2] afs: Dynamic root improvements

[David Howells]

Here's a pair of patches to make a number of improvements to the AFS
dynamic root:

 (1) Create an /afs/.<cell> mountpoint to match the /afs/<cell> mountpoint
     when a cell is created.

 (2) Change the handling of /afs/@cell from being a dentry name
     substitution at lookup time to making it a symlink to the current cell
     name and also provide a /afs/.@cell symlink to point to the dotted
     cell mountpoint.

The patches are here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=afs-next

Thanks,
David

David Howells (2):
  afs: Make /afs/.<cell> as well /afs/<cell> mountpoints
  afs: Make /afs/@cell and /afs/.@cell symlinks

 fs/afs/cell.c              |  13 ++-
 fs/afs/dynroot.c           | 190 +++++++++++++++++++++++++++----------
 include/trace/events/afs.h |   2 +
 3 files changed, 152 insertions(+), 53 deletions(-)

[PATCH 1/2] afs: Make /afs/.<cell> as well /afs/<cell> mountpoints

[David Howells]

When a cell is instantiated, automatically create an /afs/.<cell>
mountpoint to match the /afs/<cell> mountpoint to match other AFS clients.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 fs/afs/cell.c    | 13 +++++++-----
 fs/afs/dynroot.c | 52 ++++++++++++++++++++++++++++++------------------
 2 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/fs/afs/cell.c b/fs/afs/cell.c
index caa09875f520..1aba6d4d03a9 100644
--- a/fs/afs/cell.c
+++ b/fs/afs/cell.c
@@ -146,18 +146,20 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	cell->name = kmalloc(namelen + 1, GFP_KERNEL);
+	cell->name = kmalloc(1 + namelen + 1, GFP_KERNEL);
 	if (!cell->name) {
 		kfree(cell);
 		return ERR_PTR(-ENOMEM);
 	}
 
-	cell->net = net;
+	cell->name[0] = '.';
+	cell->name++;
 	cell->name_len = namelen;
 	for (i = 0; i < namelen; i++)
 		cell->name[i] = tolower(name[i]);
 	cell->name[i] = 0;
 
+	cell->net = net;
 	refcount_set(&cell->ref, 1);
 	atomic_set(&cell->active, 0);
 	INIT_WORK(&cell->manager, afs_manage_cell_work);
@@ -211,7 +213,7 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
 	if (ret == -EINVAL)
 		printk(KERN_ERR "kAFS: bad VL server IP address\n");
 error:
-	kfree(cell->name);
+	kfree(cell->name - 1);
 	kfree(cell);
 	_leave(" = %d", ret);
 	return ERR_PTR(ret);
@@ -502,7 +504,7 @@ static void afs_cell_destroy(struct rcu_head *rcu)
 	afs_put_vlserverlist(net, rcu_access_pointer(cell->vl_servers));
 	afs_unuse_cell(net, cell->alias_of, afs_cell_trace_unuse_alias);
 	key_put(cell->anonymous_key);
-	kfree(cell->name);
+	kfree(cell->name - 1);
 	kfree(cell);
 
 	afs_dec_cells_outstanding(net);
@@ -710,7 +712,8 @@ static void afs_deactivate_cell(struct afs_net *net, struct afs_cell *cell)
 	afs_proc_cell_remove(cell);
 
 	mutex_lock(&net->proc_cells_lock);
-	hlist_del_rcu(&cell->proc_link);
+	if (!hlist_unhashed(&cell->proc_link))
+		hlist_del_rcu(&cell->proc_link);
 	afs_dynroot_rmdir(net, cell);
 	mutex_unlock(&net->proc_cells_lock);
 
diff --git a/fs/afs/dynroot.c b/fs/afs/dynroot.c
index c4d2711e20ad..f80a4244b9d2 100644
--- a/fs/afs/dynroot.c
+++ b/fs/afs/dynroot.c
@@ -271,7 +271,8 @@ const struct dentry_operations afs_dynroot_dentry_operations = {
 int afs_dynroot_mkdir(struct afs_net *net, struct afs_cell *cell)
 {
 	struct super_block *sb = net->dynroot_sb;
-	struct dentry *root, *subdir;
+	struct dentry *root, *subdir, *dsubdir;
+	char *dotname = cell->name - 1;
 	int ret;
 
 	if (!sb || atomic_read(&sb->s_active) == 0)
@@ -286,34 +287,31 @@ int afs_dynroot_mkdir(struct afs_net *net, struct afs_cell *cell)
 		goto unlock;
 	}
 
-	/* Note that we're retaining an extra ref on the dentry */
+	dsubdir = lookup_one_len(dotname, root, cell->name_len + 1);
+	if (IS_ERR(dsubdir)) {
+		ret = PTR_ERR(dsubdir);
+		dput(subdir);
+		goto unlock;
+	}
+
+	/* Note that we're retaining extra refs on the dentries. */
 	subdir->d_fsdata = (void *)1UL;
+	dsubdir->d_fsdata = (void *)1UL;
 	ret = 0;
 unlock:
 	inode_unlock(root->d_inode);
 	return ret;
 }
 
-/*
- * Remove a manually added cell mount directory.
- * - The caller must hold net->proc_cells_lock
- */
-void afs_dynroot_rmdir(struct afs_net *net, struct afs_cell *cell)
+static void afs_dynroot_rm_one_dir(struct dentry *root, const char *name, size_t name_len)
 {
-	struct super_block *sb = net->dynroot_sb;
-	struct dentry *root, *subdir;
-
-	if (!sb || atomic_read(&sb->s_active) == 0)
-		return;
-
-	root = sb->s_root;
-	inode_lock(root->d_inode);
+	struct dentry *subdir;
 
 	/* Don't want to trigger a lookup call, which will re-add the cell */
-	subdir = try_lookup_one_len(cell->name, root, cell->name_len);
+	subdir = try_lookup_one_len(name, root, name_len);
 	if (IS_ERR_OR_NULL(subdir)) {
 		_debug("lookup %ld", PTR_ERR(subdir));
-		goto no_dentry;
+		return;
 	}
 
 	_debug("rmdir %pd %u", subdir, d_count(subdir));
@@ -324,8 +322,24 @@ void afs_dynroot_rmdir(struct afs_net *net, struct afs_cell *cell)
 		dput(subdir);
 	}
 	dput(subdir);
-no_dentry:
-	inode_unlock(root->d_inode);
+}
+
+/*
+ * Remove a manually added cell mount directory.
+ * - The caller must hold net->proc_cells_lock
+ */
+void afs_dynroot_rmdir(struct afs_net *net, struct afs_cell *cell)
+{
+	struct super_block *sb = net->dynroot_sb;
+	char *dotname = cell->name - 1;
+
+	if (!sb || atomic_read(&sb->s_active) == 0)
+		return;
+
+	inode_lock(sb->s_root->d_inode);
+	afs_dynroot_rm_one_dir(sb->s_root, cell->name, cell->name_len);
+	afs_dynroot_rm_one_dir(sb->s_root, dotname, cell->name_len + 1);
+	inode_unlock(sb->s_root->d_inode);
 	_leave("");
 }
 

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

[David Howells]

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(-)

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") \

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

[Al Viro]

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...

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

[David Howells]

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

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

[David Howells]

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