Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1230468 > unrolled thread

[PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions inline

Started byChristophe Leroy <christophe.leroy@c-s.fr>
First post2015-09-22 19:00 +0200
Last post2015-09-29 02:30 +0200
Articles 11 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Christophe Leroy <christophe.leroy@c-s.fr> - 2015-09-22 19:00 +0200
    Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Joakim Tjernlund <joakim.tjernlund@transmode.se> - 2015-09-22 20:20 +0200
      Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Scott Wood <scottwood@freescale.com> - 2015-09-22 21:00 +0200
        Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Joakim Tjernlund <joakim.tjernlund@transmode.se> - 2015-09-22 21:40 +0200
          Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Joakim Tjernlund <joakim.tjernlund@transmode.se> - 2015-09-22 22:00 +0200
            Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Joakim Tjernlund <joakim.tjernlund@transmode.se> - 2015-09-22 22:10 +0200
            Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Joakim Tjernlund <joakim.tjernlund@transmode.se> - 2015-09-22 22:40 +0200
              Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Christophe Leroy <christophe.leroy@c-s.fr> - 2015-09-22 23:00 +0200
                Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Christophe Leroy <christophe.leroy@c-s.fr> - 2015-09-23 00:50 +0200
            Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Joakim Tjernlund <joakim.tjernlund@transmode.se> - 2015-09-22 22:40 +0200
    Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions  inline Scott Wood <scottwood@freescale.com> - 2015-09-29 02:30 +0200

#1230468 — [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions inline

FromChristophe Leroy <christophe.leroy@c-s.fr>
Date2015-09-22 19:00 +0200
Subject[PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions inline
Message-ID<qbz58-2KT-17@gated-at.bofh.it>
flush/clean/invalidate _dcache_range() functions are all very
similar and are quite short. They are mainly used in __dma_sync()
perf_event locate them in the top 3 consumming functions during
heavy ethernet activity

They are good candidate for inlining, as __dma_sync() does
almost nothing but calling them

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
New in v2

 arch/powerpc/include/asm/cacheflush.h | 55 +++++++++++++++++++++++++++--
 arch/powerpc/kernel/misc_32.S         | 65 -----------------------------------
 arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
 3 files changed, 54 insertions(+), 68 deletions(-)

diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index 6229e6b..6169604 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -47,12 +47,61 @@ static inline void __flush_dcache_icache_phys(unsigned long physaddr)
 }
 #endif
 
-extern void flush_dcache_range(unsigned long start, unsigned long stop);
 #ifdef CONFIG_PPC32
-extern void clean_dcache_range(unsigned long start, unsigned long stop);
-extern void invalidate_dcache_range(unsigned long start, unsigned long stop);
+/*
+ * Write any modified data cache blocks out to memory and invalidate them.
+ * Does not invalidate the corresponding instruction cache blocks.
+ */
+static inline void flush_dcache_range(unsigned long start, unsigned long stop)
+{
+	void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
+	unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
+	unsigned int i;
+
+	for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
+		dcbf(addr);
+	if (i)
+		mb();	/* sync */
+}
+
+/*
+ * Write any modified data cache blocks out to memory.
+ * Does not invalidate the corresponding cache lines (especially for
+ * any corresponding instruction cache).
+ */
+static inline void clean_dcache_range(unsigned long start, unsigned long stop)
+{
+	void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
+	unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
+	unsigned int i;
+
+	for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
+		dcbst(addr);
+	if (i)
+		mb();	/* sync */
+}
+
+/*
+ * Like above, but invalidate the D-cache.  This is used by the 8xx
+ * to invalidate the cache so the PPC core doesn't get stale data
+ * from the CPM (no cache snooping here :-).
+ */
+static inline void invalidate_dcache_range(unsigned long start,
+					   unsigned long stop)
+{
+	void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
+	unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
+	unsigned int i;
+
+	for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
+		dcbi(addr);
+	if (i)
+		mb();	/* sync */
+}
+
 #endif /* CONFIG_PPC32 */
 #ifdef CONFIG_PPC64
+extern void flush_dcache_range(unsigned long start, unsigned long stop);
 extern void flush_inval_dcache_range(unsigned long start, unsigned long stop);
 extern void flush_dcache_phys_range(unsigned long start, unsigned long stop);
 #endif
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index ce3ca08..1728f61 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -375,71 +375,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
 	isync
 	blr
 /*
- * Write any modified data cache blocks out to memory.
- * Does not invalidate the corresponding cache lines (especially for
- * any corresponding instruction cache).
- *
- * clean_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(clean_dcache_range)
-	li	r5,L1_CACHE_BYTES-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-
-1:	dcbst	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbst's to get to ram */
-	blr
-
-/*
- * Write any modified data cache blocks out to memory and invalidate them.
- * Does not invalidate the corresponding instruction cache blocks.
- *
- * flush_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(flush_dcache_range)
-	li	r5,L1_CACHE_BYTES-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-
-1:	dcbf	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbst's to get to ram */
-	blr
-
-/*
- * Like above, but invalidate the D-cache.  This is used by the 8xx
- * to invalidate the cache so the PPC core doesn't get stale data
- * from the CPM (no cache snooping here :-).
- *
- * invalidate_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(invalidate_dcache_range)
-	li	r5,L1_CACHE_BYTES-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-
-1:	dcbi	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbi's to get to ram */
-	blr
-
-/*
  * Flush a particular page from the data cache to RAM.
  * Note: this is necessary because the instruction cache does *not*
  * snoop from the data cache.
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index 202963e..0546947 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -6,7 +6,9 @@
 #include <asm/cacheflush.h>
 #include <asm/epapr_hcalls.h>
 
+#ifdef CONFIG_PPC64
 EXPORT_SYMBOL(flush_dcache_range);
+#endif
 EXPORT_SYMBOL(flush_icache_range);
 
 EXPORT_SYMBOL(empty_zero_page);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1230655

FromJoakim Tjernlund <joakim.tjernlund@transmode.se>
Date2015-09-22 20:20 +0200
Message-ID<qbAkA-4J7-73@gated-at.bofh.it>
In reply to#1230468
On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
> flush/clean/invalidate _dcache_range() functions are all very
> similar and are quite short. They are mainly used in __dma_sync()
> perf_event locate them in the top 3 consumming functions during
> heavy ethernet activity
> 
> They are good candidate for inlining, as __dma_sync() does
> almost nothing but calling them
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> New in v2
> 
>  arch/powerpc/include/asm/cacheflush.h | 55 +++++++++++++++++++++++++++--
>  arch/powerpc/kernel/misc_32.S         | 65 -----------------------------------
>  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
>  3 files changed, 54 insertions(+), 68 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> index 6229e6b..6169604 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -47,12 +47,61 @@ static inline void __flush_dcache_icache_phys(unsigned long physaddr)
>  }
>  #endif
>  
> -extern void flush_dcache_range(unsigned long start, unsigned long stop);
>  #ifdef CONFIG_PPC32
> -extern void clean_dcache_range(unsigned long start, unsigned long stop);
> -extern void invalidate_dcache_range(unsigned long start, unsigned long stop);
> +/*
> + * Write any modified data cache blocks out to memory and invalidate them.
> + * Does not invalidate the corresponding instruction cache blocks.
> + */
> +static inline void flush_dcache_range(unsigned long start, unsigned long stop)
> +{
> +	void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> +	unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> +	unsigned int i;
> +
> +	for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> +		dcbf(addr);
> +	if (i)
> +		mb();	/* sync */
> +}

This feels optimized for the uncommon case when there is no invalidation.
I THINK it would be better to bail early and use do { .. } while(--i); instead.

 Jocke--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230774

FromScott Wood <scottwood@freescale.com>
Date2015-09-22 21:00 +0200
Message-ID<qbAXh-5tC-41@gated-at.bofh.it>
In reply to#1230655
On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote:
> On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
> > flush/clean/invalidate _dcache_range() functions are all very
> > similar and are quite short. They are mainly used in __dma_sync()
> > perf_event locate them in the top 3 consumming functions during
> > heavy ethernet activity
> > 
> > They are good candidate for inlining, as __dma_sync() does
> > almost nothing but calling them
> > 
> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > ---
> > New in v2
> > 
> >  arch/powerpc/include/asm/cacheflush.h | 55 +++++++++++++++++++++++++++--
> >  arch/powerpc/kernel/misc_32.S         | 65 ------------------------------
> > -----
> >  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
> >  3 files changed, 54 insertions(+), 68 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/cacheflush.h 
> > b/arch/powerpc/include/asm/cacheflush.h
> > index 6229e6b..6169604 100644
> > --- a/arch/powerpc/include/asm/cacheflush.h
> > +++ b/arch/powerpc/include/asm/cacheflush.h
> > @@ -47,12 +47,61 @@ static inline void 
> > __flush_dcache_icache_phys(unsigned long physaddr)
> >  }
> >  #endif
> >  
> > -extern void flush_dcache_range(unsigned long start, unsigned long stop);
> >  #ifdef CONFIG_PPC32
> > -extern void clean_dcache_range(unsigned long start, unsigned long stop);
> > -extern void invalidate_dcache_range(unsigned long start, unsigned long 
> > stop);
> > +/*
> > + * Write any modified data cache blocks out to memory and invalidate 
> > them.
> > + * Does not invalidate the corresponding instruction cache blocks.
> > + */
> > +static inline void flush_dcache_range(unsigned long start, unsigned long 
> > stop)
> > +{
> > +   void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> > +   unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> > +   unsigned int i;
> > +
> > +   for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> > +           dcbf(addr);
> > +   if (i)
> > +           mb();   /* sync */
> > +}
> 
> This feels optimized for the uncommon case when there is no invalidation.

