Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
From: Jung-JaeJoon <rgbi3307@gmail.com>
To: "Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Matthew Wilcox <willy@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peng Zhang <zhangpeng.00@bytedance.com>
Cc: Jung-JaeJoon <rgbi3307@gmail.com>,
	maple-tree@lists.infradead.org, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] Modified XArray entry bit flags as macro constants
Date: Fri, 24 May 2024 11:49:45 +0900	[thread overview]
Message-ID: <20240524024945.9309-1-rgbi3307@naver.com> (raw)

From: Jung-JaeJoon <rgbi3307@gmail.com>

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@gmail.com>

---
 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


             reply	other threads:[~2024-05-24  2:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-24  2:49 Jung-JaeJoon [this message]
2024-05-24  3:05 ` [PATCH] Modified XArray entry bit flags as macro constants Matthew Wilcox

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=20240524024945.9309-1-rgbi3307@naver.com \
    --to=rgbi3307@gmail.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=maple-tree@lists.infradead.org \
    --cc=willy@infradead.org \
    --cc=zhangpeng.00@bytedance.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).