| Message ID | 1515180624-27516-1-git-send-email-gvrose8192@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,V2] compat:inet_frag.h: Check for frag_percpu_counter_batch | expand |
> On Jan 5, 2018, at 11:30 AM, Greg Rose <gvrose8192@gmail.com> wrote: > > +#ifdef frag_percpu_counter_batch > ... > +#else /* frag_percpu_counter_batch */ This is kind of a nit, but I would have thought this "#else" comment would be "!frag_percpu_counter_batch", since that's the case when it's not defined. However, I'm not sure how it's handled usually in the kernel. Looking through our compat code, I see examples of it being done both ways, though. Thoughts? --Justin
On 1/23/2018 12:00 PM, Justin Pettit wrote: > >> On Jan 5, 2018, at 11:30 AM, Greg Rose <gvrose8192@gmail.com> wrote: >> >> +#ifdef frag_percpu_counter_batch >> ... >> +#else /* frag_percpu_counter_batch */ > This is kind of a nit, but I would have thought this "#else" comment would be "!frag_percpu_counter_batch", since that's the case when it's not defined. However, I'm not sure how it's handled usually in the kernel. Looking through our compat code, I see examples of it being done both ways, though. Thoughts? > > --Justin > > It's a valid nit. I can send V2 or you can fix it on push. Which do you prefer? Thanks, - Greg
On Tue, Jan 23, 2018 at 12:00:20PM -0800, Justin Pettit wrote: > > > > On Jan 5, 2018, at 11:30 AM, Greg Rose <gvrose8192@gmail.com> wrote: > > > > +#ifdef frag_percpu_counter_batch > > ... > > +#else /* frag_percpu_counter_batch */ > > This is kind of a nit, but I would have thought this "#else" comment > would be "!frag_percpu_counter_batch", since that's the case when it's > not defined. However, I'm not sure how it's handled usually in the > kernel. Looking through our compat code, I see examples of it being > done both ways, though. Thoughts? This is one of those weird places where style differs. GNU coding standards would mandate "!frag_percpu_counter_batch" here, and that's what I prefer myself, but I've seen the opposite convention in (if I recall correctly) some BSD code.
> On Jan 23, 2018, at 12:01 PM, Gregory Rose <gvrose8192@gmail.com> wrote: > > On 1/23/2018 12:00 PM, Justin Pettit wrote: >> >>> On Jan 5, 2018, at 11:30 AM, Greg Rose <gvrose8192@gmail.com> wrote: >>> >>> +#ifdef frag_percpu_counter_batch >>> ... >>> +#else /* frag_percpu_counter_batch */ >> This is kind of a nit, but I would have thought this "#else" comment would be "!frag_percpu_counter_batch", since that's the case when it's not defined. However, I'm not sure how it's handled usually in the kernel. Looking through our compat code, I see examples of it being done both ways, though. Thoughts? >> >> --Justin >> >> > It's a valid nit. > > I can send V2 or you can fix it on push. Which do you prefer? I went ahead and made the change. I pushed it to master and branch-2.9. --Justin
> On Jan 23, 2018, at 12:34 PM, Ben Pfaff <blp@ovn.org> wrote: > > On Tue, Jan 23, 2018 at 12:00:20PM -0800, Justin Pettit wrote: >> >> >>> On Jan 5, 2018, at 11:30 AM, Greg Rose <gvrose8192@gmail.com> wrote: >>> >>> +#ifdef frag_percpu_counter_batch >>> ... >>> +#else /* frag_percpu_counter_batch */ >> >> This is kind of a nit, but I would have thought this "#else" comment >> would be "!frag_percpu_counter_batch", since that's the case when it's >> not defined. However, I'm not sure how it's handled usually in the >> kernel. Looking through our compat code, I see examples of it being >> done both ways, though. Thoughts? > > This is one of those weird places where style differs. GNU coding > standards would mandate "!frag_percpu_counter_batch" here, and that's > what I prefer myself, but I've seen the opposite convention in (if I > recall correctly) some BSD code. Interesting. I decided to add the "!", since I think it makes it clearer. --Justin
On 1/23/2018 1:07 PM, Justin Pettit wrote: >> On Jan 23, 2018, at 12:01 PM, Gregory Rose <gvrose8192@gmail.com> wrote: >> >> On 1/23/2018 12:00 PM, Justin Pettit wrote: >>>> On Jan 5, 2018, at 11:30 AM, Greg Rose <gvrose8192@gmail.com> wrote: >>>> >>>> +#ifdef frag_percpu_counter_batch >>>> ... >>>> +#else /* frag_percpu_counter_batch */ >>> This is kind of a nit, but I would have thought this "#else" comment would be "!frag_percpu_counter_batch", since that's the case when it's not defined. However, I'm not sure how it's handled usually in the kernel. Looking through our compat code, I see examples of it being done both ways, though. Thoughts? >>> >>> --Justin >>> >>> >> It's a valid nit. >> >> I can send V2 or you can fix it on push. Which do you prefer? > I went ahead and made the change. I pushed it to master and branch-2.9. > > --Justin > > > Thanks!
diff --git a/datapath/linux/compat/include/net/inet_frag.h b/datapath/linux/compat/include/net/inet_frag.h index 34078c8..c98c3a4 100644 --- a/datapath/linux/compat/include/net/inet_frag.h +++ b/datapath/linux/compat/include/net/inet_frag.h @@ -30,6 +30,7 @@ static inline bool inet_frag_evicting(struct inet_frag_queue *q) #endif #ifndef HAVE_SUB_FRAG_MEM_LIMIT_ARG_STRUCT_NETNS_FRAGS +#ifdef frag_percpu_counter_batch static inline void rpl_sub_frag_mem_limit(struct netns_frags *nf, int i) { __percpu_counter_add(&nf->mem, -i, frag_percpu_counter_batch); @@ -41,6 +42,19 @@ static inline void rpl_add_frag_mem_limit(struct netns_frags *nf, int i) __percpu_counter_add(&nf->mem, i, frag_percpu_counter_batch); } #define add_frag_mem_limit rpl_add_frag_mem_limit +#else /* frag_percpu_counter_batch */ +static inline void rpl_sub_frag_mem_limit(struct netns_frags *nf, int i) +{ + atomic_sub(i, &nf->mem); +} +#define sub_frag_mem_limit rpl_sub_frag_mem_limit + +static inline void rpl_add_frag_mem_limit(struct netns_frags *nf, int i) +{ + atomic_add(i, &nf->mem); +} +#define add_frag_mem_limit rpl_add_frag_mem_limit +#endif /* frag_percpu_counter_batch */ #endif #ifdef HAVE_VOID_INET_FRAGS_INIT
Fix up the compat layer to check for frag_percpu_counter_batch and if not present then use atomic_sub and atomic_add as per the backport in the 3.16.50 LTS kernel. Fixes compile errors on 3.16 series kernels from 3.16.50 on. Signed-off-by: Greg Rose <gvrose8192@gmail.com> --- V2 - Fix typo --- datapath/linux/compat/include/net/inet_frag.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)