If you mean the "if (i)", yes, that looks odd.

> I THINK it would be better to bail early 

Bail under what conditions?

> and use do { .. } while(--i); instead.

GCC knows how to optimize loops.  Please don't make them less readable.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230861

FromJoakim Tjernlund <joakim.tjernlund@transmode.se>
Date2015-09-22 21:40 +0200
Message-ID<qbBzY-6tc-13@gated-at.bofh.it>
In reply to#1230774
On Tue, 2015-09-22 at 13:58 -0500, Scott Wood wrote:
> On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote:
> > On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
> > > flush/clean/invalidate _dcache_range() functions are all very
> > > similar and are quite short. They are mainly used in __dma_sync()
> > > perf_event locate them in the top 3 consumming functions during
> > > heavy ethernet activity
> > > 
> > > They are good candidate for inlining, as __dma_sync() does
> > > almost nothing but calling them
> > > 
> > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > ---
> > > New in v2
> > > 
> > >  arch/powerpc/include/asm/cacheflush.h | 55 +++++++++++++++++++++++++++--
> > >  arch/powerpc/kernel/misc_32.S         | 65 ------------------------------
> > > -----
> > >  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
> > >  3 files changed, 54 insertions(+), 68 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/include/asm/cacheflush.h 
> > > b/arch/powerpc/include/asm/cacheflush.h
> > > index 6229e6b..6169604 100644
> > > --- a/arch/powerpc/include/asm/cacheflush.h
> > > +++ b/arch/powerpc/include/asm/cacheflush.h
> > > @@ -47,12 +47,61 @@ static inline void 
> > > __flush_dcache_icache_phys(unsigned long physaddr)
> > >  }
> > >  #endif
> > >  
> > > -extern void flush_dcache_range(unsigned long start, unsigned long stop);
> > >  #ifdef CONFIG_PPC32
> > > -extern void clean_dcache_range(unsigned long start, unsigned long stop);
> > > -extern void invalidate_dcache_range(unsigned long start, unsigned long 
> > > stop);
> > > +/*
> > > + * Write any modified data cache blocks out to memory and invalidate 
> > > them.
> > > + * Does not invalidate the corresponding instruction cache blocks.
> > > + */
> > > +static inline void flush_dcache_range(unsigned long start, unsigned long 
> > > stop)
> > > +{
> > > +   void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> > > +   unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> > > +   unsigned int i;
> > > +
> > > +   for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> > > +           dcbf(addr);
> > > +   if (i)
> > > +           mb();   /* sync */
> > > +}
> > 
> > This feels optimized for the uncommon case when there is no invalidation.
> 
> If you mean the "if (i)", yes, that looks odd.

