GNU/Weeb Mailing List <[email protected]>
 help / color / mirror / Atom feed
From: Hugh Dickins <[email protected]>
To: Liam Howlett <[email protected]>
Cc: Andrew Morton <[email protected]>,
	 kernel test robot <[email protected]>,
	 "[email protected]" <[email protected]>,
	 "[email protected]" <[email protected]>,
	 Ammar Faizi <[email protected]>,
	 GNU/Weeb Mailing List <[email protected]>,
	 Linux Memory Management List <[email protected]>
Subject: Re: [ammarfaizi2-block:akpm/mm/mm-unstable 46/244] lib/test_maple_tree.c:453:12: warning: result of comparison of constant 4398046511104 with expression of type 'unsigned long' is always false
Date: Tue, 1 Nov 2022 17:12:50 -0700 (PDT)	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <20221101170854.43lt42yokl34vemc@revolver>

On Tue, 1 Nov 2022, Liam Howlett wrote:
> * Andrew Morton <[email protected]> [221031 19:10]:
> > 
> > Liam, what's with that 4398046511104?  Wouldn't 0x40000000000 be clearer?  
> 
> I had the hex and changed it to decimal so I could easily search for it
> in a tree dump - it was more unique.  At one point I had it as a comment
> next to the hex, which might be a better way to keep it around for
> searching if there is ever a bug here again.  I'll include that in the
> patch.

Huh.  I made the point before, that IMO the tree dump is itself insane
to be using decimal rather than hex.  There can be patterns immediately
obvious in hex, that are thoroughly obscure in decimal (unless you're
some kind of savant).

Fortunately, it's a long time since I needed to look at a maple tree dump
(all credit to you), but here's the patch I continue to carry in my tree,
in case I ever do need to.  (But I've not checked whether more long
decimal has crept in recently.)

So long as you are the only one doing the debugging, the choice should
remain yours; but if anyone else has to look...

