KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: kvm@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	mst@redhat.com, borntraeger@de.ibm.com,
	Paolo Bonzini <pbonzini@redhat.com>,
	Emanuele Giuseppe Esposito <eesposit@redhat.com>
Subject: [RFC PATCH 2/5] statsfs API: create, add and remove statsfs
Date: Wed, 29 Apr 2020 11:49:22 +0200
Message-ID: <20200429094922.55032-1-eesposit@redhat.com> (raw)
In-Reply-To: <20200427154727.GH29705@bombadil.infradead.org>

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


  parent reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-27 14:18 [RFC PATCH 0/5] Statsfs: a new ram-based file sytem for Linux kernel statistics Emanuele Giuseppe Esposito
2020-04-27 14:18 ` [RFC PATCH 1/5] refcount, kref: add dec-and-test wrappers for rw_semaphores Emanuele Giuseppe Esposito
2020-04-27 14:18 ` [RFC PATCH 2/5] statsfs API: create, add and remove statsfs sources and values Emanuele Giuseppe Esposito
2020-04-27 15:47   ` Matthew Wilcox
2020-04-27 16:48     ` Emanuele Giuseppe Esposito
2020-04-29  9:49     ` Emanuele Giuseppe Esposito [this message]
2020-04-27 21:53   ` Andreas Dilger
2020-04-29 10:55     ` Emanuele Giuseppe Esposito
2020-04-28 17:47   ` Randy Dunlap
2020-04-29 10:34     ` Paolo Bonzini
2020-04-27 14:18 ` [RFC PATCH 3/5] kunit: tests for statsfs API Emanuele Giuseppe Esposito
2020-04-28 17:50   ` Randy Dunlap
2020-04-27 14:18 ` [RFC PATCH 4/5] statsfs fs: virtual fs to show stats to the end-user Emanuele Giuseppe Esposito
2020-04-27 14:18 ` [RFC PATCH 5/5] kvm_main: replace debugfs with statsfs Emanuele Giuseppe Esposito
2020-04-28 17:56   ` Randy Dunlap
2020-04-29 10:34     ` Emanuele Giuseppe Esposito
2020-04-29 10:35       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429094922.55032-1-eesposit@redhat.com \
    --to=eesposit@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git