Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 14 Feb 2023 22:06:31 GMT
From:      =?utf-8?Q?Jean-S=C3=A9bastien=20P=C3=A9dron?= <dumbbell@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 3102ea3b15b6 - main - linuxkpi: Accept NULL as a value in `linux_xarray`
Message-ID:  <202302142206.31EM6V9K004026@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
The branch main has been updated by dumbbell (ports committer):

URL: https://cgit.FreeBSD.org/src/commit/?id=3102ea3b15b6c3ed1ea50716d65980b680375ebc

commit 3102ea3b15b6c3ed1ea50716d65980b680375ebc
Author:     Jean-Sébastien Pédron <dumbbell@FreeBSD.org>
AuthorDate: 2023-02-11 10:12:08 +0000
Commit:     Jean-Sébastien Pédron <dumbbell@FreeBSD.org>
CommitDate: 2023-02-14 22:01:02 +0000

    linuxkpi: Accept NULL as a value in `linux_xarray`
    
    Linux' XArray allows to store a NULL pointer as a value. `xa_load()`
    would return NULL for both an unused index and an index set to NULL. But
    it impacts `xa_alloc()` which needs to find the next available index.
    
    However, our implementation relies on a radix tree (see `linux_radix.c`)
    which does not accept NULL pointers as values. I'm not sure if this is a
    limitation or a feature, so to work around this, a NULL value is
    replaced by `NULL_VALUE`, an unlikely address, when we pass it to
    linux_radix.
    
    Reviewed by:    emaste, manu
    Approved by:    emaste, manu
    Differential Revision:  https://reviews.freebsd.org/D38543
---
 sys/compat/linuxkpi/common/src/linux_xarray.c | 38 ++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/sys/compat/linuxkpi/common/src/linux_xarray.c b/sys/compat/linuxkpi/common/src/linux_xarray.c
index a41784103852..e8a107fa6f27 100644
--- a/sys/compat/linuxkpi/common/src/linux_xarray.c
+++ b/sys/compat/linuxkpi/common/src/linux_xarray.c
@@ -31,6 +31,18 @@ __FBSDID("$FreeBSD$");
 
 #include <vm/vm_pageout.h>
 
+/*
+ * Linux' XArray allows to store a NULL pointer as a value. xa_load() would
+ * return NULL for both an unused index and an index set to NULL. But it
+ * impacts xa_alloc() which needs to find the next available index.
+ *
+ * However, our implementation relies on a radix tree (see `linux_radix.c`)
+ * which does not accept NULL pointers as values. I'm not sure this is a
+ * limitation or a feature, so to work around this, a NULL value is replaced by
+ * `NULL_VALUE`, an unlikely address, when we pass it to linux_radix.
+ */
+#define	NULL_VALUE	(void *)0x1
+
 /*
  * This function removes the element at the given index and returns
  * the pointer to the removed element, if any.
@@ -38,9 +50,15 @@ __FBSDID("$FreeBSD$");
 void *
 __xa_erase(struct xarray *xa, uint32_t index)
 {
+	void *retval;
+
 	XA_ASSERT_LOCKED(xa);
 
-	return (radix_tree_delete(&xa->root, index));
+	retval = radix_tree_delete(&xa->root, index);
+	if (retval == NULL_VALUE)
+		retval = NULL;
+
+	return (retval);
 }
 
 void *
@@ -68,6 +86,9 @@ xa_load(struct xarray *xa, uint32_t index)
 	retval = radix_tree_lookup(&xa->root, index);
 	xa_unlock(xa);
 
+	if (retval == NULL_VALUE)
+		retval = NULL;
+
 	return (retval);
 }
 
@@ -109,6 +130,8 @@ __xa_alloc(struct xarray *xa, uint32_t *pindex, void *ptr, uint32_t mask, gfp_t
 	MPASS((mask & (mask + 1)) == 0);
 
 	*pindex = (xa->flags & XA_FLAGS_ALLOC1) != 0 ? 1 : 0;
+	if (ptr == NULL)
+		ptr = NULL_VALUE;
 retry:
 	retval = radix_tree_insert(&xa->root, *pindex, ptr);
 
@@ -137,6 +160,9 @@ xa_alloc(struct xarray *xa, uint32_t *pindex, void *ptr, uint32_t mask, gfp_t gf
 {
 	int retval;
 
+	if (ptr == NULL)
+		ptr = NULL_VALUE;
+
 	xa_lock(xa);
 	retval = __xa_alloc(xa, pindex, ptr, mask, gfp);
 	xa_unlock(xa);
@@ -166,6 +192,8 @@ __xa_alloc_cyclic(struct xarray *xa, uint32_t *pindex, void *ptr, uint32_t mask,
 	MPASS((mask & (mask + 1)) == 0);
 
 	*pnext_index = (xa->flags & XA_FLAGS_ALLOC1) != 0 ? 1 : 0;
+	if (ptr == NULL)
+		ptr = NULL_VALUE;
 retry:
 	retval = radix_tree_insert(&xa->root, *pnext_index, ptr);
 
@@ -220,6 +248,8 @@ __xa_insert(struct xarray *xa, uint32_t index, void *ptr, gfp_t gfp)
 	int retval;
 
 	XA_ASSERT_LOCKED(xa);
+	if (ptr == NULL)
+		ptr = NULL_VALUE;
 retry:
 	retval = radix_tree_insert(&xa->root, index, ptr);
 
@@ -262,11 +292,15 @@ __xa_store(struct xarray *xa, uint32_t index, void *ptr, gfp_t gfp)
 	int retval;
 
 	XA_ASSERT_LOCKED(xa);
+	if (ptr == NULL)
+		ptr = NULL_VALUE;
 retry:
 	retval = radix_tree_store(&xa->root, index, &ptr);
 
 	switch (retval) {
 	case 0:
+		if (ptr == NULL_VALUE)
+			ptr = NULL;
 		break;
 	case -ENOMEM:
 		if (likely(gfp & M_WAITOK)) {
@@ -374,6 +408,8 @@ __xa_next(struct xarray *xa, unsigned long *pindex, bool not_first)
 	found = radix_tree_iter_find(&xa->root, &iter, &ppslot);
 	if (likely(found)) {
 		retval = *ppslot;
+		if (retval == NULL_VALUE)
+			retval = NULL;
 		*pindex = iter.index;
 	} else {
 		retval = NULL;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202302142206.31EM6V9K004026>