Yes.

> 
> > I THINK it would be better to bail early 
> 
> Bail under what conditions?

test for "i = 0" and return. 

> 
> > and use do { .. } while(--i); instead.
> 
> GCC knows how to optimize loops.  Please don't make them less readable.

Been a while since I checked but it used to be bad att transforming post inc to pre inc/dec
I remain unconvinced until I have seen it.

 Jocke
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230901

FromJoakim Tjernlund <joakim.tjernlund@transmode.se>
Date2015-09-22 22:00 +0200
Message-ID<qbBTl-6Qf-41@gated-at.bofh.it>
In reply to#1230861
On Tue, 2015-09-22 at 14:42 -0500, Scott Wood wrote:
> On Tue, 2015-09-22 at 19:34 +0000, Joakim Tjernlund wrote:
> > On Tue, 2015-09-22 at 13:58 -0500, Scott Wood wrote:
> > > On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote:
> > > > On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
> > > > > flush/clean/invalidate _dcache_range() functions are all very
> > > > > similar and are quite short. They are mainly used in __dma_sync()
> > > > > perf_event locate them in the top 3 consumming functions during
> > > > > heavy ethernet activity
> > > > > 
> > > > > They are good candidate for inlining, as __dma_sync() does
> > > > > almost nothing but calling them
> > > > > 
> > > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > > > ---
> > > > > New in v2
> > > > > 
> > > > >  arch/powerpc/include/asm/cacheflush.h | 55 
> > > > > +++++++++++++++++++++++++++--
> > > > >  arch/powerpc/kernel/misc_32.S         | 65 --------------------------
> > > > > ----
> > > > > -----
> > > > >  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
> > > > >  3 files changed, 54 insertions(+), 68 deletions(-)
> > > > > 
> > > > > diff --git a/arch/powerpc/include/asm/cacheflush.h 
> > > > > b/arch/powerpc/include/asm/cacheflush.h
> > > > > index 6229e6b..6169604 100644
> > > > > --- a/arch/powerpc/include/asm/cacheflush.h
> > > > > +++ b/arch/powerpc/include/asm/cacheflush.h
> > > > > @@ -47,12 +47,61 @@ static inline void 
> > > > > __flush_dcache_icache_phys(unsigned long physaddr)
> > > > >  }
> > > > >  #endif
> > > > >  
> > > > > -extern void flush_dcache_range(unsigned long start, unsigned long 
> > > > > stop);
> > > > >  #ifdef CONFIG_PPC32
> > > > > -extern void clean_dcache_range(unsigned long start, unsigned long 
> > > > > stop);
> > > > > -extern void invalidate_dcache_range(unsigned long start, unsigned 
> > > > > long 
> > > > > stop);
> > > > > +/*
> > > > > + * Write any modified data cache blocks out to memory and invalidate 
> > > > > them.
> > > > > + * Does not invalidate the corresponding instruction cache blocks.
> > > > > + */
> > > > > +static inline void flush_dcache_range(unsigned long start, unsigned 
> > > > > long 
> > > > > stop)
> > > > > +{
> > > > > +   void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> > > > > +   unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES -
> > > > > 1);
> > > > > +   unsigned int i;
> > > > > +
> > > > > +   for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += 
> > > > > L1_CACHE_BYTES)
> > > > > +           dcbf(addr);
> > > > > +   if (i)
> > > > > +           mb();   /* sync */
> > > > > +}
> > > > 
> > > > This feels optimized for the uncommon case when there is no 
> > > > invalidation.
> > > 
> > > If you mean the "if (i)", yes, that looks odd.
> > 
> > Yes.
> > 
> > > 
> > > > I THINK it would be better to bail early 
> > > 
> > > Bail under what conditions?
> > 
> > test for "i = 0" and return. 
> 
> Why bother?

