From: Jung-JaeJoon <rgbi3307@xxxxxxxxx>
It would be better to modify the operation on the last two bits of the entry
with a macro constant name rather than using a numeric constant.
#define XA_VALUE_ENTRY 1UL
#define XA_INTERNAL_ENTRY 2UL
#define XA_POINTER_ENTRY 3UL
In particular, in the xa_to_node() function, it is more consistent and efficient
to perform a logical AND operation as shown below than a subtraction operation.
- return (struct xa_node *)((unsigned long)entry - 2);
+ return (struct xa_node *)((unsigned long)entry & ~XA_INTERNAL_ENTRY);
Additionally, it is better to modify the if condition below
in the mas_store_root() function of lib/maple_tree.c to the xa_is_internal() inline function.
- else if (((unsigned long) (entry) & 3) == 2)
+ else if (xa_is_internal(entry))
And there is no reason to declare XA_CHECK_SCHED as an enum data type.
-enum {
- XA_CHECK_SCHED = 4096,
-};
+#define XA_CHECK_SCHED 4096
Signed-off-by: JaeJoon Jung <rgbi3307@xxxxxxxxx>
---
include/linux/xarray.h | 55 ++++++++++++++++++++++--------------------
lib/maple_tree.c | 2 +-
2 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/include/linux/xarray.h b/include/linux/xarray.h
index cb571dfcf4b1..d73dfe35a005 100644
--- a/include/linux/xarray.h
+++ b/include/linux/xarray.h
@@ -42,6 +42,16 @@
* returned by the normal API.
*/
+#define XA_VALUE_ENTRY 1UL
+#define XA_INTERNAL_ENTRY 2UL
+#define XA_POINTER_ENTRY 3UL
+
+/*
+ * If iterating while holding a lock, drop the lock and reschedule
+ * every %XA_CHECK_SCHED loops.
+ */
+#define XA_CHECK_SCHED 4096
+
#define BITS_PER_XA_VALUE (BITS_PER_LONG - 1)
/**
@@ -54,7 +64,7 @@
static inline void *xa_mk_value(unsigned long v)
{
WARN_ON((long)v < 0);
- return (void *)((v << 1) | 1);
+ return (void *)((v << XA_VALUE_ENTRY) | XA_VALUE_ENTRY);
}
/**
@@ -66,7 +76,7 @@ static inline void *xa_mk_value(unsigned long v)
*/
static inline unsigned long xa_to_value(const void *entry)
{
- return (unsigned long)entry >> 1;
+ return (unsigned long)entry >> XA_VALUE_ENTRY;
}
/**
@@ -78,7 +88,7 @@ static inline unsigned long xa_to_value(const void *entry)
*/
static inline bool xa_is_value(const void *entry)
{
- return (unsigned long)entry & 1;
+ return (unsigned long)entry & XA_VALUE_ENTRY;
}
/**
@@ -111,7 +121,7 @@ static inline void *xa_tag_pointer(void *p, unsigned long tag)
*/
static inline void *xa_untag_pointer(void *entry)
{
- return (void *)((unsigned long)entry & ~3UL);
+ return (void *)((unsigned long)entry & ~XA_POINTER_ENTRY);
}
/**
@@ -126,7 +136,7 @@ static inline void *xa_untag_pointer(void *entry)
*/
static inline unsigned int xa_pointer_tag(void *entry)
{
- return (unsigned long)entry & 3UL;
+ return (unsigned long)entry & XA_POINTER_ENTRY;
}
/*
@@ -144,7 +154,7 @@ static inline unsigned int xa_pointer_tag(void *entry)
*/
static inline void *xa_mk_internal(unsigned long v)
{
- return (void *)((v << 2) | 2);
+ return (void *)((v << XA_INTERNAL_ENTRY) | XA_INTERNAL_ENTRY);
}
/*
@@ -156,7 +166,7 @@ static inline void *xa_mk_internal(unsigned long v)
*/
static inline unsigned long xa_to_internal(const void *entry)
{
- return (unsigned long)entry >> 2;
+ return (unsigned long)entry >> XA_INTERNAL_ENTRY;
}
/*
@@ -168,7 +178,7 @@ static inline unsigned long xa_to_internal(const void *entry)
*/
static inline bool xa_is_internal(const void *entry)
{
- return ((unsigned long)entry & 3) == 2;
+ return ((unsigned long)entry & XA_POINTER_ENTRY) == XA_INTERNAL_ENTRY;
}
#define XA_ZERO_ENTRY xa_mk_internal(257)
@@ -220,7 +230,7 @@ static inline int xa_err(void *entry)
{
/* xa_to_internal() would not do sign extension. */
if (xa_is_err(entry))
- return (long)entry >> 2;
+ return (long)entry >> XA_INTERNAL_ENTRY;
return 0;
}
@@ -1245,19 +1255,19 @@ static inline struct xa_node *xa_parent_locked(const struct xarray *xa,
/* Private */
static inline void *xa_mk_node(const struct xa_node *node)
{
- return (void *)((unsigned long)node | 2);
+ return (void *)((unsigned long)node | XA_INTERNAL_ENTRY);
}
/* Private */
static inline struct xa_node *xa_to_node(const void *entry)
{
- return (struct xa_node *)((unsigned long)entry - 2);
+ return (struct xa_node *)((unsigned long)entry & ~XA_INTERNAL_ENTRY);
}
/* Private */
static inline bool xa_is_node(const void *entry)
{
- return xa_is_internal(entry) && (unsigned long)entry > 4096;
+ return xa_is_internal(entry) && (unsigned long)entry > XA_CHECK_SCHED;
}
/* Private */
@@ -1358,9 +1368,10 @@ struct xa_state {
* We encode errnos in the xas->xa_node. If an error has happened, we need to
* drop the lock to fix it, and once we've done so the xa_state is invalid.
*/
-#define XA_ERROR(errno) ((struct xa_node *)(((unsigned long)errno << 2) | 2UL))
-#define XAS_BOUNDS ((struct xa_node *)1UL)
-#define XAS_RESTART ((struct xa_node *)3UL)
+#define XA_ERROR(errno) ((struct xa_node *) \
+ (((unsigned long)errno << XA_INTERNAL_ENTRY) | XA_INTERNAL_ENTRY))
+#define XAS_BOUNDS ((struct xa_node *)XA_VALUE_ENTRY)
+#define XAS_RESTART ((struct xa_node *)XA_POINTER_ENTRY)
#define __XA_STATE(array, index, shift, sibs) { \
.xa = array, \
@@ -1449,7 +1460,7 @@ static inline void xas_set_err(struct xa_state *xas, long err)
*/
static inline bool xas_invalid(const struct xa_state *xas)
{
- return (unsigned long)xas->xa_node & 3;
+ return (unsigned long)xas->xa_node & XA_POINTER_ENTRY;
}
/**
@@ -1477,13 +1488,13 @@ static inline bool xas_is_node(const struct xa_state *xas)
/* True if the pointer is something other than a node */
static inline bool xas_not_node(struct xa_node *node)
{
- return ((unsigned long)node & 3) || !node;
+ return ((unsigned long)node & XA_POINTER_ENTRY) || !node;
}
/* True if the node represents RESTART or an error */
static inline bool xas_frozen(struct xa_node *node)
{
- return (unsigned long)node & 2;
+ return (unsigned long)node & XA_INTERNAL_ENTRY;
}
/* True if the node represents head-of-tree, RESTART or BOUNDS */
@@ -1764,14 +1775,6 @@ static inline void *xas_next_marked(struct xa_state *xas, unsigned long max,
return entry;
}
-/*
- * If iterating while holding a lock, drop the lock and reschedule
- * every %XA_CHECK_SCHED loops.
- */
-enum {
- XA_CHECK_SCHED = 4096,
-};
-
/**
* xas_for_each() - Iterate over a range of an XArray.
* @xas: XArray operation state.
diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 2d7d27e6ae3c..c08545f8b09b 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -3515,7 +3515,7 @@ static inline void mas_store_root(struct ma_state *mas, void *entry)
{
if (likely((mas->last != 0) || (mas->index != 0)))
mas_root_expand(mas, entry);
- else if (((unsigned long) (entry) & 3) == 2)
+ else if (xa_is_internal(entry))
mas_root_expand(mas, entry);
else {
rcu_assign_pointer(mas->tree->ma_root, entry);
--
2.17.1
