linux-kernel - [PATCH] xarray: Document erasing entries during iteration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190212072958.17373-1-tobin@kernel.org>
Date:   Tue, 12 Feb 2019 18:29:58 +1100
From:   "Tobin C. Harding" <tobin@...nel.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     "Tobin C. Harding" <tobin@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] xarray: Document erasing entries during iteration

The XArray is, in a way, a replacement data structure for linked lists,
as such, on first use developers may wonder if it is safe to remove
items while iterating over the array.

For example, this is fine:

	DEFINE_XARRAY(things);

	void cleanup()
	{
		struct thing *thing;
		unsigned long index;

		xa_for_each(&things, index, thing)
			xa_erase(&things, index);
	}

Document this feature explicitly in the docs and also for the macro
definition.

Signed-off-by: Tobin C. Harding <tobin@...nel.org>
---

Hi Willy,

I had my first go using the XArray today and during that I wondered if
it was safe to remove items during iteration.  Conceptually it seems
fine and it seemed to work just fine in code - is this something people
should not be doing for any reason?  Is this the best way to traverse
the tree and get every thing just to erase it?  Are we even supposed to
be thinking this is a tree or should we just be thinking it is an array?

(As you might have guessed I _still_ don't know exactly how a radix tree
works :)

Oh, and FTR the XArray is hot - good effort man.

thanks,
Tobin.


 Documentation/core-api/xarray.rst | 3 ++-
 include/linux/xarray.h            | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/core-api/xarray.rst b/Documentation/core-api/xarray.rst
index 5d54b27c6eba..2578e0bdaa17 100644
--- a/Documentation/core-api/xarray.rst
+++ b/Documentation/core-api/xarray.rst
@@ -97,7 +97,8 @@ You can copy entries out of the XArray into a plain array by calling
 :c:func:`xa_extract`.  Or you can iterate over the present entries in
 the XArray by calling :c:func:`xa_for_each`.  You may prefer to use
 :c:func:`xa_find` or :c:func:`xa_find_after` to move to the next present
-entry in the XArray.
+entry in the XArray.  It is safe to call :c:func:`xa_release` on entries
+as you iterate over the array using :c:func:`xa_for_each`.
 
 Calling :c:func:`xa_store_range` stores the same entry in a range
 of indices.  If you do this, some of the other operations will behave
diff --git a/include/linux/xarray.h b/include/linux/xarray.h
index 5d9d318bcf7a..1f8974281a0a 100644
--- a/include/linux/xarray.h
+++ b/include/linux/xarray.h
@@ -407,6 +407,8 @@ static inline bool xa_marked(const struct xarray *xa, xa_mark_t mark)
  * you should use the xas_for_each() iterator instead.  The xas_for_each()
  * iterator will expand into more inline code than xa_for_each().
  *
+ * It is safe to erase entries from the XArray as you iterate over it.
+ *
  * Context: Any context.  Takes and releases the RCU lock.
  */
 #define xa_for_each(xa, index, entry) \
-- 
2.20.1