I usally find it better to dela with special cases upfront så the rest doesn't need to
bother. i=0 is a NOP and it is clearer to show that upfront.

> 
> > 
> > > 
> > > > and use do { .. } while(--i); instead.
> > > 
> > > GCC knows how to optimize loops.  Please don't make them less readable.
> > 
> > Been a while since I checked but it used to be bad att transforming post 
> > inc to pre inc/dec
> > I remain unconvinced until I have seen it.
> 
> I would expect it to use bdnz for this loop, as the loop variable isn't 
> referenced in the loop body.
> 
> And generally the one proposing uglification-for-optimization should provide 
> the evidence. :-)

When it comes to gcc, past history is my evidence until proven otherwise :)
Maybe I will check again ...--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230909

FromJoakim Tjernlund <joakim.tjernlund@transmode.se>
Date2015-09-22 22:10 +0200
Message-ID<qbC30-7gK-27@gated-at.bofh.it>
In reply to#1230901
> > And generally the one proposing uglification-for-optimization should provide 
> > the evidence. :-)
> 
> When it comes to gcc, past history is my evidence until proven otherwise :)
> Maybe I will check again ...

OK then:
static inline void mb(void)
{
       __asm__ __volatile__ ("sync" : : : "memory");
}

static inline void dcbf(void *addr)
{
       __asm__ __volatile__ ("dcbf 0, %0" : : "r"(addr) : "memory");
}
#define L1_CACHE_SHIFT 5
#define L1_CACHE_BYTES  (1 << L1_CACHE_SHIFT)
void flush_dcache_range(unsigned long start, unsigned long stop)
{
       void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
       unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
       unsigned int i;

       for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
               dcbf(addr);
       if (i)
               mb();   /* sync */
}

gives:
flush_dcache_range:
	stwu %r1,-16(%r1)
	rlwinm %r3,%r3,0,0,26
	addi %r4,%r4,31
	subf %r9,%r3,%r4
	srwi. %r10,%r9,5
	beq %cr0,.L1
	mtctr %r10
	.p2align 4,,15
.L4:
#APP
 # 8 "gccloop.c" 1
	dcbf 0, %r3
 # 0 "" 2
#NO_APP
	addi %r3,%r3,32
	bdnz .L4
#APP
 # 3 "gccloop.c" 1
	sync
 # 0 "" 2
#NO_APP
.L1:
	addi %r1,%r1,16
	blr

good enough :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230928

