Re: [ovs-dev] [PATCH] rculist: Fix iteration macros.
On 11/23/22 15:01, Ilya Maximets wrote:
> On 11/21/22 16:54, Adrian Moreno wrote:
>>
>>
>> On 11/4/22 15:25, Ilya Maximets wrote:
>>> Some macros for rculist have no users and there are no unit tests
>>> specific to that library as well, so broken code wasn't spotted
>>> while updating to multi-variable iterators.
>>>
>>> Fixing multiple problems like missing commas, parenthesis, incorrect
>>> variable and macro names.
>>>
>>> Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.")
>>> Reported-by: Subrata Nath
>>> Co-authored-by: Dumitru Ceara
>>> Signed-off-by: Dumitru Ceara
>>> Signed-off-by: Ilya Maximets
>>> ---
>>> lib/rculist.h | 18 +-
>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/lib/rculist.h b/lib/rculist.h
>>> index c0d77acf9..9bb8cbf3e 100644
>>> --- a/lib/rculist.h
>>> +++ b/lib/rculist.h
>>> @@ -380,18 +380,18 @@ rculist_is_singleton_protected(const struct rculist
>>> *list)
>>> #define RCULIST_FOR_EACH_REVERSE_PROTECTED(ITER, MEMBER, RCULIST)
>>> \
>>> for (INIT_MULTIVAR(ITER, MEMBER, (RCULIST)->prev, struct rculist);
>>> \
>>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>>> \
>>> - UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
>>> + UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
>>> #define RCULIST_FOR_EACH_REVERSE_PROTECTED_CONTINUE(ITER, MEMBER,
>>> RCULIST) \
>>> for (INIT_MULTIVAR(ITER, MEMBER, (ITER)->MEMBER.prev, struct
>>> rculist); \
>>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>>> \
>>> - UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
>>> + UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
>>>
>>
>> There's another hidden problem with the REVERSE iterators that has not
>> popped up yet: They access 'prev' member directly which will not compile
>> when using clang's thread-safety macros because of a fake mutex specifically
>> added to avoid it.
>> Since the macros are PROTECTED it should be OK to use
>> rculist_back_protected() instead.
>
> Hmm, interesting.
>
>>
>> I have written a unit test for rculist that I was planning to post soon. If
>> you prefer I can fix this at the same time.
>
> If you can fix that in your patch that would be easier, I think.
> I'll try to merge the current fix somewhere soon (Just got back
> from PTO today).
Applied now and backported down to 2.13.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rculist: Fix iteration macros.
On 11/21/22 16:54, Adrian Moreno wrote:
>
>
> On 11/4/22 15:25, Ilya Maximets wrote:
>> Some macros for rculist have no users and there are no unit tests
>> specific to that library as well, so broken code wasn't spotted
>> while updating to multi-variable iterators.
>>
>> Fixing multiple problems like missing commas, parenthesis, incorrect
>> variable and macro names.
>>
>> Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.")
>> Reported-by: Subrata Nath
>> Co-authored-by: Dumitru Ceara
>> Signed-off-by: Dumitru Ceara
>> Signed-off-by: Ilya Maximets
>> ---
>> lib/rculist.h | 18 +-
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/rculist.h b/lib/rculist.h
>> index c0d77acf9..9bb8cbf3e 100644
>> --- a/lib/rculist.h
>> +++ b/lib/rculist.h
>> @@ -380,18 +380,18 @@ rculist_is_singleton_protected(const struct rculist
>> *list)
>> #define RCULIST_FOR_EACH_REVERSE_PROTECTED(ITER, MEMBER, RCULIST)
>> \
>> for (INIT_MULTIVAR(ITER, MEMBER, (RCULIST)->prev, struct rculist);
>> \
>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
>> + UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
>> #define RCULIST_FOR_EACH_REVERSE_PROTECTED_CONTINUE(ITER, MEMBER,
>> RCULIST) \
>> for (INIT_MULTIVAR(ITER, MEMBER, (ITER)->MEMBER.prev, struct rculist);
>> \
>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
>> + UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
>>
>
> There's another hidden problem with the REVERSE iterators that has not popped
> up yet: They access 'prev' member directly which will not compile when using
> clang's thread-safety macros because of a fake mutex specifically added to
> avoid it.
> Since the macros are PROTECTED it should be OK to use
> rculist_back_protected() instead.
Hmm, interesting.
>
> I have written a unit test for rculist that I was planning to post soon. If
> you prefer I can fix this at the same time.
If you can fix that in your patch that would be easier, I think.
I'll try to merge the current fix somewhere soon (Just got back
from PTO today).
Best regards, Ilya Maximets.
>
>
>> #define RCULIST_FOR_EACH_PROTECTED(ITER, MEMBER, RCULIST)
>> \
>> for (INIT_MULTIVAR(ITER, MEMBER, rculist_next_protected(RCULIST),
>> \
>> struct rculist);
>> \
>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER)))
>> \
>> + UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER
>> \
>> #define RCULIST_FOR_EACH_SAFE_SHORT_PROTECTED(ITER, MEMBER, RCULIST)
>> \
>> for (INIT_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
>> \
>> @@ -399,18 +399,18 @@ rculist_is_singleton_protected(const struct rculist
>> *list)
>> struct rculist);
>> \
>> CONDITION_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
>> \
>> ITER_VAR(ITER) != (RCULIST),
>> \
>> - ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(VAR)));
>> \
>> - UPDATE_MULTIVAR_SHORT(ITER))
>> + ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(ITER)));
>> \
>> + UPDATE_MULTIVAR_SAFE_SHORT(ITER))
>> #define RCULIST_FOR_EACH_SAFE_LONG_PROTECTED(ITER, NEXT, MEMBER,
>> RCULIST) \
>> for (INIT_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
>> \
>> - rculist_next_protected(RCULIST)
>> \
>> + rculist_next_protected(RCULIST),
>> \
>> struct rculist);
>> \
>> - CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER
>> \
>> + CONDITION_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
>> \
>> ITER_VAR(ITER) != (RCULIST),
>> \
>> - ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(VAR)),
>> \
>> + ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(ITER)),
>> \
>> ITER_VAR(NEXT) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR_LONG(ITER))
>> + UPDATE_MULTIVAR_SAFE_LONG(ITER, NEXT))
>> #define RCULIST_FOR_EACH_SAFE_PROTECTED(...)
>> \
>> OVERLOAD_SAFE_MACRO(RCULIST_FOR_EACH_SAFE_LONG_PROTECTED,
>> \
>
> Thanks
___
dev
Re: [ovs-dev] [PATCH] rculist: Fix iteration macros.
On 11/4/22 15:25, Ilya Maximets wrote:
Some macros for rculist have no users and there are no unit tests
specific to that library as well, so broken code wasn't spotted
while updating to multi-variable iterators.
Fixing multiple problems like missing commas, parenthesis, incorrect
variable and macro names.
Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.")
Reported-by: Subrata Nath
Co-authored-by: Dumitru Ceara
Signed-off-by: Dumitru Ceara
Signed-off-by: Ilya Maximets
---
lib/rculist.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/rculist.h b/lib/rculist.h
index c0d77acf9..9bb8cbf3e 100644
--- a/lib/rculist.h
+++ b/lib/rculist.h
@@ -380,18 +380,18 @@ rculist_is_singleton_protected(const struct rculist *list)
#define RCULIST_FOR_EACH_REVERSE_PROTECTED(ITER, MEMBER, RCULIST)
\
for (INIT_MULTIVAR(ITER, MEMBER, (RCULIST)->prev, struct rculist);
\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
\
- UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
+ UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
#define RCULIST_FOR_EACH_REVERSE_PROTECTED_CONTINUE(ITER, MEMBER, RCULIST)\
for (INIT_MULTIVAR(ITER, MEMBER, (ITER)->MEMBER.prev, struct rculist);
\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
\
- UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
+ UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
There's another hidden problem with the REVERSE iterators that has not popped up
yet: They access 'prev' member directly which will not compile when using
clang's thread-safety macros because of a fake mutex specifically added to avoid it.
Since the macros are PROTECTED it should be OK to use rculist_back_protected()
instead.
I have written a unit test for rculist that I was planning to post soon. If you
prefer I can fix this at the same time.
#define RCULIST_FOR_EACH_PROTECTED(ITER, MEMBER, RCULIST)
\
for (INIT_MULTIVAR(ITER, MEMBER, rculist_next_protected(RCULIST),
\
struct rculist);
\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
\
- UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER)))\
+ UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER \
#define RCULIST_FOR_EACH_SAFE_SHORT_PROTECTED(ITER, MEMBER, RCULIST) \
for (INIT_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
\
@@ -399,18 +399,18 @@ rculist_is_singleton_protected(const struct rculist *list)
struct rculist);
\
CONDITION_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
\
ITER_VAR(ITER) != (RCULIST),
\
- ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(VAR)));\
-UPDATE_MULTIVAR_SHORT(ITER))
+ ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(ITER))); \
+UPDATE_MULTIVAR_SAFE_SHORT(ITER))
#define RCULIST_FOR_EACH_SAFE_LONG_PROTECTED(ITER, NEXT, MEMBER, RCULIST) \
for (INIT_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
\
- rculist_next_protected(RCULIST) \
+ rculist_next_protected(RCULIST), \
struct rculist);
\
- CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER \
+ CONDITION_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER, \
ITER_VAR(ITER) != (RCULIST),
\
- ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(VAR)), \
+ ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(ITER)), \
ITER_VAR(NEXT) != (RCULIST));
\
-UPDATE_MULTIVAR_LONG(ITER))
+UPDATE_MULTIVAR_SAFE_LONG(ITER, NEXT))
#define RCULIST_FOR_EACH_SAFE_PROTECTED(...) \
OVERLOAD_SAFE_MACRO(RCULIST_FOR_EACH_SAFE_LONG_PROTECTED,
\
Thanks
--
Adrián Moreno
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rculist: Fix iteration macros.
Acked-by: Alin-Gabriel Serdean
-Original Message-
From: dev On Behalf Of Ilya Maximets
Sent: Friday, November 4, 2022 4:26 PM
To: ovs-dev@openvswitch.org
Cc: Subrata Nath ; Dumitru Ceara
; Ilya Maximets
Subject: [ovs-dev] [PATCH] rculist: Fix iteration macros.
Some macros for rculist have no users and there are no unit tests specific
to that library as well, so broken code wasn't spotted while updating to
multi-variable iterators.
Fixing multiple problems like missing commas, parenthesis, incorrect
variable and macro names.
Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.")
Reported-by: Subrata Nath
Co-authored-by: Dumitru Ceara
Signed-off-by: Dumitru Ceara
Signed-off-by: Ilya Maximets
---
lib/rculist.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/rculist.h b/lib/rculist.h index c0d77acf9..9bb8cbf3e 100644
--- a/lib/rculist.h
+++ b/lib/rculist.h
@@ -380,18 +380,18 @@ rculist_is_singleton_protected(const struct rculist
*list)
#define RCULIST_FOR_EACH_REVERSE_PROTECTED(ITER, MEMBER, RCULIST)
\
for (INIT_MULTIVAR(ITER, MEMBER, (RCULIST)->prev, struct rculist);
\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
\
- UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
+ UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
#define RCULIST_FOR_EACH_REVERSE_PROTECTED_CONTINUE(ITER, MEMBER, RCULIST)
\
for (INIT_MULTIVAR(ITER, MEMBER, (ITER)->MEMBER.prev, struct rculist);
\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
\
- UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
+ UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
#define RCULIST_FOR_EACH_PROTECTED(ITER, MEMBER, RCULIST)
\
for (INIT_MULTIVAR(ITER, MEMBER, rculist_next_protected(RCULIST),
\
struct rculist);
\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
\
- UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER)))
\
+ UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER
\
#define RCULIST_FOR_EACH_SAFE_SHORT_PROTECTED(ITER, MEMBER, RCULIST)
\
for (INIT_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
\
@@ -399,18 +399,18 @@ rculist_is_singleton_protected(const struct rculist
*list)
struct rculist);
\
CONDITION_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
\
ITER_VAR(ITER) != (RCULIST),
\
- ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(VAR)));
\
-UPDATE_MULTIVAR_SHORT(ITER))
+ ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(ITER)));
\
+UPDATE_MULTIVAR_SAFE_SHORT(ITER))
#define RCULIST_FOR_EACH_SAFE_LONG_PROTECTED(ITER, NEXT, MEMBER, RCULIST)
\
for (INIT_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
\
- rculist_next_protected(RCULIST)
\
+ rculist_next_protected(RCULIST),
\
struct rculist);
\
- CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER
\
+ CONDITION_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
\
ITER_VAR(ITER) != (RCULIST),
\
- ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(VAR)),
\
+ ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(ITER)),
\
ITER_VAR(NEXT) != (RCULIST));
\
-UPDATE_MULTIVAR_LONG(ITER))
+UPDATE_MULTIVAR_SAFE_LONG(ITER, NEXT))
#define RCULIST_FOR_EACH_SAFE_PROTECTED(...)
\
OVERLOAD_SAFE_MACRO(RCULIST_FOR_EACH_SAFE_LONG_PROTECTED,
\
--
2.37.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] rculist: Fix iteration macros.
Some macros for rculist have no users and there are no unit tests
specific to that library as well, so broken code wasn't spotted
while updating to multi-variable iterators.
Fixing multiple problems like missing commas, parenthesis, incorrect
variable and macro names.
Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.")
Reported-by: Subrata Nath
Co-authored-by: Dumitru Ceara
Signed-off-by: Dumitru Ceara
Signed-off-by: Ilya Maximets
---
lib/rculist.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/rculist.h b/lib/rculist.h
index c0d77acf9..9bb8cbf3e 100644
--- a/lib/rculist.h
+++ b/lib/rculist.h
@@ -380,18 +380,18 @@ rculist_is_singleton_protected(const struct rculist *list)
#define RCULIST_FOR_EACH_REVERSE_PROTECTED(ITER, MEMBER, RCULIST) \
for (INIT_MULTIVAR(ITER, MEMBER, (RCULIST)->prev, struct rculist);\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST)); \
- UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
+ UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
#define RCULIST_FOR_EACH_REVERSE_PROTECTED_CONTINUE(ITER, MEMBER, RCULIST)\
for (INIT_MULTIVAR(ITER, MEMBER, (ITER)->MEMBER.prev, struct rculist);\
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST)); \
- UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
+ UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
#define RCULIST_FOR_EACH_PROTECTED(ITER, MEMBER, RCULIST) \
for (INIT_MULTIVAR(ITER, MEMBER, rculist_next_protected(RCULIST), \
struct rculist); \
CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST)); \
- UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER)))\
+ UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER \
#define RCULIST_FOR_EACH_SAFE_SHORT_PROTECTED(ITER, MEMBER, RCULIST) \
for (INIT_MULTIVAR_SAFE_SHORT(ITER, MEMBER, \
@@ -399,18 +399,18 @@ rculist_is_singleton_protected(const struct rculist *list)
struct rculist);\
CONDITION_MULTIVAR_SAFE_SHORT(ITER, MEMBER, \
ITER_VAR(ITER) != (RCULIST), \
- ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(VAR)));\
-UPDATE_MULTIVAR_SHORT(ITER))
+ ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(ITER))); \
+UPDATE_MULTIVAR_SAFE_SHORT(ITER))
#define RCULIST_FOR_EACH_SAFE_LONG_PROTECTED(ITER, NEXT, MEMBER, RCULIST) \
for (INIT_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER, \
- rculist_next_protected(RCULIST) \
+ rculist_next_protected(RCULIST), \
struct rculist); \
- CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER \
+ CONDITION_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER, \
ITER_VAR(ITER) != (RCULIST),\
- ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(VAR)), \
+ ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(ITER)), \
ITER_VAR(NEXT) != (RCULIST)); \
-UPDATE_MULTIVAR_LONG(ITER))
+UPDATE_MULTIVAR_SAFE_LONG(ITER, NEXT))
#define RCULIST_FOR_EACH_SAFE_PROTECTED(...) \
OVERLOAD_SAFE_MACRO(RCULIST_FOR_EACH_SAFE_LONG_PROTECTED, \
--
2.37.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
