[PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Rasmus Villemoes]

While looking through list.h, I saw that the regular list_* helpers
(and one of the hlist_* ones) still contain the prefetch() that was
removed in linux 14 years ago. It doesn't do anything, but makes the
macros harder to read, so get rid of it, and the fallback, no-op
definition that they relied on. That requires removing a few uses
outside list.h as well.

checkpatch warns about some whitespace issues in list.h, but as I've
copied whole kerneldoc+#define blocks directly from the linux kernel,
I think it's better to just accept that so that we don't introduce
needless diffs. The "macro argument reuse" arguments should also be
ignored, as e.g. the "member" arguments are obviously always just bare
identifiers, and the "pos" arguments must be assigned to multiple
times.

Rasmus Villemoes (4):
  linux/list.h: drop use of prefetch()
  treewide: drop no-op prefetch() calls
  mips: drop unused prefetch code and logic
  linux/list.h: drop fallback definition of prefetch()

 arch/mips/include/asm/processor.h |  16 -----
 drivers/net/mvpp2.c               |   1 -
 drivers/net/octeontx/nicvf_main.c |   2 -
 drivers/usb/gadget/at91_udc.c     |   1 -
 drivers/usb/musb-new/musb_core.c  |   2 -
 include/linux/list.h              | 104 +++++++++++++++++-------------
 6 files changed, 59 insertions(+), 67 deletions(-)

-- 
2.49.0

[PATCH 1/4] linux/list.h: drop use of prefetch()

[Rasmus Villemoes]