FromJoakim Tjernlund <joakim.tjernlund@transmode.se>
Date2015-09-22 22:40 +0200
Message-ID<qbCw1-7OJ-15@gated-at.bofh.it>
In reply to#1230901
On Tue, 2015-09-22 at 15:35 -0500, Scott Wood wrote:
> On Tue, 2015-09-22 at 20:32 +0000, Joakim Tjernlund wrote:
> > On Tue, 2015-09-22 at 15:14 -0500, Scott Wood wrote:
> > > On Tue, 2015-09-22 at 19:55 +0000, Joakim Tjernlund wrote:
> > > > On Tue, 2015-09-22 at 14:42 -0500, Scott Wood wrote:
> > > > > On Tue, 2015-09-22 at 19:34 +0000, Joakim Tjernlund wrote:
> > > > > > On Tue, 2015-09-22 at 13:58 -0500, Scott Wood wrote:
> > > > > > > On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote:
> > > > > > > > On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
> > > > > > > > > flush/clean/invalidate _dcache_range() functions are all very
> > > > > > > > > similar and are quite short. They are mainly used in 
> > > > > > > > > __dma_sync()
> > > > > > > > > perf_event locate them in the top 3 consumming functions 
> > > > > > > > > during
> > > > > > > > > heavy ethernet activity
> > > > > > > > > 
> > > > > > > > > They are good candidate for inlining, as __dma_sync() does
> > > > > > > > > almost nothing but calling them
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > > > > > > > ---
> > > > > > > > > New in v2
> > > > > > > > > 
> > > > > > > > >  arch/powerpc/include/asm/cacheflush.h | 55 
> > > > > > > > > +++++++++++++++++++++++++++--
> > > > > > > > >  arch/powerpc/kernel/misc_32.S         | 65 ------------------
> > > > > > > > > ----
> > > > > > > > > ----
> > > > > > > > > ----
> > > > > > > > > -----
> > > > > > > > >  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
> > > > > > > > >  3 files changed, 54 insertions(+), 68 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/arch/powerpc/include/asm/cacheflush.h 
> > > > > > > > > b/arch/powerpc/include/asm/cacheflush.h
> > > > > > > > > index 6229e6b..6169604 100644
> > > > > > > > > --- a/arch/powerpc/include/asm/cacheflush.h
> > > > > > > > > +++ b/arch/powerpc/include/asm/cacheflush.h
> > > > > > > > > @@ -47,12 +47,61 @@ static inline void 
> > > > > > > > > __flush_dcache_icache_phys(unsigned long physaddr)
> > > > > > > > >  }
> > > > > > > > >  #endif
> > > > > > > > >  
> > > > > > > > > -extern void flush_dcache_range(unsigned long start, unsigned 
> > > > > > > > > long 
> > > > > > > > > stop);
> > > > > > > > >  #ifdef CONFIG_PPC32
> > > > > > > > > -extern void clean_dcache_range(unsigned long start, unsigned 
> > > > > > > > > long 
> > > > > > > > > stop);
> > > > > > > > > -extern void invalidate_dcache_range(unsigned long start, 
> > > > > > > > > unsigned 
> > > > > > > > > long 
> > > > > > > > > stop);
> > > > > > > > > +/*
> > > > > > > > > + * Write any modified data cache blocks out to memory and 
> > > > > > > > > invalidate 
> > > > > > > > > them.
> > > > > > > > > + * Does not invalidate the corresponding instruction cache 
> > > > > > > > > blocks.
> > > > > > > > > + */
> > > > > > > > > +static inline void flush_dcache_range(unsigned long start, 
> > > > > > > > > unsigned 
> > > > > > > > > long 
> > > > > > > > > stop)
> > > > > > > > > +{
> > > > > > > > > +   void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> > > > > > > > > +   unsigned int size = stop - (unsigned long)addr + 
> > > > > > > > > (L1_CACHE_BYTES -
> > > > > > > > > 1);
> > > > > > > > > +   unsigned int i;
> > > > > > > > > +
> > > > > > > > > +   for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += 
> > > > > > > > > L1_CACHE_BYTES)
> > > > > > > > > +           dcbf(addr);
> > > > > > > > > +   if (i)
> > > > > > > > > +           mb();   /* sync */
> > > > > > > > > +}
> > > > > > > > 
> > > > > > > > This feels optimized for the uncommon case when there is no 
> > > > > > > > invalidation.
> > > > > > > 
> > > > > > > If you mean the "if (i)", yes, that looks odd.
> > > > > > 
> > > > > > Yes.
> > > > > > 
> > > > > > > 
> > > > > > > > I THINK it would be better to bail early 
> > > > > > > 
> > > > > > > Bail under what conditions?
> > > > > > 
> > > > > > test for "i = 0" and return. 
> > > > > 
> > > > > Why bother?
> > > > 
> > > > I usally find it better to dela with special cases upfront så the rest 
> > > > doesn't need to
> > > > bother. i=0 is a NOP and it is clearer to show that upfront.
> > > 
> > > No, I mean why bother special casing this at all?
> > 
> > I just said why? 
> > You to found the if(i) mb() a bit odd and it took a little time to see why 
> > it was there.
> > Ahh, you mean just skip the if(i) and have mb() unconditionally?
> 
> Yes.
> 
> > That changes the semantics a little from the ASM version but perhaps that 
> > doesn't matter?
> 
> Adding more barriers than strictly necessary is a performance concern, not a 
> semantic change.
Semantics :)

>   How often are we really calling this function over an empty 
> range?

Never hopefully, it does not make much sense.

> 
> Not that it matters much one way or another...
probably not.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230950

FromChristophe Leroy <christophe.leroy@c-s.fr>
Date2015-09-22 23:00 +0200
Message-ID<qbCPo-8bs-25@gated-at.bofh.it>
In reply to#1230928

