[ovs-dev,V2] compat:inet_frag.h: Check for frag_percpu_counter_batch

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
Related show

Commit Message

Gregory Rose Jan. 5, 2018, 7:30 p.m.
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(+)

Comments

Justin Pettit Jan. 23, 2018, 8 p.m. | #1
> 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
Gregory Rose Jan. 23, 2018, 8:01 p.m. | #2
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
Ben Pfaff Jan. 23, 2018, 8:34 p.m. | #3
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.
Justin Pettit Jan. 23, 2018, 9:07 p.m. | #4
> 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
Justin Pettit Jan. 23, 2018, 9:09 p.m. | #5
> 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
Gregory Rose Jan. 23, 2018, 9:19 p.m. | #6
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!

Patch

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