[RFC PATCH 2/5] statsfs API: create, add and remove statsfs

[Emanuele Giuseppe Esposito <eesposit@redhat.com>]

Hi Mattew,
I am trying to apply your Xarrays suggestion, but I don't
understand how to make them properly work. In particular, the __xa_alloc
function always returns -EINVAL.

I tried to follow the Xarrays kernel doc and the example you provided to
replace the subordinates linked list, but alloc always returns that error.

Below you can find the changes I intended to do.
Can you help me?

Thank you,
Emanuele

------ 8< -----------
From ad5d20b6ce7995b2d1164104cf958f7bc3e692fa Mon Sep 17 00:00:00 2001
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Date: Tue, 28 Apr 2020 12:21:00 +0200
Subject: [PATCH] statsfs: switch subordinate sources to xarray

---
 fs/statsfs/statsfs.c    | 45 +++++++++++++++++++++++++++--------------
 include/linux/statsfs.h |  5 ++---
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/fs/statsfs/statsfs.c b/fs/statsfs/statsfs.c
index c8cfa590a3b0..0cf135c36776 100644
--- a/fs/statsfs/statsfs.c
+++ b/fs/statsfs/statsfs.c
@@ -107,11 +107,12 @@ const struct file_operations statsfs_ops = {
 static void statsfs_source_remove_files_locked(struct statsfs_source *src)
 {
 	struct statsfs_source *child;
+	unsigned long index;
 
 	if (src->source_dentry == NULL)
 		return;
 
-	list_for_each_entry(child, &src->subordinates_head, list_element)
+	xa_for_each (&src->subordinates, index, child)
 		statsfs_source_remove_files(child);
 
 	statsfs_remove_recursive(src->source_dentry);
@@ -180,6 +181,7 @@ static void statsfs_create_files_recursive_locked(struct statsfs_source *source,
 						  struct dentry *parent_dentry)
 {
 	struct statsfs_source *child;
+	unsigned long index;
 
 	/* first check values in this folder, since it might be new */
 	if (!source->source_dentry) {
@@ -189,7 +191,7 @@ static void statsfs_create_files_recursive_locked(struct statsfs_source *source,
 
 	statsfs_create_files_locked(source);
 
-	list_for_each_entry(child, &source->subordinates_head, list_element) {
+	xa_for_each (&source->subordinates, index, child) {
 		if (child->source_dentry == NULL) {
 			/* assume that if child has a folder,
 			 * also the sub-child have that.
@@ -258,10 +260,23 @@ EXPORT_SYMBOL_GPL(statsfs_source_add_values);
 void statsfs_source_add_subordinate(struct statsfs_source *source,
 				    struct statsfs_source *sub)
 {
+	int err;
+	uint32_t index;
+
 	down_write(&source->rwsem);
 
 	statsfs_source_get(sub);
-	list_add(&sub->list_element, &source->subordinates_head);
+	err = __xa_alloc(&source->subordinates, &index, sub, xa_limit_32b,
+		       GFP_KERNEL);
+
+	if (err) {
+		printk(KERN_ERR "Failed to insert subordinate %s\n"
+			"Too many subordinates in source %s\n",
+			sub->name, source->name);
+		up_write(&source->rwsem);
+		return;
+	}
+
 	if (source->source_dentry)
 		statsfs_create_files_recursive_locked(sub,
 						      source->source_dentry);
@@ -276,10 +291,11 @@ statsfs_source_remove_subordinate_locked(struct statsfs_source *source,
 					 struct statsfs_source *sub)
 {
 	struct statsfs_source *src_entry;
+	unsigned long index;
 
-	list_for_each_entry(src_entry, &source->subordinates_head, list_element) {
+	xa_for_each (&source->subordinates, index, src_entry) {
 		if (src_entry == sub) {
-			list_del_init(&src_entry->list_element);
+			xa_erase(&source->subordinates, index);
 			statsfs_source_remove_files(src_entry);
 			statsfs_source_put(src_entry);
 			return;
@@ -431,13 +447,13 @@ static void do_recursive_aggregation(struct statsfs_source *root,
 				     struct statsfs_aggregate_value *agg)
 {
 	struct statsfs_source *subordinate;
+	unsigned long index;
 
 	/* search all simple values in this folder */
 	search_all_simple_values(root, ref_src_entry, val, agg);
 
 	/* recursively search in all subfolders */
-	list_for_each_entry(subordinate, &root->subordinates_head,
-			     list_element) {
+	xa_for_each (&root->subordinates, index, subordinate) {
 		down_read(&subordinate->rwsem);
 		do_recursive_aggregation(subordinate, ref_src_entry, val, agg);
 		up_read(&subordinate->rwsem);
@@ -571,13 +587,13 @@ static void do_recursive_clean(struct statsfs_source *root,
 			       struct statsfs_value *val)
 {
 	struct statsfs_source *subordinate;
+	unsigned long index;
 
 	/* search all simple values in this folder */
 	set_all_simple_values(root, ref_src_entry, val);
 
 	/* recursively search in all subfolders */
-	list_for_each_entry(subordinate, &root->subordinates_head,
-			     list_element) {
+	xa_for_each (&root->subordinates, index, subordinate) {
 		down_read(&subordinate->rwsem);
 		do_recursive_clean(subordinate, ref_src_entry, val);
 		up_read(&subordinate->rwsem);
@@ -703,9 +719,10 @@ EXPORT_SYMBOL_GPL(statsfs_source_revoke);
  */
 static void statsfs_source_destroy(struct kref *kref_source)
 {
-	struct statsfs_value_source *val_src_entry;
 	struct list_head *it, *safe;
+	struct statsfs_value_source *val_src_entry;
 	struct statsfs_source *child, *source;
+	unsigned long index;
 
 	source = container_of(kref_source, struct statsfs_source, refcount);
 
@@ -717,15 +734,14 @@ static void statsfs_source_destroy(struct kref *kref_source)
 	}
 
 	/* iterate through the subordinates and delete them */
-	list_for_each_safe(it, safe, &source->subordinates_head) {
-		child = list_entry(it, struct statsfs_source, list_element);
+	xa_for_each (&source->subordinates, index, child)
 		statsfs_source_remove_subordinate_locked(source, child);
-	}
 
 	statsfs_source_remove_files_locked(source);
 
 	up_write(&source->rwsem);
 	kfree(source->name);
+	xa_destroy(&source->subordinates);
 	kfree(source);
 }
 
@@ -761,8 +777,7 @@ struct statsfs_source *statsfs_source_create(const char *fmt, ...)
 	init_rwsem(&ret->rwsem);
 
 	INIT_LIST_HEAD(&ret->values_head);
-	INIT_LIST_HEAD(&ret->subordinates_head);
-	INIT_LIST_HEAD(&ret->list_element);
+	xa_init(&ret->subordinates);
 
 	return ret;
 }
diff --git a/include/linux/statsfs.h b/include/linux/statsfs.h
index f6e8eead1124..20153f50ffc0 100644
--- a/include/linux/statsfs.h
+++ b/include/linux/statsfs.h
@@ -11,6 +11,7 @@
 #define _STATSFS_H_
 
 #include <linux/list.h>
+#include <linux/xarray.h>
 
 /* Used to distinguish signed types */
 #define STATSFS_SIGN 0x8000
@@ -64,9 +65,7 @@ struct statsfs_source {
 	struct list_head values_head;
 
 	/* list of struct statsfs_source for subordinate sources */
-	struct list_head subordinates_head;
-
-	struct list_head list_element;
+	struct xarray subordinates;
 
 	struct rw_semaphore rwsem;
 
-- 
2.25.2