Le 22/09/2015 22:38, Joakim Tjernlund a écrit :
> On Tue, 2015-09-22 at 15:35 -0500, Scott Wood wrote:
>> On Tue, 2015-09-22 at 20:32 +0000, Joakim Tjernlund wrote:
>>> On Tue, 2015-09-22 at 15:14 -0500, Scott Wood wrote:
>>>> On Tue, 2015-09-22 at 19:55 +0000, Joakim Tjernlund wrote:
>>>>> On Tue, 2015-09-22 at 14:42 -0500, Scott Wood wrote:
>>>>>> On Tue, 2015-09-22 at 19:34 +0000, Joakim Tjernlund wrote:
>>>>>>> On Tue, 2015-09-22 at 13:58 -0500, Scott Wood wrote:
>>>>>>>> On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote:
>>>>>>>>> On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
>>>>>>>>>> flush/clean/invalidate _dcache_range() functions are all very
>>>>>>>>>> similar and are quite short. They are mainly used in
>>>>>>>>>> __dma_sync()
>>>>>>>>>> perf_event locate them in the top 3 consumming functions
>>>>>>>>>> during
>>>>>>>>>> heavy ethernet activity
>>>>>>>>>>
>>>>>>>>>> They are good candidate for inlining, as __dma_sync() does
>>>>>>>>>> almost nothing but calling them
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>>>>>>> ---
>>>>>>>>>> New in v2
>>>>>>>>>>
>>>>>>>>>>   arch/powerpc/include/asm/cacheflush.h | 55
>>>>>>>>>> +++++++++++++++++++++++++++--
>>>>>>>>>>   arch/powerpc/kernel/misc_32.S         | 65 ------------------
>>>>>>>>>> ----
>>>>>>>>>> ----
>>>>>>>>>> ----
>>>>>>>>>> -----
>>>>>>>>>>   arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
>>>>>>>>>>   3 files changed, 54 insertions(+), 68 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/powerpc/include/asm/cacheflush.h
>>>>>>>>>> b/arch/powerpc/include/asm/cacheflush.h
>>>>>>>>>> index 6229e6b..6169604 100644
>>>>>>>>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>>>>>>>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>>>>>>>>> @@ -47,12 +47,61 @@ static inline void
>>>>>>>>>> __flush_dcache_icache_phys(unsigned long physaddr)
>>>>>>>>>>   }
>>>>>>>>>>   #endif
>>>>>>>>>>   
>>>>>>>>>> -extern void flush_dcache_range(unsigned long start, unsigned
>>>>>>>>>> long
>>>>>>>>>> stop);
>>>>>>>>>>   #ifdef CONFIG_PPC32
>>>>>>>>>> -extern void clean_dcache_range(unsigned long start, unsigned
>>>>>>>>>> long
>>>>>>>>>> stop);
>>>>>>>>>> -extern void invalidate_dcache_range(unsigned long start,
>>>>>>>>>> unsigned
>>>>>>>>>> long
>>>>>>>>>> stop);
>>>>>>>>>> +/*
>>>>>>>>>> + * Write any modified data cache blocks out to memory and
>>>>>>>>>> invalidate
>>>>>>>>>> them.
>>>>>>>>>> + * Does not invalidate the corresponding instruction cache
>>>>>>>>>> blocks.
>>>>>>>>>> + */
>>>>>>>>>> +static inline void flush_dcache_range(unsigned long start,
>>>>>>>>>> unsigned
>>>>>>>>>> long
>>>>>>>>>> stop)
>>>>>>>>>> +{
>>>>>>>>>> +   void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
>>>>>>>>>> +   unsigned int size = stop - (unsigned long)addr +
>>>>>>>>>> (L1_CACHE_BYTES -
>>>>>>>>>> 1);
>>>>>>>>>> +   unsigned int i;
>>>>>>>>>> +
>>>>>>>>>> +   for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr +=
>>>>>>>>>> L1_CACHE_BYTES)
>>>>>>>>>> +           dcbf(addr);
>>>>>>>>>> +   if (i)
>>>>>>>>>> +           mb();   /* sync */
>>>>>>>>>> +}
>>>>>>>>> This feels optimized for the uncommon case when there is no
>>>>>>>>> invalidation.
>>>>>>>> If you mean the "if (i)", yes, that looks odd.
>>>>>>> Yes.
>>>>>>>
>>>>>>>>> I THINK it would be better to bail early
>>>>>>>> Bail under what conditions?
>>>>>>> test for "i = 0" and return.
>>>>>> Why bother?
>>>>> I usally find it better to dela with special cases upfront så the rest
>>>>> doesn't need to
>>>>> bother. i=0 is a NOP and it is clearer to show that upfront.
>>>> No, I mean why bother special casing this at all?
>>> I just said why?
>>> You to found the if(i) mb() a bit odd and it took a little time to see why
>>> it was there.
>>> Ahh, you mean just skip the if(i) and have mb() unconditionally?
>> Yes.
>>
>>> That changes the semantics a little from the ASM version but perhaps that
>>> doesn't matter?
>> Adding more barriers than strictly necessary is a performance concern, not a
>> semantic change.
> Semantics :)
>
>>    How often are we really calling this function over an empty
>> range?
> Never hopefully, it does not make much sense.
>
>> Not that it matters much one way or another...
> probably not.
>

