diff mbox series

[v5,05/22] mbuf: stop using mbuf cacheline marker fields

Message ID 1708762927-14126-6-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series stop using RTE_MARKER extensions |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Tyler Retzlaff Feb. 24, 2024, 8:21 a.m. UTC 
  Update prefetch inline functions to access rte_mbuf struct fields
directly instead of via cacheline{0,1} marker extension fields.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/mbuf/rte_mbuf.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon Feb. 24, 2024, 10:58 a.m. UTC | #1 
24/02/2024 09:21, Tyler Retzlaff:
> Update prefetch inline functions to access rte_mbuf struct fields
> directly instead of via cacheline{0,1} marker extension fields.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
[...]
>  rte_mbuf_prefetch_part1(struct rte_mbuf *m)
>  {
> -	rte_prefetch0(&m->cacheline0);
> +	rte_prefetch0(&m->buf_addr);

Should be simply "m", no need to point to the first field explicitly.

[...]
>  rte_mbuf_prefetch_part2(struct rte_mbuf *m)
>  {
>  #if RTE_CACHE_LINE_SIZE == 64
> -	rte_prefetch0(&m->cacheline1);
> +#if RTE_IOVA_IN_MBUF
> +	rte_prefetch0(&m->next);
> +#else
> +	rte_prefetch0(&m->dynfield2);
> +#endif

I think it is better to calculate m + min cache line size
instead of relying on fields.
Stephen Hemminger Feb. 26, 2024, 6:20 p.m. UTC | #2 
On Sat, 24 Feb 2024 11:58:59 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 24/02/2024 09:21, Tyler Retzlaff:
> > Update prefetch inline functions to access rte_mbuf struct fields
> > directly instead of via cacheline{0,1} marker extension fields.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>  
> [...]
> >  rte_mbuf_prefetch_part1(struct rte_mbuf *m)
> >  {
> > -	rte_prefetch0(&m->cacheline0);
> > +	rte_prefetch0(&m->buf_addr);  
> 
> Should be simply "m", no need to point to the first field explicitly.
> 
> [...]
> >  rte_mbuf_prefetch_part2(struct rte_mbuf *m)
> >  {
> >  #if RTE_CACHE_LINE_SIZE == 64
> > -	rte_prefetch0(&m->cacheline1);
> > +#if RTE_IOVA_IN_MBUF
> > +	rte_prefetch0(&m->next);
> > +#else
> > +	rte_prefetch0(&m->dynfield2);
> > +#endif  
> 
> I think it is better to calculate m + min cache line size
> instead of relying on fields.
> 
> 

Agree with Thomas, the markers and field dependency are bad idea.
diff mbox series

Patch

diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 286b32b..04cde0f 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -108,7 +108,7 @@ 
 static inline void
 rte_mbuf_prefetch_part1(struct rte_mbuf *m)
 {
-	rte_prefetch0(&m->cacheline0);
+	rte_prefetch0(&m->buf_addr);
 }
 
 /**
@@ -126,7 +126,11 @@ 
 rte_mbuf_prefetch_part2(struct rte_mbuf *m)
 {
 #if RTE_CACHE_LINE_SIZE == 64
-	rte_prefetch0(&m->cacheline1);
+#if RTE_IOVA_IN_MBUF
+	rte_prefetch0(&m->next);
+#else
+	rte_prefetch0(&m->dynfield2);
+#endif
 #else
 	RTE_SET_USED(m);
 #endif