The use of prefetch() in these list helpers was dropped back in 2011
in linux (e66eed651fd1 ("list: remove prefetching from regular list
iterators")). No arch in U-Boot defines any actual prefetch(), and as
the referenced commit says, it's usually not a win anyway.

Whole-sale sync of list.h is not really feasible, but we can
synchronize the macros containing a prefetch() with their linux
implementations as of v6.15-rc5, also importing the various helpers
needed, e.g. list_is_head() and list_next_entry().

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 include/linux/list.h | 99 ++++++++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 40 deletions(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index 0f9d939b05f..e21ae03b208 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -169,6 +169,16 @@ static inline int list_is_last(const struct list_head *list,
 	return list->next == head;
 }
 
+/**
+ * list_is_head - tests whether @list is the list @head
+ * @list: the entry to test
+ * @head: the head of the list
+ */
+static inline int list_is_head(const struct list_head *list, const struct list_head *head)
+{
+	return list == head;
+}
+
 /**
  * list_empty - tests whether a list is empty
  * @head: the list to test.
@@ -363,26 +373,28 @@ static inline void list_splice_tail_init(struct list_head *list,
 })
 
 /**
- * list_for_each	-	iterate over a list
- * @pos:	the &struct list_head to use as a loop cursor.
- * @head:	the head for your list.
+ * list_next_entry - get the next element in list
+ * @pos:	the type * to cursor
+ * @member:	the name of the list_head within the struct.
  */
-#define list_for_each(pos, head) \
-	for (pos = (head)->next; prefetch(pos->next), pos != (head); \
-		pos = pos->next)
+#define list_next_entry(pos, member) \
+	list_entry((pos)->member.next, typeof(*(pos)), member)
+
+/**
+ * list_prev_entry - get the prev element in list
+ * @pos:	the type * to cursor
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_prev_entry(pos, member) \
+	list_entry((pos)->member.prev, typeof(*(pos)), member)
 
 /**
- * __list_for_each	-	iterate over a list
+ * list_for_each	-	iterate over a list
  * @pos:	the &struct list_head to use as a loop cursor.
  * @head:	the head for your list.
- *
- * This variant differs from list_for_each() in that it's the
- * simplest possible list iteration code, no prefetching is done.
- * Use this for code that knows the list to be very short (empty
- * or 1 entry) most of the time.
  */
-#define __list_for_each(pos, head) \
-	for (pos = (head)->next; pos != (head); pos = pos->next)
+#define list_for_each(pos, head) \
+	for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
 
 /**
  * list_for_each_prev	-	iterate over a list backwards
@@ -390,8 +402,7 @@ static inline void list_splice_tail_init(struct list_head *list,
  * @head:	the head for your list.
  */
 #define list_for_each_prev(pos, head) \
-	for (pos = (head)->prev; prefetch(pos->prev), pos != (head); \
-		pos = pos->prev)
+	for (pos = (head)->prev; !list_is_head(pos, (head)); pos = pos->prev)
 
 /**
  * list_for_each_safe - iterate over a list safe against removal of list entry
@@ -411,30 +422,39 @@ static inline void list_splice_tail_init(struct list_head *list,
  */
 #define list_for_each_prev_safe(pos, n, head) \
 	for (pos = (head)->prev, n = pos->prev; \
-	     prefetch(pos->prev), pos != (head); \
+	     !list_is_head(pos, (head)); \
 	     pos = n, n = pos->prev)
 
+/**
+ * list_entry_is_head - test if the entry points to the head of the list
+ * @pos:	the type * to cursor
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_entry_is_head(pos, head, member)				\
+	list_is_head(&pos->member, (head))
+
 /**
  * list_for_each_entry	-	iterate over list of given type
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.
- * @member:	the name of the list_struct within the struct.
+ * @member:	the name of the list_head within the struct.
  */
 #define list_for_each_entry(pos, head, member)				\
-	for (pos = list_entry((head)->next, typeof(*pos), member);	\
-	     prefetch(pos->member.next), &pos->member != (head);	\
-	     pos = list_entry(pos->member.next, typeof(*pos), member))
+	for (pos = list_first_entry(head, typeof(*pos), member);	\
+	     !list_entry_is_head(pos, head, member);			\
+	     pos = list_next_entry(pos, member))
 
 /**
  * list_for_each_entry_reverse - iterate backwards over list of given type.
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.
- * @member:	the name of the list_struct within the struct.
+ * @member:	the name of the list_head within the struct.
  */
 #define list_for_each_entry_reverse(pos, head, member)			\
-	for (pos = list_entry((head)->prev, typeof(*pos), member);	\
-	     prefetch(pos->member.prev), &pos->member != (head);	\
-	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+	for (pos = list_last_entry(head, typeof(*pos), member);		\
+	     !list_entry_is_head(pos, head, member); 			\
+	     pos = list_prev_entry(pos, member))
 
 /**
  * list_prepare_entry - prepare a pos entry for use in list_for_each_entry_continue()
@@ -451,41 +471,41 @@ static inline void list_splice_tail_init(struct list_head *list,
  * list_for_each_entry_continue - continue iteration over list of given type
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.
- * @member:	the name of the list_struct within the struct.
+ * @member:	the name of the list_head within the struct.
  *
  * Continue to iterate over list of given type, continuing after
  * the current position.
  */
-#define list_for_each_entry_continue(pos, head, member)			\
-	for (pos = list_entry(pos->member.next, typeof(*pos), member);	\
-	     prefetch(pos->member.next), &pos->member != (head);	\
-	     pos = list_entry(pos->member.next, typeof(*pos), member))
+#define list_for_each_entry_continue(pos, head, member) 		\
+	for (pos = list_next_entry(pos, member);			\
+	     !list_entry_is_head(pos, head, member);			\
+	     pos = list_next_entry(pos, member))
 
 /**
  * list_for_each_entry_continue_reverse - iterate backwards from the given point
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.
- * @member:	the name of the list_struct within the struct.
+ * @member:	the name of the list_head within the struct.
  *
  * Start to iterate over list of given type backwards, continuing after
  * the current position.
  */
 #define list_for_each_entry_continue_reverse(pos, head, member)		\
-	for (pos = list_entry(pos->member.prev, typeof(*pos), member);	\
-	     prefetch(pos->member.prev), &pos->member != (head);	\
-	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+	for (pos = list_prev_entry(pos, member);			\
+	     !list_entry_is_head(pos, head, member);			\
+	     pos = list_prev_entry(pos, member))
 
 /**
  * list_for_each_entry_from - iterate over list of given type from the current point
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.
- * @member:	the name of the list_struct within the struct.
+ * @member:	the name of the list_head within the struct.
  *
  * Iterate over list of given type, continuing from current position.
  */
-#define list_for_each_entry_from(pos, head, member)			\
-	for (; prefetch(pos->member.next), &pos->member != (head);	\
-	     pos = list_entry(pos->member.next, typeof(*pos), member))
+#define list_for_each_entry_from(pos, head, member) 			\
+	for (; !list_entry_is_head(pos, head, member);			\
+	     pos = list_next_entry(pos, member))
 
 /**
  * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry
@@ -654,8 +674,7 @@ static inline void hlist_add_after(struct hlist_node *n,
 #define hlist_entry(ptr, type, member) container_of(ptr,type,member)
 
 #define hlist_for_each(pos, head) \
-	for (pos = (head)->first; pos && ({ prefetch(pos->next); 1; }); \
-	     pos = pos->next)
+	for (pos = (head)->first; pos ; pos = pos->next)
 
 #define hlist_for_each_safe(pos, n, head) \
 	for (pos = (head)->first; pos && ({ n = pos->next; 1; }); \
-- 
2.49.0

[PATCH 2/4] treewide: drop no-op prefetch() calls

[Rasmus Villemoes]

These all end up using the no-op prefetch() defined in linux/list.h,
because the only possible real implementation is in
arch/mips/include/asm/processor.h, which is behing
CONFIG_CPU_HAS_PREFETCH which is nowhere defined.

In order to be able to drop that fallback definition from list.h,
first remove all uses.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 drivers/net/mvpp2.c               | 1 -
 drivers/net/octeontx/nicvf_main.c | 2 --
 drivers/usb/gadget/at91_udc.c     | 1 -
 drivers/usb/musb-new/musb_core.c  | 2 --
 4 files changed, 6 deletions(-)

diff --git a/drivers/net/mvpp2.c b/drivers/net/mvpp2.c
index ae545fe229c..184c1f9a46a 100644
--- a/drivers/net/mvpp2.c
+++ b/drivers/net/mvpp2.c
@@ -3876,7 +3876,6 @@ mvpp2_rxq_next_desc_get(struct mvpp2_rx_queue *rxq)
 	int rx_desc = rxq->next_desc_to_proc;
 
 	rxq->next_desc_to_proc = MVPP2_QUEUE_NEXT_DESC(rxq, rx_desc);
-	prefetch(rxq->descs + rxq->next_desc_to_proc);
 	return rxq->descs + rx_desc;
 }
 
diff --git a/drivers/net/octeontx/nicvf_main.c b/drivers/net/octeontx/nicvf_main.c
index 6e4d0a05121..27d0327c88a 100644
--- a/drivers/net/octeontx/nicvf_main.c
+++ b/drivers/net/octeontx/nicvf_main.c
@@ -279,8 +279,6 @@ int nicvf_cq_handler(struct nicvf *nic, void **ppkt, int *pkt_len)
 		cq_desc = (struct cqe_rx_t *)GET_CQ_DESC(cq, cqe_head);
 		cqe_head++;
 		cqe_head &= (cq->dmem.q_len - 1);
-		/* Initiate prefetch for next descriptor */
-		prefetch((struct cqe_rx_t *)GET_CQ_DESC(cq, cqe_head));
 
 		switch (cq_desc->cqe_type) {
 		case CQE_TYPE_RX:
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 86b2cbf3f6a..b3c780a4e35 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -228,7 +228,6 @@ static int write_fifo(struct at91_ep *ep, struct at91_request *req)
 	}
 
 	buf = req->req.buf + req->req.actual;
-	prefetch(buf);
 	total = req->req.length - req->req.actual;
 	if (ep->ep.maxpacket < total) {
 		count = ep->ep.maxpacket;
diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index 6375be741ae..a14b127dc37 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -223,8 +223,6 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src)
 	struct musb *musb = hw_ep->musb;
 	void __iomem *fifo = hw_ep->fifo;
 
-	prefetch((u8 *)src);
-
 	dev_dbg(musb->controller, "%cX ep%d fifo %p count %d buf %p\n",
 			'T', hw_ep->epnum, fifo, len, src);
 
-- 
2.49.0

[PATCH 3/4] mips: drop unused prefetch code and logic

[Rasmus Villemoes]

AFAICT, CONFIG_CPU_HAS_PREFETCH has never existed as a proper Kconfig
symbol in U-Boot, nor has any board include file ever defined it.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 arch/mips/include/asm/processor.h | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h
index 39a4f435a84..4c6322ed1c7 100644
--- a/arch/mips/include/asm/processor.h
+++ b/arch/mips/include/asm/processor.h
@@ -118,20 +118,4 @@ struct task_struct;
  */
 #define return_address() ({__asm__ __volatile__("":::"$31");__builtin_return_address(0);})
 
-#ifdef CONFIG_CPU_HAS_PREFETCH
-
-#define ARCH_HAS_PREFETCH
-
-static inline void prefetch(const void *addr)
-{
-	__asm__ __volatile__(
-	"	.set	mips4		\n"
-	"	pref	%0, (%1)	\n"
-	"	.set	mips0		\n"
-	:
-	: "i" (Pref_Load), "r" (addr));
-}
-
-#endif
-
 #endif /* _ASM_PROCESSOR_H */
-- 
2.49.0

[PATCH 4/4] linux/list.h: drop fallback definition of prefetch()

[Rasmus Villemoes]

None of the list helpers use prefetch() anymore, and no C code relies
on getting this definition from list.h. In any case, such an arch/cpu
specific thing does not belong in a header that just consists of cpp
helper macros.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 include/linux/list.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index e21ae03b208..3dc38279716 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -4,11 +4,6 @@
 #include <linux/stddef.h>
 #include <linux/poison.h>
 
-#ifndef ARCH_HAS_PREFETCH
-#define ARCH_HAS_PREFETCH
-static inline void prefetch(const void *x) {;}
-#endif
-
 /*
  * Simple doubly linked list implementation.
  *
-- 
2.49.0

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

On Wed, May 07, 2025 at 02:12:42PM +0200, Rasmus Villemoes wrote:

> While looking through list.h, I saw that the regular list_* helpers
> (and one of the hlist_* ones) still contain the prefetch() that was
> removed in linux 14 years ago. It doesn't do anything, but makes the
> macros harder to read, so get rid of it, and the fallback, no-op
> definition that they relied on. That requires removing a few uses
> outside list.h as well.
> 
> checkpatch warns about some whitespace issues in list.h, but as I've
> copied whole kerneldoc+#define blocks directly from the linux kernel,
> I think it's better to just accept that so that we don't introduce
> needless diffs. The "macro argument reuse" arguments should also be
> ignored, as e.g. the "member" arguments are obviously always just bare
> identifiers, and the "pos" arguments must be assigned to multiple
> times.
> 
> Rasmus Villemoes (4):
>   linux/list.h: drop use of prefetch()
>   treewide: drop no-op prefetch() calls
>   mips: drop unused prefetch code and logic
>   linux/list.h: drop fallback definition of prefetch()
> 
>  arch/mips/include/asm/processor.h |  16 -----
>  drivers/net/mvpp2.c               |   1 -
>  drivers/net/octeontx/nicvf_main.c |   2 -
>  drivers/usb/gadget/at91_udc.c     |   1 -
>  drivers/usb/musb-new/musb_core.c  |   2 -
>  include/linux/list.h              | 104 +++++++++++++++++-------------
>  6 files changed, 59 insertions(+), 67 deletions(-)

A good cleanup, thanks for doing it.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1610 bytes --]

On Wed, May 07, 2025 at 02:12:42PM +0200, Rasmus Villemoes wrote:

> While looking through list.h, I saw that the regular list_* helpers
> (and one of the hlist_* ones) still contain the prefetch() that was
> removed in linux 14 years ago. It doesn't do anything, but makes the
> macros harder to read, so get rid of it, and the fallback, no-op
> definition that they relied on. That requires removing a few uses
> outside list.h as well.
> 
> checkpatch warns about some whitespace issues in list.h, but as I've
> copied whole kerneldoc+#define blocks directly from the linux kernel,
> I think it's better to just accept that so that we don't introduce
> needless diffs. The "macro argument reuse" arguments should also be
> ignored, as e.g. the "member" arguments are obviously always just bare
> identifiers, and the "pos" arguments must be assigned to multiple
> times.
> 
> Rasmus Villemoes (4):
>   linux/list.h: drop use of prefetch()
>   treewide: drop no-op prefetch() calls
>   mips: drop unused prefetch code and logic
>   linux/list.h: drop fallback definition of prefetch()
> 
>  arch/mips/include/asm/processor.h |  16 -----
>  drivers/net/mvpp2.c               |   1 -
>  drivers/net/octeontx/nicvf_main.c |   2 -
>  drivers/usb/gadget/at91_udc.c     |   1 -
>  drivers/usb/musb-new/musb_core.c  |   2 -
>  include/linux/list.h              | 104 +++++++++++++++++-------------
>  6 files changed, 59 insertions(+), 67 deletions(-)

Unfortunately this leads to build problems on lots of platforms:
https://source.denx.de/u-boot/u-boot/-/jobs/1141951

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Rasmus Villemoes]

On Thu, May 22 2025, Tom Rini <trini@konsulko.com> wrote:

> On Wed, May 07, 2025 at 02:12:42PM +0200, Rasmus Villemoes wrote:
>
>> While looking through list.h, I saw that the regular list_* helpers
>> (and one of the hlist_* ones) still contain the prefetch() that was
>> removed in linux 14 years ago. It doesn't do anything, but makes the
>> macros harder to read, so get rid of it, and the fallback, no-op
>> definition that they relied on. That requires removing a few uses
>> outside list.h as well.
>> 
>> checkpatch warns about some whitespace issues in list.h, but as I've
>> copied whole kerneldoc+#define blocks directly from the linux kernel,
>> I think it's better to just accept that so that we don't introduce
>> needless diffs. The "macro argument reuse" arguments should also be
>> ignored, as e.g. the "member" arguments are obviously always just bare
>> identifiers, and the "pos" arguments must be assigned to multiple
>> times.
>> 
>> Rasmus Villemoes (4):
>>   linux/list.h: drop use of prefetch()
>>   treewide: drop no-op prefetch() calls
>>   mips: drop unused prefetch code and logic
>>   linux/list.h: drop fallback definition of prefetch()
>> 
>>  arch/mips/include/asm/processor.h |  16 -----
>>  drivers/net/mvpp2.c               |   1 -
>>  drivers/net/octeontx/nicvf_main.c |   2 -
>>  drivers/usb/gadget/at91_udc.c     |   1 -
>>  drivers/usb/musb-new/musb_core.c  |   2 -
>>  include/linux/list.h              | 104 +++++++++++++++++-------------
>>  6 files changed, 59 insertions(+), 67 deletions(-)
>
> Unfortunately this leads to build problems on lots of platforms:
> https://source.denx.de/u-boot/u-boot/-/jobs/1141951

:( so we've been relying on that prefetch() laundering away the
volatile.

Which really begs the question: Why, exactly, is it that gd even has
that volatile qualifier in the first place? 

I'm 98% certain that we could drop that and get better code generation
and avoid a ton of places where we cast away that volatile which
shouldn't really be there anyway.

Rasmus

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2380 bytes --]

On Sun, May 25, 2025 at 10:07:56PM +0200, Rasmus Villemoes wrote:
> On Thu, May 22 2025, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Wed, May 07, 2025 at 02:12:42PM +0200, Rasmus Villemoes wrote:
> >
> >> While looking through list.h, I saw that the regular list_* helpers
> >> (and one of the hlist_* ones) still contain the prefetch() that was
> >> removed in linux 14 years ago. It doesn't do anything, but makes the
> >> macros harder to read, so get rid of it, and the fallback, no-op
> >> definition that they relied on. That requires removing a few uses
> >> outside list.h as well.
> >> 
> >> checkpatch warns about some whitespace issues in list.h, but as I've
> >> copied whole kerneldoc+#define blocks directly from the linux kernel,
> >> I think it's better to just accept that so that we don't introduce
> >> needless diffs. The "macro argument reuse" arguments should also be
> >> ignored, as e.g. the "member" arguments are obviously always just bare
> >> identifiers, and the "pos" arguments must be assigned to multiple
> >> times.
> >> 
> >> Rasmus Villemoes (4):
> >>   linux/list.h: drop use of prefetch()
> >>   treewide: drop no-op prefetch() calls
> >>   mips: drop unused prefetch code and logic
> >>   linux/list.h: drop fallback definition of prefetch()
> >> 
> >>  arch/mips/include/asm/processor.h |  16 -----
> >>  drivers/net/mvpp2.c               |   1 -
> >>  drivers/net/octeontx/nicvf_main.c |   2 -
> >>  drivers/usb/gadget/at91_udc.c     |   1 -
> >>  drivers/usb/musb-new/musb_core.c  |   2 -
> >>  include/linux/list.h              | 104 +++++++++++++++++-------------
> >>  6 files changed, 59 insertions(+), 67 deletions(-)
> >
> > Unfortunately this leads to build problems on lots of platforms:
> > https://source.denx.de/u-boot/u-boot/-/jobs/1141951
> 
> :( so we've been relying on that prefetch() laundering away the
> volatile.
> 
> Which really begs the question: Why, exactly, is it that gd even has
> that volatile qualifier in the first place? 

The answer is likely early 2000s GCC.

> I'm 98% certain that we could drop that and get better code generation
> and avoid a ton of places where we cast away that volatile which
> shouldn't really be there anyway.

It would be a good thing to experiment with now and maybe try for real
in a near-future merge window.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Rasmus Villemoes]

On Sun, May 25 2025, Tom Rini <trini@konsulko.com> wrote:

> On Sun, May 25, 2025 at 10:07:56PM +0200, Rasmus Villemoes wrote:
>> 
>> :( so we've been relying on that prefetch() laundering away the
>> volatile.
>> 
>> Which really begs the question: Why, exactly, is it that gd even has
>> that volatile qualifier in the first place? 
>
> The answer is likely early 2000s GCC.
>
>> I'm 98% certain that we could drop that and get better code generation
>> and avoid a ton of places where we cast away that volatile which
>> shouldn't really be there anyway.
>
> It would be a good thing to experiment with now and maybe try for real
> in a near-future merge window.

So since it's declared per architecture I just tried with arm for now,
and this

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index 45401d5e3c8..f7a47204b7c 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -133,9 +133,9 @@ static inline gd_t *get_gd(void)
 #else
 
 #ifdef CONFIG_ARM64
-#define DECLARE_GLOBAL_DATA_PTR                register volatile gd_t *gd asm ("x18")
+#define DECLARE_GLOBAL_DATA_PTR                register gd_t *gd asm ("x18")
 #else
-#define DECLARE_GLOBAL_DATA_PTR                register volatile gd_t *gd asm ("r9")
+#define DECLARE_GLOBAL_DATA_PTR                register gd_t *gd asm ("r9")
 #endif
 #endif
 

builds, boots to linux, and passes our test suite on our
beagleboneblack, imx7 SabreSD, RPi 3, RPi 4, RPi-cm4, wandboard,
imx8mp-evk; i.e. a good mix of 32 and 64 bit arm.

Rasmus

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1908 bytes --]

On Mon, May 26, 2025 at 11:10:33PM +0200, Rasmus Villemoes wrote:
> On Sun, May 25 2025, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Sun, May 25, 2025 at 10:07:56PM +0200, Rasmus Villemoes wrote:
> >> 
> >> :( so we've been relying on that prefetch() laundering away the
> >> volatile.
> >> 
> >> Which really begs the question: Why, exactly, is it that gd even has
> >> that volatile qualifier in the first place? 
> >
> > The answer is likely early 2000s GCC.
> >
> >> I'm 98% certain that we could drop that and get better code generation
> >> and avoid a ton of places where we cast away that volatile which
> >> shouldn't really be there anyway.
> >
> > It would be a good thing to experiment with now and maybe try for real
> > in a near-future merge window.
> 
> So since it's declared per architecture I just tried with arm for now,
> and this
> 
> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
> index 45401d5e3c8..f7a47204b7c 100644
> --- a/arch/arm/include/asm/global_data.h
> +++ b/arch/arm/include/asm/global_data.h
> @@ -133,9 +133,9 @@ static inline gd_t *get_gd(void)
>  #else
>  
>  #ifdef CONFIG_ARM64
> -#define DECLARE_GLOBAL_DATA_PTR                register volatile gd_t *gd asm ("x18")
> +#define DECLARE_GLOBAL_DATA_PTR                register gd_t *gd asm ("x18")
>  #else
> -#define DECLARE_GLOBAL_DATA_PTR                register volatile gd_t *gd asm ("r9")
> +#define DECLARE_GLOBAL_DATA_PTR                register gd_t *gd asm ("r9")
>  #endif
>  #endif
>  
> 
> builds, boots to linux, and passes our test suite on our
> beagleboneblack, imx7 SabreSD, RPi 3, RPi 4, RPi-cm4, wandboard,
> imx8mp-evk; i.e. a good mix of 32 and 64 bit arm.

That's a good set of datapoints, thanks. Do you want to post a real
series at some point or should I do it and Suggested-by you? Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Rasmus Villemoes]

On Mon, May 26 2025, Tom Rini <trini@konsulko.com> wrote:

> On Mon, May 26, 2025 at 11:10:33PM +0200, Rasmus Villemoes wrote:
>> On Sun, May 25 2025, Tom Rini <trini@konsulko.com> wrote:
>> 
>> > On Sun, May 25, 2025 at 10:07:56PM +0200, Rasmus Villemoes wrote:
>> >> 
>> >> :( so we've been relying on that prefetch() laundering away the
>> >> volatile.
>> >> 
>> >> Which really begs the question: Why, exactly, is it that gd even has
>> >> that volatile qualifier in the first place? 
>> >
>> > The answer is likely early 2000s GCC.
>> >
>> >> I'm 98% certain that we could drop that and get better code generation
>> >> and avoid a ton of places where we cast away that volatile which
>> >> shouldn't really be there anyway.
>> >
>> > It would be a good thing to experiment with now and maybe try for real
>> > in a near-future merge window.
>> 
>> So since it's declared per architecture I just tried with arm for now,
>> and this
>> 
>> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
>> index 45401d5e3c8..f7a47204b7c 100644
>> --- a/arch/arm/include/asm/global_data.h
>> +++ b/arch/arm/include/asm/global_data.h
>> @@ -133,9 +133,9 @@ static inline gd_t *get_gd(void)
>>  #else
>>  
>>  #ifdef CONFIG_ARM64
>> -#define DECLARE_GLOBAL_DATA_PTR                register volatile gd_t *gd asm ("x18")
>> +#define DECLARE_GLOBAL_DATA_PTR                register gd_t *gd asm ("x18")
>>  #else
>> -#define DECLARE_GLOBAL_DATA_PTR                register volatile gd_t *gd asm ("r9")
>> +#define DECLARE_GLOBAL_DATA_PTR                register gd_t *gd asm ("r9")
>>  #endif
>>  #endif
>>  
>> 
>> builds, boots to linux, and passes our test suite on our
>> beagleboneblack, imx7 SabreSD, RPi 3, RPi 4, RPi-cm4, wandboard,
>> imx8mp-evk; i.e. a good mix of 32 and 64 bit arm.
>
> That's a good set of datapoints, thanks. Do you want to post a real
> series at some point or should I do it and Suggested-by you? Thanks.

I'll do a series and let the azure CI chew on it before posting it for
real.

Rasmus

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Rasmus Villemoes]

On Thu, May 22 2025, Tom Rini <trini@konsulko.com> wrote:

> On Wed, May 07, 2025 at 02:12:42PM +0200, Rasmus Villemoes wrote:
>
>> While looking through list.h, I saw that the regular list_* helpers
>> (and one of the hlist_* ones) still contain the prefetch() that was
>> removed in linux 14 years ago. It doesn't do anything, but makes the
>> macros harder to read, so get rid of it, and the fallback, no-op
>> definition that they relied on. That requires removing a few uses
>> outside list.h as well.
>> 
>
> Unfortunately this leads to build problems on lots of platforms:
> https://source.denx.de/u-boot/u-boot/-/jobs/1141951

Hi Tom

Now that the 'remove volatile from gd' series is in -next, could you see
how these cleanups would fare on top of that?

Thanks,
Rasmus

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Wed, Jun 18, 2025 at 08:23:02AM +0200, Rasmus Villemoes wrote:
> On Thu, May 22 2025, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Wed, May 07, 2025 at 02:12:42PM +0200, Rasmus Villemoes wrote:
> >
> >> While looking through list.h, I saw that the regular list_* helpers
> >> (and one of the hlist_* ones) still contain the prefetch() that was
> >> removed in linux 14 years ago. It doesn't do anything, but makes the
> >> macros harder to read, so get rid of it, and the fallback, no-op
> >> definition that they relied on. That requires removing a few uses
> >> outside list.h as well.
> >> 
> >
> > Unfortunately this leads to build problems on lots of platforms:
> > https://source.denx.de/u-boot/u-boot/-/jobs/1141951
> 
> Hi Tom
> 
> Now that the 'remove volatile from gd' series is in -next, could you see
> how these cleanups would fare on top of that?

Yes, I'll give it another spin right now.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/4] list.h/treewide: get rid of no-op prefetch()

[Tom Rini]

On Wed, 07 May 2025 14:12:42 +0200, Rasmus Villemoes wrote:

> While looking through list.h, I saw that the regular list_* helpers
> (and one of the hlist_* ones) still contain the prefetch() that was
> removed in linux 14 years ago. It doesn't do anything, but makes the
> macros harder to read, so get rid of it, and the fallback, no-op
> definition that they relied on. That requires removing a few uses
> outside list.h as well.
> 
> [...]

Applied to u-boot/next, thanks!

[1/4] linux/list.h: drop use of prefetch()
      commit: c1c2b5c6a4c09fe57933b70ec6289795f732ac83
[2/4] treewide: drop no-op prefetch() calls
      commit: f5fa73a625f671152aa65be4b039060f8baed4cc
[3/4] mips: drop unused prefetch code and logic
      commit: f0ba1435a8eee45138353f6114ae658e3802dc9d
[4/4] linux/list.h: drop fallback definition of prefetch()
      commit: 1d19bbcb68280814952bdc18d7dec6d083b2c0ef
-- 
Tom