Here is what I get in asm. First one is with "if (i) mb();". We see gcc 
puts a beqlr. This is the form that is closest to what we had in the 
former misc_32.S
Second one if with "mb()". Here we get a branch to sync for a useless sync

c000e0ac <my_flush_dcache_range1>:
c000e0ac:       54 63 00 36     rlwinm  r3,r3,0,0,27
c000e0b0:       38 84 00 0f     addi    r4,r4,15
c000e0b4:       7d 23 20 50     subf    r9,r3,r4
c000e0b8:       55 29 e1 3f     rlwinm. r9,r9,28,4,31
c000e0bc:       4d 82 00 20     beqlr
c000e0c0:       7d 29 03 a6     mtctr   r9
c000e0c4:       7c 00 18 6c     dcbst   0,r3
c000e0c8:       38 63 00 10     addi    r3,r3,16
c000e0cc:       42 00 ff f8     bdnz    c000e0c4 
<my_flush_dcache_range1+0x18>
c000e0d0:       7c 00 04 ac     sync
c000e0d4:       4e 80 00 20     blr

c000e0d8 <my_flush_dcache_range2>:
c000e0d8:       54 63 00 36     rlwinm  r3,r3,0,0,27
c000e0dc:       38 84 00 0f     addi    r4,r4,15
c000e0e0:       7d 23 20 50     subf    r9,r3,r4
c000e0e4:       55 29 e1 3f     rlwinm. r9,r9,28,4,31
c000e0e8:       41 82 00 14     beq     c000e0fc 
<my_flush_dcache_range2+0x24>
c000e0ec:       7d 29 03 a6     mtctr   r9
c000e0f0:       7c 00 18 6c     dcbst   0,r3
c000e0f4:       38 63 00 10     addi    r3,r3,16
c000e0f8:       42 00 ff f8     bdnz    c000e0f0 
<my_flush_dcache_range2+0x18>
c000e0fc:       7c 00 04 ac     sync
c000e100:       4e 80 00 20     blr

Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1231020

FromChristophe Leroy <christophe.leroy@c-s.fr>
Date2015-09-23 00:50 +0200
Message-ID<qbExP-2ia-13@gated-at.bofh.it>
In reply to#1230950

Le 23/09/2015 00:34, Scott Wood a écrit :
> On Tue, 2015-09-22 at 22:57 +0200, Christophe Leroy wrote:
>> >Here is what I get in asm. First one is with "if (i) mb();". We see gcc
>> >puts a beqlr. This is the form that is closest to what we had in the
>> >former misc_32.S
>> >Second one if with "mb()". Here we get a branch to sync for a useless sync
> I was more concerned with keeping the code simple than the asm output.
>
Right, but is that so complicated to say: if we did nothing in the loop, 
no need to sync ?

Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230935

