| 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. | expand |
> -----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
> >> -----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
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); } }
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(-)