--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -5641,7 +5641,7 @@ void *mas_store(struct ma_state *mas, vo
 	trace_ma_write(__func__, mas, 0, entry);
 #ifdef CONFIG_DEBUG_MAPLE_TREE
 	if (mas->index > mas->last)
-		pr_err("Error %lu > %lu %p\n", mas->index, mas->last, entry);
+		pr_err("Error %lx > %lx %px\n", mas->index, mas->last, entry);
 	MT_BUG_ON(mas->tree, mas->index > mas->last);
 	if (mas->index > mas->last) {
 		mas_set_err(mas, -EINVAL);
@@ -6669,9 +6669,9 @@ static void mt_dump_range(unsigned long
 	static const char spaces[] = "                                ";
 
 	if (min == max)
-		pr_info("%.*s%lu: ", depth * 2, spaces, min);
+		pr_info("%.*s%lx: ", depth * 2, spaces, min);
 	else
-		pr_info("%.*s%lu-%lu: ", depth * 2, spaces, min, max);
+		pr_info("%.*s%lx-%lx: ", depth * 2, spaces, min, max);
 }
 
 static void mt_dump_entry(void *entry, unsigned long min, unsigned long max,
@@ -6680,14 +6680,14 @@ static void mt_dump_entry(void *entry, u
 	mt_dump_range(min, max, depth);
 
 	if (xa_is_value(entry))
-		pr_cont("value %ld (0x%lx) [%p]\n", xa_to_value(entry),
+		pr_cont("value %lx (0x%lx) [%px]\n", xa_to_value(entry),
 				xa_to_value(entry), entry);
 	else if (xa_is_zero(entry))
-		pr_cont("zero (%ld)\n", xa_to_internal(entry));
+		pr_cont("zero (%lx)\n", xa_to_internal(entry));
 	else if (mt_is_reserved(entry))
-		pr_cont("UNKNOWN ENTRY (%p)\n", entry);
+		pr_cont("UNKNOWN ENTRY (%px)\n", entry);
 	else
-		pr_cont("%p\n", entry);
+		pr_cont("%px\n", entry);
 }
 
 static void mt_dump_range64(const struct maple_tree *mt, void *entry,
@@ -6700,8 +6700,8 @@ static void mt_dump_range64(const struct
 
 	pr_cont(" contents: ");
 	for (i = 0; i < MAPLE_RANGE64_SLOTS - 1; i++)
-		pr_cont("%p %lu ", node->slot[i], node->pivot[i]);
-	pr_cont("%p\n", node->slot[i]);
+		pr_cont("%px %lx ", node->slot[i], node->pivot[i]);
+	pr_cont("%px\n", node->slot[i]);
 	for (i = 0; i < MAPLE_RANGE64_SLOTS; i++) {
 		unsigned long last = max;
 
@@ -6721,7 +6721,7 @@ static void mt_dump_range64(const struct
 		if (last == max)
 			break;
 		if (last > max) {
-			pr_err("node %p last (%lu) > max (%lu) at pivot %d!\n",
+			pr_err("node %px last (%lx) > max (%lx) at pivot %d!\n",
 					node, last, max, i);
 			break;
 		}
@@ -6739,11 +6739,11 @@ static void mt_dump_arange64(const struc
 
 	pr_cont(" contents: ");
 	for (i = 0; i < MAPLE_ARANGE64_SLOTS; i++)
-		pr_cont("%lu ", node->gap[i]);
+		pr_cont("%lx ", node->gap[i]);
 	pr_cont("| %02X %02X| ", node->meta.end, node->meta.gap);
 	for (i = 0; i < MAPLE_ARANGE64_SLOTS - 1; i++)
-		pr_cont("%p %lu ", node->slot[i], node->pivot[i]);
-	pr_cont("%p\n", node->slot[i]);
+		pr_cont("%px %lx ", node->slot[i], node->pivot[i]);
+	pr_cont("%px\n", node->slot[i]);
 	for (i = 0; i < MAPLE_ARANGE64_SLOTS; i++) {
 		unsigned long last = max;
 
@@ -6763,7 +6763,7 @@ static void mt_dump_arange64(const struc
 		if (last == max)
 			break;
 		if (last > max) {
-			pr_err("node %p last (%lu) > max (%lu) at pivot %d!\n",
+			pr_err("node %px last (%lx) > max (%lx) at pivot %d!\n",
 					node, last, max, i);
 			break;
 		}
@@ -6780,7 +6780,7 @@ static void mt_dump_node(const struct ma
 
 	mt_dump_range(min, max, depth);
 
-	pr_cont("node %p depth %d type %d parent %p", node, depth, type,
+	pr_cont("node %px depth %d type %d parent %px", node, depth, type,
 			node ? node->parent : NULL);
 	switch (type) {
 	case maple_dense:
@@ -6809,7 +6809,7 @@ void mt_dump(const struct maple_tree *mt
 {
 	void *entry = rcu_dereference_check(mt->ma_root, mt_locked(mt));
 
-	pr_info("maple_tree(%p) flags %X, height %u root %p\n",
+	pr_info("maple_tree(%px) flags %X, height %u root %px\n",
 		 mt, mt->ma_flags, mt_height(mt), entry);
 	if (!xa_is_node(entry))
 		mt_dump_entry(entry, 0, 0, 0);
@@ -6862,7 +6862,7 @@ static void mas_validate_gaps(struct ma_
 			gap = gaps[i];
 			if (!entry) {
 				if (gap != p_end - p_start + 1) {
-					pr_err("%p[%u] -> %p %lu != %lu - %lu + 1\n",
+					pr_err("%px[%u] -> %px %lx != %lx - %lx + 1\n",
 						mas_mn(mas), i,
 						mas_get_slot(mas, i), gap,
 						p_end, p_start);
@@ -6873,7 +6873,7 @@ static void mas_validate_gaps(struct ma_
 				}
 			} else {
 				if (gap > p_end - p_start + 1) {
-					pr_err("%p[%u] %lu >= %lu - %lu + 1 (%lu)\n",
+					pr_err("%px[%u] %lx >= %lx - %lx + 1 (%lx)\n",
 					mas_mn(mas), i, gap, p_end, p_start,
 					p_end - p_start + 1);
 					MT_BUG_ON(mas->tree,
@@ -6898,7 +6898,7 @@ counted:
 	p_mn = mte_parent(mte);
 	MT_BUG_ON(mas->tree, max_gap > mas->max);
 	if (ma_gaps(p_mn, mas_parent_enum(mas, mte))[p_slot] != max_gap) {
-		pr_err("gap %p[%u] != %lu\n", p_mn, p_slot, max_gap);
+		pr_err("gap %px[%u] != %lx\n", p_mn, p_slot, max_gap);
 		mt_dump(mas->tree);
 	}
 
@@ -6928,11 +6928,11 @@ static void mas_validate_parent_slot(str
 		node = mas_slot(mas, slots, i);
 		if (i == p_slot) {
 			if (node != mas->node)
-				pr_err("parent %p[%u] does not have %p\n",
+				pr_err("parent %px[%u] does not have %px\n",
 					parent, i, mas_mn(mas));
 			MT_BUG_ON(mas->tree, node != mas->node);
 		} else if (node == mas->node) {
-			pr_err("Invalid child %p at parent %p[%u] p_slot %u\n",
+			pr_err("Invalid child %px at parent %px[%u] p_slot %u\n",
 			       mas_mn(mas), parent, i, p_slot);
 			MT_BUG_ON(mas->tree, node == mas->node);
 		}
@@ -6959,14 +6959,14 @@ static void mas_validate_child_slot(stru
 			break;
 
 		if (mte_parent_slot(child) != i) {
-			pr_err("Slot error at %p[%u]: child %p has pslot %u\n",
+			pr_err("Slot error at %px[%u]: child %px has pslot %u\n",
 			       mas_mn(mas), i, mte_to_node(child),
 			       mte_parent_slot(child));
 			MT_BUG_ON(mas->tree, 1);
 		}
 
 		if (mte_parent(child) != mte_to_node(mas->node)) {
-			pr_err("child %p has parent %p not %p\n",
+			pr_err("child %px has parent %px not %px\n",
 			       mte_to_node(child), mte_parent(child),
 			       mte_to_node(mas->node));
 			MT_BUG_ON(mas->tree, 1);
@@ -7001,25 +7001,25 @@ static void mas_validate_limits(struct m
 			void *entry = mas_slot(mas, slots, i);
 
 			if (!entry)
-				pr_err("%p[%u] cannot be null\n",
+				pr_err("%px[%u] cannot be null\n",
 				       mas_mn(mas), i);
 
 			MT_BUG_ON(mas->tree, !entry);
 		}
 
 		if (prev_piv > piv) {
-			pr_err("%p[%u] piv %lu < prev_piv %lu\n",
+			pr_err("%px[%u] piv %lx < prev_piv %lx\n",
 				mas_mn(mas), i, piv, prev_piv);
 			MT_BUG_ON(mas->tree, piv < prev_piv);
 		}
 
 		if (piv < mas->min) {
-			pr_err("%p[%u] %lu < %lu\n", mas_mn(mas), i,
+			pr_err("%px[%u] %lx < %lx\n", mas_mn(mas), i,
 				piv, mas->min);
 			MT_BUG_ON(mas->tree, piv < mas->min);
 		}
 		if (piv > mas->max) {
-			pr_err("%p[%u] %lu > %lu\n", mas_mn(mas), i,
+			pr_err("%px[%u] %lx > %lx\n", mas_mn(mas), i,
 				piv, mas->max);
 			MT_BUG_ON(mas->tree, piv > mas->max);
 		}
@@ -7031,7 +7031,7 @@ static void mas_validate_limits(struct m
 		void *entry = mas_slot(mas, slots, i);
 
 		if (entry && (i != mt_slots[type] - 1)) {
-			pr_err("%p[%u] should not have entry %p\n", mas_mn(mas),
+			pr_err("%px[%u] should not have entry %px\n", mas_mn(mas),
 			       i, entry);
 			MT_BUG_ON(mas->tree, entry != NULL);
 		}
@@ -7042,7 +7042,7 @@ static void mas_validate_limits(struct m
 			if (!piv)
 				continue;
 
-			pr_err("%p[%u] should not have piv %lu\n",
+			pr_err("%px[%u] should not have piv %lx\n",
 			       mas_mn(mas), i, piv);
 			MT_BUG_ON(mas->tree, i < mt_pivots[type] - 1);
 		}
@@ -7067,7 +7067,7 @@ static void mt_validate_nulls(struct map
 	do {
 		entry = mas_slot(&mas, slots, offset);
 		if (!last && !entry) {
-			pr_err("Sequential nulls end at %p[%u]\n",
+			pr_err("Sequential nulls end at %px[%u]\n",
 				mas_mn(&mas), offset);
 		}
 		MT_BUG_ON(mt, !last && !entry);
@@ -7108,7 +7108,7 @@ void mt_validate(struct maple_tree *mt)
 			end = mas_data_end(&mas);
 			if ((end < mt_min_slot_count(mas.node)) &&
 			    (mas.max != ULONG_MAX)) {
-				pr_err("Invalid size %u of %p\n", end,
+				pr_err("Invalid size %u of %px\n", end,
 				mas_mn(&mas));
 				MT_BUG_ON(mas.tree, 1);
 			}

  reply	other threads:[~2022-11-02  0:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-30  5:04 [ammarfaizi2-block:akpm/mm/mm-unstable 46/244] lib/test_maple_tree.c:453:12: warning: result of comparison of constant 4398046511104 with expression of type 'unsigned long' is always false kernel test robot
2022-10-31 23:10 ` Andrew Morton
2022-11-01 17:24   ` Liam Howlett
2022-11-02  0:12     ` Hugh Dickins [this message]
2022-11-04 18:57       ` Liam Howlett

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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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