FromJoakim Tjernlund <joakim.tjernlund@transmode.se>
Date2015-09-22 22:40 +0200
Message-ID<qbCw1-7OJ-19@gated-at.bofh.it>
In reply to#1230901
On Tue, 2015-09-22 at 15:14 -0500, Scott Wood wrote:
> On Tue, 2015-09-22 at 19:55 +0000, Joakim Tjernlund wrote:
> > On Tue, 2015-09-22 at 14:42 -0500, Scott Wood wrote:
> > > On Tue, 2015-09-22 at 19:34 +0000, Joakim Tjernlund wrote:
> > > > On Tue, 2015-09-22 at 13:58 -0500, Scott Wood wrote:
> > > > > On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote:
> > > > > > On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote:
> > > > > > > flush/clean/invalidate _dcache_range() functions are all very
> > > > > > > similar and are quite short. They are mainly used in __dma_sync()
> > > > > > > perf_event locate them in the top 3 consumming functions during
> > > > > > > heavy ethernet activity
> > > > > > > 
> > > > > > > They are good candidate for inlining, as __dma_sync() does
> > > > > > > almost nothing but calling them
> > > > > > > 
> > > > > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > > > > > ---
> > > > > > > New in v2
> > > > > > > 
> > > > > > >  arch/powerpc/include/asm/cacheflush.h | 55 
> > > > > > > +++++++++++++++++++++++++++--
> > > > > > >  arch/powerpc/kernel/misc_32.S         | 65 ----------------------
> > > > > > > ----
> > > > > > > ----
> > > > > > > -----
> > > > > > >  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
> > > > > > >  3 files changed, 54 insertions(+), 68 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/arch/powerpc/include/asm/cacheflush.h 
> > > > > > > b/arch/powerpc/include/asm/cacheflush.h
> > > > > > > index 6229e6b..6169604 100644
> > > > > > > --- a/arch/powerpc/include/asm/cacheflush.h
> > > > > > > +++ b/arch/powerpc/include/asm/cacheflush.h
> > > > > > > @@ -47,12 +47,61 @@ static inline void 
> > > > > > > __flush_dcache_icache_phys(unsigned long physaddr)
> > > > > > >  }
> > > > > > >  #endif
> > > > > > >  
> > > > > > > -extern void flush_dcache_range(unsigned long start, unsigned 
> > > > > > > long 
> > > > > > > stop);
> > > > > > >  #ifdef CONFIG_PPC32
> > > > > > > -extern void clean_dcache_range(unsigned long start, unsigned 
> > > > > > > long 
> > > > > > > stop);
> > > > > > > -extern void invalidate_dcache_range(unsigned long start, 
> > > > > > > unsigned 
> > > > > > > long 
> > > > > > > stop);
> > > > > > > +/*
> > > > > > > + * Write any modified data cache blocks out to memory and 
> > > > > > > invalidate 
> > > > > > > them.
> > > > > > > + * Does not invalidate the corresponding instruction cache 
> > > > > > > blocks.
> > > > > > > + */
> > > > > > > +static inline void flush_dcache_range(unsigned long start, 
> > > > > > > unsigned 
> > > > > > > long 
> > > > > > > stop)
> > > > > > > +{
> > > > > > > +   void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> > > > > > > +   unsigned int size = stop - (unsigned long)addr + 
> > > > > > > (L1_CACHE_BYTES -
> > > > > > > 1);
> > > > > > > +   unsigned int i;
> > > > > > > +
> > > > > > > +   for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += 
> > > > > > > L1_CACHE_BYTES)
> > > > > > > +           dcbf(addr);
> > > > > > > +   if (i)
> > > > > > > +           mb();   /* sync */
> > > > > > > +}
> > > > > > 
> > > > > > This feels optimized for the uncommon case when there is no 
> > > > > > invalidation.
> > > > > 
> > > > > If you mean the "if (i)", yes, that looks odd.
> > > > 
> > > > Yes.
> > > > 
> > > > > 
> > > > > > I THINK it would be better to bail early 
> > > > > 
> > > > > Bail under what conditions?
> > > > 
> > > > test for "i = 0" and return. 
> > > 
> > > Why bother?
> > 
> > I usally find it better to dela with special cases upfront så the rest 
> > doesn't need to
> > bother. i=0 is a NOP and it is clearer to show that upfront.
> 
> No, I mean why bother special casing this at all?

I just said why? 
You to found the if(i) mb() a bit odd and it took a little time to see why it was there.
Ahh, you mean just skip the if(i) and have mb() unconditionally?
That changes the semantics a little from the ASM version but perhaps that doesn't matter?--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234595

FromScott Wood <scottwood@freescale.com>
Date2015-09-29 02:30 +0200
Message-ID<qdQXU-759-27@gated-at.bofh.it>
In reply to#1230468
On Tue, Sep 22, 2015 at 06:51:13PM +0200, Christophe Leroy wrote:
> flush/clean/invalidate _dcache_range() functions are all very
> similar and are quite short. They are mainly used in __dma_sync()
> perf_event locate them in the top 3 consumming functions during
> heavy ethernet activity
> 
> They are good candidate for inlining, as __dma_sync() does
> almost nothing but calling them
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> New in v2
> 
>  arch/powerpc/include/asm/cacheflush.h | 55 +++++++++++++++++++++++++++--
>  arch/powerpc/kernel/misc_32.S         | 65 -----------------------------------
>  arch/powerpc/kernel/ppc_ksyms.c       |  2 ++
>  3 files changed, 54 insertions(+), 68 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> index 6229e6b..6169604 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -47,12 +47,61 @@ static inline void __flush_dcache_icache_phys(unsigned long physaddr)
>  }
>  #endif
>  
> -extern void flush_dcache_range(unsigned long start, unsigned long stop);
>  #ifdef CONFIG_PPC32
> -extern void clean_dcache_range(unsigned long start, unsigned long stop);
> -extern void invalidate_dcache_range(unsigned long start, unsigned long stop);
> +/*
> + * Write any modified data cache blocks out to memory and invalidate them.
> + * Does not invalidate the corresponding instruction cache blocks.
> + */
> +static inline void flush_dcache_range(unsigned long start, unsigned long stop)
> +{
> +	void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> +	unsigned int size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> +	unsigned int i;
> +
> +	for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> +		dcbf(addr);
> +	if (i)
> +		mb();	/* sync */
> +}

I know this is 32-bit-specific code, but it's still bad practice to use
"unsigned int" for addresses or sizes thereof.

-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web