[ovs-dev,2/4] util: Extend ovs_prefetch_range to include prefetch type.

Message ID 1515778879-60075-2-git-send-email-bhanuprakash.bodireddy@intel.com
State Changes Requested
Delegated to: Ian Stokes
Headers show
Series
  • [ovs-dev,1/4] compiler: Introduce OVS_PREFETCH variants.
Related show

Commit Message

Bodireddy, Bhanuprakash Jan. 12, 2018, 5:41 p.m.
With ovs_prefetch_range(), large amounts of data can be prefetched in to
caches. Prefetch type gives better control over data caching strategy;
Meaning where the data should be prefetched(L1/L2/L3) and if the data
reference is temporal or non-temporal.

Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
---
 lib/pvector.h | 6 ++++--
 lib/util.h    | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Ian Stokes March 13, 2018, 10:37 a.m. | #1
> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Bhanuprakash Bodireddy
> Sent: Friday, January 12, 2018 5:41 PM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH 2/4] util: Extend ovs_prefetch_range to include
> prefetch type.
> 
> With ovs_prefetch_range(), large amounts of data can be prefetched in to
> caches. Prefetch type gives better control over data caching strategy;
> Meaning where the data should be prefetched(L1/L2/L3) and if the data
> reference is temporal or non-temporal.
> 
> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
> ---
>  lib/pvector.h | 6 ++++--
>  lib/util.h    | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/pvector.h b/lib/pvector.h index b175b21..d5655f0 100644
> --- a/lib/pvector.h
> +++ b/lib/pvector.h
> @@ -177,7 +177,8 @@ pvector_cursor_init(const struct pvector *pvec,
> 
>      impl = ovsrcu_get(struct pvector_impl *, &pvec->impl);
> 
> -    ovs_prefetch_range(impl->vector, impl->size * sizeof impl-
> >vector[0]);
> +    ovs_prefetch_range(impl->vector, impl->size * sizeof impl->vector[0],
> +                       OPCH_HTR);

Overall I think this look ok,

Again my only comment would be if people agree with the classification of OPCH_HTR for these calls. Could the hint OPCH_HTR have a different meaning on a different platform or is it expected to be the same across platforms?

> 
>      cursor.size = impl->size;
>      cursor.vector = impl->vector;
> @@ -208,7 +209,8 @@ static inline void pvector_cursor_lookahead(const
> struct pvector_cursor *cursor,
>                                              int n, size_t size)  {
>      if (cursor->entry_idx + n < cursor->size) {
> -        ovs_prefetch_range(cursor->vector[cursor->entry_idx + n].ptr,
> size);
> +        ovs_prefetch_range(cursor->vector[cursor->entry_idx + n].ptr,
> size,
> +                           OPCH_HTR);
>      }
>  }
> 
> diff --git a/lib/util.h b/lib/util.h
> index b6639b8..0a8ae23 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -73,13 +73,13 @@ BUILD_ASSERT_DECL(IS_POW2(CACHE_LINE_SIZE));
>  typedef uint8_t OVS_CACHE_LINE_MARKER[1];
> 
>  static inline void
> -ovs_prefetch_range(const void *start, size_t size)
> +ovs_prefetch_range(const void *start, size_t size, enum
> +ovs_prefetch_type type)
>  {
>      const char *addr = (const char *)start;
>      size_t ofs;
> 
>      for (ofs = 0; ofs < size; ofs += CACHE_LINE_SIZE) {
> -        OVS_PREFETCH(addr + ofs);
> +        OVS_PREFETCH_CACHE(addr + ofs, type);
>      }
>  }
> 
> --
> 2.4.11
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Bodireddy, Bhanuprakash March 13, 2018, 3:40 p.m. | #2
>
>> -----Original Message-----
>> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>> bounces@openvswitch.org] On Behalf Of Bhanuprakash Bodireddy
>> Sent: Friday, January 12, 2018 5:41 PM
>> To: dev@openvswitch.org
>> Subject: [ovs-dev] [PATCH 2/4] util: Extend ovs_prefetch_range to
>> include prefetch type.
>>
>> With ovs_prefetch_range(), large amounts of data can be prefetched in
>> to caches. Prefetch type gives better control over data caching
>> strategy; Meaning where the data should be prefetched(L1/L2/L3) and if
>> the data reference is temporal or non-temporal.
>>
>> Signed-off-by: Bhanuprakash Bodireddy
>> <bhanuprakash.bodireddy@intel.com>
>> ---
>>  lib/pvector.h | 6 ++++--
>>  lib/util.h    | 4 ++--
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/pvector.h b/lib/pvector.h index b175b21..d5655f0
>> 100644
>> --- a/lib/pvector.h
>> +++ b/lib/pvector.h
>> @@ -177,7 +177,8 @@ pvector_cursor_init(const struct pvector *pvec,
>>
>>      impl = ovsrcu_get(struct pvector_impl *, &pvec->impl);
>>
>> -    ovs_prefetch_range(impl->vector, impl->size * sizeof impl-
>> >vector[0]);
>> +    ovs_prefetch_range(impl->vector, impl->size * sizeof impl->vector[0],
>> +                       OPCH_HTR);
>
>Overall I think this look ok,
>
>Again my only comment would be if people agree with the classification of
>OPCH_HTR for these calls. Could the hint OPCH_HTR have a different meaning
>on a different platform or is it expected to be the same across platforms?

This patch doesn't change any functionality.

 On the master
     ovs_prefetch_range()  -->  OVS_PREFETCH(addr)  -->  __builtin_prefetch((addr))  -->  prefetcht0

With the patch
     ovs_prefetch_range(.., OPCH_HTR)  -->  OVS_PREFETCH_CACHE(addr , OPCH_HTR);-->  __builtin_prefetch(addr, 0, 3)  -->  prefetcht0

Please note that __builtin_prefetch(addr)  behaves as __builtin_prefetch(addr, 0, 3)  because with __builtin_prefetch(addr, rw, locality),
 the default value of 'rw' is 0 and 'locality' is '3' when 'rw' and 'locality' aren't explicitly passed to the function.

(Src: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html)

So this patch doesn't introduce anything different except that it makes the hint explicit with the new macro implementation.

Regards,
Bhanuprakash.

>
>>
>>      cursor.size = impl->size;
>>      cursor.vector = impl->vector;
>> @@ -208,7 +209,8 @@ static inline void pvector_cursor_lookahead(const
>> struct pvector_cursor *cursor,
>>                                              int n, size_t size)  {
>>      if (cursor->entry_idx + n < cursor->size) {
>> -        ovs_prefetch_range(cursor->vector[cursor->entry_idx + n].ptr,
>> size);
>> +        ovs_prefetch_range(cursor->vector[cursor->entry_idx + n].ptr,
>> size,
>> +                           OPCH_HTR);
>>      }
>>  }
>>
>> diff --git a/lib/util.h b/lib/util.h
>> index b6639b8..0a8ae23 100644
>> --- a/lib/util.h
>> +++ b/lib/util.h
>> @@ -73,13 +73,13 @@ BUILD_ASSERT_DECL(IS_POW2(CACHE_LINE_SIZE));
>>  typedef uint8_t OVS_CACHE_LINE_MARKER[1];
>>
>>  static inline void
>> -ovs_prefetch_range(const void *start, size_t size)
>> +ovs_prefetch_range(const void *start, size_t size, enum
>> +ovs_prefetch_type type)
>>  {
>>      const char *addr = (const char *)start;
>>      size_t ofs;
>>
>>      for (ofs = 0; ofs < size; ofs += CACHE_LINE_SIZE) {
>> -        OVS_PREFETCH(addr + ofs);
>> +        OVS_PREFETCH_CACHE(addr + ofs, type);
>>      }
>>  }
>>
>> --
>> 2.4.11
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch

diff --git a/lib/pvector.h b/lib/pvector.h
index b175b21..d5655f0 100644
--- a/lib/pvector.h
+++ b/lib/pvector.h
@@ -177,7 +177,8 @@  pvector_cursor_init(const struct pvector *pvec,
 
     impl = ovsrcu_get(struct pvector_impl *, &pvec->impl);
 
-    ovs_prefetch_range(impl->vector, impl->size * sizeof impl->vector[0]);
+    ovs_prefetch_range(impl->vector, impl->size * sizeof impl->vector[0],
+                       OPCH_HTR);
 
     cursor.size = impl->size;
     cursor.vector = impl->vector;
@@ -208,7 +209,8 @@  static inline void pvector_cursor_lookahead(const struct pvector_cursor *cursor,
                                             int n, size_t size)
 {
     if (cursor->entry_idx + n < cursor->size) {
-        ovs_prefetch_range(cursor->vector[cursor->entry_idx + n].ptr, size);
+        ovs_prefetch_range(cursor->vector[cursor->entry_idx + n].ptr, size,
+                           OPCH_HTR);
     }
 }
 
diff --git a/lib/util.h b/lib/util.h
index b6639b8..0a8ae23 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -73,13 +73,13 @@  BUILD_ASSERT_DECL(IS_POW2(CACHE_LINE_SIZE));
 typedef uint8_t OVS_CACHE_LINE_MARKER[1];
 
 static inline void
-ovs_prefetch_range(const void *start, size_t size)
+ovs_prefetch_range(const void *start, size_t size, enum ovs_prefetch_type type)
 {
     const char *addr = (const char *)start;
     size_t ofs;
 
     for (ofs = 0; ofs < size; ofs += CACHE_LINE_SIZE) {
-        OVS_PREFETCH(addr + ofs);
+        OVS_PREFETCH_CACHE(addr + ofs, type);
     }
 }