* [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
@ 2020-09-29 8:40 Junchang Wang
2020-10-03 2:24 ` Akira Yokosawa
0 siblings, 1 reply; 10+ messages in thread
From: Junchang Wang @ 2020-09-29 8:40 UTC (permalink / raw)
To: paulmck; +Cc: perfbook
The typo in atomic_add_return() incurs tricky bugs. For example, the current
atomic_dec_return() is indeed an atomic_add_return(). A similar typo appears
in the function atomic_add_negative().
Signed-off-by: Junchang Wang <junchang2020@gmail.com>
---
CodeSamples/api-pthreads/api-gcc.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/CodeSamples/api-pthreads/api-gcc.h b/CodeSamples/api-pthreads/api-gcc.h
index fbbedf2..9ad8a68 100644
--- a/CodeSamples/api-pthreads/api-gcc.h
+++ b/CodeSamples/api-pthreads/api-gcc.h
@@ -140,7 +140,7 @@ static __inline__ int atomic_inc_and_test(atomic_t *v)
*/
static __inline__ int atomic_add_negative(int i, atomic_t *v)
{
- return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST) < 0;
+ return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST) < 0;
}
/**
@@ -152,7 +152,7 @@ static __inline__ int atomic_add_negative(int i, atomic_t *v)
*/
static __inline__ int atomic_add_return(int i, atomic_t *v)
{
- return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST);
+ return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST);
}
static __inline__ int atomic_sub_return(int i, atomic_t *v)
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-09-29 8:40 [PATCH] api-gcc.h: fix typos in the functions atomic_add_* Junchang Wang
@ 2020-10-03 2:24 ` Akira Yokosawa
2020-10-03 4:28 ` Junchang Wang
0 siblings, 1 reply; 10+ messages in thread
From: Akira Yokosawa @ 2020-10-03 2:24 UTC (permalink / raw)
To: Junchang Wang, paulmck; +Cc: perfbook, Akira Yokosawa
Hi Junchang,
On Tue, 29 Sep 2020 16:40:06 +0800, Junchang Wang wrote:
> The typo in atomic_add_return() incurs tricky bugs. For example, the current
> atomic_dec_return() is indeed an atomic_add_return(). A similar typo appears
> in the function atomic_add_negative().
Nice catches!
You mean the current atomic_add_return(), atomic_sub_return(), and
atomic_dec_return() actually are atomic_inc_return(), don't you?
I wondered if these typos have affected code under CodeSamples.
Looks like atomic_add_negative(), atomic_add_return(), and
atomic_dec_return() are defined in api-gcc.h, but not used in anywhere
under CodeSamples.
On the other hand, atomic_sub_return() is used in SMPdesign/lockrwdeq.c.
./SMPdesign/lockrwdeq.c:100: if (atomic_sub_return(2, &d->counter) < 1) {
./SMPdesign/lockrwdeq.c:116: if (atomic_sub_return(2, &d->counter) < 1) {
./SMPdesign/lockrwdeq.c:131: if (atomic_sub_return(1, &d->counter) < 1) {
./SMPdesign/lockrwdeq.c:143: if (atomic_sub_return(1, &d->counter) < 1) {
All of them are actually atomic_inc_return(&d->counter)s!
Building lockrwdeq and running it at current master sometimes segfaults or deadlocks.
Applying Junchang's patch makes it much stable.
I've never looked into lockrqdeq.c (because it is not cited from perfbook text)
and don't have any clue.
Paul, what is your expectation of lockrwdeq's behavior?
Thanks, Akira
>
> Signed-off-by: Junchang Wang <junchang2020@gmail.com>
> ---
> CodeSamples/api-pthreads/api-gcc.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/CodeSamples/api-pthreads/api-gcc.h b/CodeSamples/api-pthreads/api-gcc.h
> index fbbedf2..9ad8a68 100644
> --- a/CodeSamples/api-pthreads/api-gcc.h
> +++ b/CodeSamples/api-pthreads/api-gcc.h
> @@ -140,7 +140,7 @@ static __inline__ int atomic_inc_and_test(atomic_t *v)
> */
> static __inline__ int atomic_add_negative(int i, atomic_t *v)
> {
> - return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST) < 0;
> + return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST) < 0;
> }
>
> /**
> @@ -152,7 +152,7 @@ static __inline__ int atomic_add_negative(int i, atomic_t *v)
> */
> static __inline__ int atomic_add_return(int i, atomic_t *v)
> {
> - return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST);
> + return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST);
> }
>
> static __inline__ int atomic_sub_return(int i, atomic_t *v)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-03 2:24 ` Akira Yokosawa
@ 2020-10-03 4:28 ` Junchang Wang
2020-10-03 16:19 ` Paul E. McKenney
0 siblings, 1 reply; 10+ messages in thread
From: Junchang Wang @ 2020-10-03 4:28 UTC (permalink / raw)
To: Akira Yokosawa; +Cc: paulmck, perfbook
On Sat, Oct 03, 2020 at 11:24:52AM +0900, Akira Yokosawa wrote:
>Hi Junchang,
>
>On Tue, 29 Sep 2020 16:40:06 +0800, Junchang Wang wrote:
>> The typo in atomic_add_return() incurs tricky bugs. For example, the current
>> atomic_dec_return() is indeed an atomic_add_return(). A similar typo appears
>> in the function atomic_add_negative().
>
>Nice catches!
Hi Akira,
>
>You mean the current atomic_add_return(), atomic_sub_return(), and
>atomic_dec_return() actually are atomic_inc_return(), don't you?
>
Yes. I meant to say "actually". :-)
>I wondered if these typos have affected code under CodeSamples.
>
No. Like you have mentioned in your mail, the affected functions are
atomic_add_negative(), atomic_add_return(), atomic_dec_return(), and
atomic_sub_return(). The code in CodeSamples does not use the first three
functions, and the last function is only used by SMPdesign/lockrwdeq.c which
does not appear in perfbook.
I encountered the bug when I was evaluating my parallel code, which utilizes
atomic_dec_return() in api-gcc.h, on the hashtorture benchmarking framework.
It would be great if we can fix the bugs because other programmers like me
will probably write their code based on the sample codes in perfbook.
Thanks,
--Junchang
>Looks like atomic_add_negative(), atomic_add_return(), and
>atomic_dec_return() are defined in api-gcc.h, but not used in anywhere
>under CodeSamples.
>
>On the other hand, atomic_sub_return() is used in SMPdesign/lockrwdeq.c.
>
>./SMPdesign/lockrwdeq.c:100: if (atomic_sub_return(2, &d->counter) < 1) {
>./SMPdesign/lockrwdeq.c:116: if (atomic_sub_return(2, &d->counter) < 1) {
>./SMPdesign/lockrwdeq.c:131: if (atomic_sub_return(1, &d->counter) < 1) {
>./SMPdesign/lockrwdeq.c:143: if (atomic_sub_return(1, &d->counter) < 1) {
>
>All of them are actually atomic_inc_return(&d->counter)s!
>
>Building lockrwdeq and running it at current master sometimes segfaults or deadlocks.
>
>Applying Junchang's patch makes it much stable.
>I've never looked into lockrqdeq.c (because it is not cited from perfbook text)
>and don't have any clue.
>
>Paul, what is your expectation of lockrwdeq's behavior?
>
> Thanks, Akira
>
>>
>> Signed-off-by: Junchang Wang <junchang2020@gmail.com>
>> ---
>> CodeSamples/api-pthreads/api-gcc.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/CodeSamples/api-pthreads/api-gcc.h b/CodeSamples/api-pthreads/api-gcc.h
>> index fbbedf2..9ad8a68 100644
>> --- a/CodeSamples/api-pthreads/api-gcc.h
>> +++ b/CodeSamples/api-pthreads/api-gcc.h
>> @@ -140,7 +140,7 @@ static __inline__ int atomic_inc_and_test(atomic_t *v)
>> */
>> static __inline__ int atomic_add_negative(int i, atomic_t *v)
>> {
>> - return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST) < 0;
>> + return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST) < 0;
>> }
>>
>> /**
>> @@ -152,7 +152,7 @@ static __inline__ int atomic_add_negative(int i, atomic_t *v)
>> */
>> static __inline__ int atomic_add_return(int i, atomic_t *v)
>> {
>> - return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST);
>> + return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST);
>> }
>>
>> static __inline__ int atomic_sub_return(int i, atomic_t *v)
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-03 4:28 ` Junchang Wang
@ 2020-10-03 16:19 ` Paul E. McKenney
2020-10-05 22:12 ` Paul E. McKenney
0 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2020-10-03 16:19 UTC (permalink / raw)
To: Junchang Wang; +Cc: Akira Yokosawa, perfbook
On Sat, Oct 03, 2020 at 12:28:47PM +0800, Junchang Wang wrote:
> On Sat, Oct 03, 2020 at 11:24:52AM +0900, Akira Yokosawa wrote:
> >Hi Junchang,
> >
> >On Tue, 29 Sep 2020 16:40:06 +0800, Junchang Wang wrote:
> >> The typo in atomic_add_return() incurs tricky bugs. For example, the current
> >> atomic_dec_return() is indeed an atomic_add_return(). A similar typo appears
> >> in the function atomic_add_negative().
> >
> >Nice catches!
What Akira said! I have pulled them in, thank you both!
(And please accept my apologies for being slow -- it has been an
eventful week.)
> Hi Akira,
>
> >
> >You mean the current atomic_add_return(), atomic_sub_return(), and
> >atomic_dec_return() actually are atomic_inc_return(), don't you?
> >
>
> Yes. I meant to say "actually". :-)
>
> >I wondered if these typos have affected code under CodeSamples.
> >
>
> No. Like you have mentioned in your mail, the affected functions are
> atomic_add_negative(), atomic_add_return(), atomic_dec_return(), and
> atomic_sub_return(). The code in CodeSamples does not use the first three
> functions, and the last function is only used by SMPdesign/lockrwdeq.c which
> does not appear in perfbook.
>
> I encountered the bug when I was evaluating my parallel code, which utilizes
> atomic_dec_return() in api-gcc.h, on the hashtorture benchmarking framework.
> It would be great if we can fix the bugs because other programmers like me
> will probably write their code based on the sample codes in perfbook.
>
>
> Thanks,
> --Junchang
>
> >Looks like atomic_add_negative(), atomic_add_return(), and
> >atomic_dec_return() are defined in api-gcc.h, but not used in anywhere
> >under CodeSamples.
> >
> >On the other hand, atomic_sub_return() is used in SMPdesign/lockrwdeq.c.
> >
> >./SMPdesign/lockrwdeq.c:100: if (atomic_sub_return(2, &d->counter) < 1) {
> >./SMPdesign/lockrwdeq.c:116: if (atomic_sub_return(2, &d->counter) < 1) {
> >./SMPdesign/lockrwdeq.c:131: if (atomic_sub_return(1, &d->counter) < 1) {
> >./SMPdesign/lockrwdeq.c:143: if (atomic_sub_return(1, &d->counter) < 1) {
> >
> >All of them are actually atomic_inc_return(&d->counter)s!
> >
> >Building lockrwdeq and running it at current master sometimes segfaults or deadlocks.
> >
> >Applying Junchang's patch makes it much stable.
> >I've never looked into lockrqdeq.c (because it is not cited from perfbook text)
> >and don't have any clue.
> >
> >Paul, what is your expectation of lockrwdeq's behavior?
I would expect it to be a double-ended queue, just like the other similar
programs. I had completely forgotten about it, but my 2015 discussion
with Dominik Dingel leads me to believe that it passed tests back then.
And it looks like I introduced this bug with my 2016 commit cee469f9a7aa
("api: Switch x86 atomics to gcc C11-like atomics"). I added a Fixes
line as shown below. Please let me know if I messed anything up.
But sadly, this change does not fix this bug:
$ ./hash_bkt_hazptr --schroedinger --nreaders 1 --duration 1000 --updatewait 0 --nbuckets 8192 --elems/writer 8192
Segmentation fault (core dumped)
Can't have everything. ;-)
It appears that hashtorture manages to fail to call the per-thread
hazard-pointers initialization on some paths, so it is no surprise
that Junchang's fix doesn't fix this bug.
Thanx, Paul
------------------------------------------------------------------------
commit 84d5f87216975a944220ab998818728608705171
Author: Junchang Wang <junchang2020@gmail.com>
Date: Tue Sep 29 16:40:06 2020 +0800
api-gcc.h: Fix typos in the functions atomic_add_*
The typo in atomic_add_return() results in tricky bugs. For example,
the current atomic_dec_return() is instead an atomic_add_return(). A
similar typo appears in the function atomic_add_negative().
Fixes: cee469f9a7aa ("api: Switch x86 atomics to gcc C11-like atomics")
Signed-off-by: Junchang Wang <junchang2020@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/CodeSamples/api-pthreads/api-gcc.h b/CodeSamples/api-pthreads/api-gcc.h
index fbbedf2..9ad8a68 100644
--- a/CodeSamples/api-pthreads/api-gcc.h
+++ b/CodeSamples/api-pthreads/api-gcc.h
@@ -140,7 +140,7 @@ static __inline__ int atomic_inc_and_test(atomic_t *v)
*/
static __inline__ int atomic_add_negative(int i, atomic_t *v)
{
- return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST) < 0;
+ return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST) < 0;
}
/**
@@ -152,7 +152,7 @@ static __inline__ int atomic_add_negative(int i, atomic_t *v)
*/
static __inline__ int atomic_add_return(int i, atomic_t *v)
{
- return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST);
+ return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST);
}
static __inline__ int atomic_sub_return(int i, atomic_t *v)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-03 16:19 ` Paul E. McKenney
@ 2020-10-05 22:12 ` Paul E. McKenney
2020-10-06 6:40 ` Junchang Wang
0 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2020-10-05 22:12 UTC (permalink / raw)
To: Junchang Wang; +Cc: Akira Yokosawa, perfbook
On Sat, Oct 03, 2020 at 09:19:25AM -0700, Paul E. McKenney wrote:
> On Sat, Oct 03, 2020 at 12:28:47PM +0800, Junchang Wang wrote:
[ . . . ]
> But sadly, this change does not fix this bug:
>
> $ ./hash_bkt_hazptr --schroedinger --nreaders 1 --duration 1000 --updatewait 0 --nbuckets 8192 --elems/writer 8192
> Segmentation fault (core dumped)
>
> Can't have everything. ;-)
>
> It appears that hashtorture manages to fail to call the per-thread
> hazard-pointers initialization on some paths, so it is no surprise
> that Junchang's fix doesn't fix this bug.
Thanx, Paul
------------------------------------------------------------------------
commit 2533decebb7fca65d2860174246966f5ed255dd0
Author: Paul E. McKenney <paulmck@kernel.org>
Date: Mon Oct 5 15:10:03 2020 -0700
datastruct/hash: Add missing hash_register_thread()
The zoo_test() function was missing the hash_register_thread() invocation
that is required to make hazard pointers work correctly. The lack
of this invocation results in a segmentation violation. This commit
therefore adds this call.
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/CodeSamples/datastruct/hash/hashtorture.h b/CodeSamples/datastruct/hash/hashtorture.h
index 6d30cb8..7d43e11 100644
--- a/CodeSamples/datastruct/hash/hashtorture.h
+++ b/CodeSamples/datastruct/hash/hashtorture.h
@@ -1211,6 +1211,7 @@ void zoo_test(void)
for (i = 0; i < nupdaters * elperupdater; i++) {
sprintf(&zoo_names[ZOO_NAMELEN * i], "a%ld", i);
}
+ hash_register_thread();
zhep = malloc(sizeof(*zhep));
BUG_ON(!zhep);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-05 22:12 ` Paul E. McKenney
@ 2020-10-06 6:40 ` Junchang Wang
2020-10-06 15:41 ` Paul E. McKenney
0 siblings, 1 reply; 10+ messages in thread
From: Junchang Wang @ 2020-10-06 6:40 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Akira Yokosawa, perfbook
On Mon, Oct 05, 2020 at 03:12:22PM -0700, Paul E. McKenney wrote:
>On Sat, Oct 03, 2020 at 09:19:25AM -0700, Paul E. McKenney wrote:
>> On Sat, Oct 03, 2020 at 12:28:47PM +0800, Junchang Wang wrote:
>
>[ . . . ]
>
>> But sadly, this change does not fix this bug:
>>
>> $ ./hash_bkt_hazptr --schroedinger --nreaders 1 --duration 1000 --updatewait 0 --nbuckets 8192 --elems/writer 8192
>> Segmentation fault (core dumped)
>>
>> Can't have everything. ;-)
>>
>> It appears that hashtorture manages to fail to call the per-thread
>> hazard-pointers initialization on some paths, so it is no surprise
>> that Junchang's fix doesn't fix this bug.
>
> Thanx, Paul
>
>------------------------------------------------------------------------
>
>commit 2533decebb7fca65d2860174246966f5ed255dd0
>Author: Paul E. McKenney <paulmck@kernel.org>
>Date: Mon Oct 5 15:10:03 2020 -0700
>
> datastruct/hash: Add missing hash_register_thread()
>
> The zoo_test() function was missing the hash_register_thread() invocation
> that is required to make hazard pointers work correctly. The lack
> of this invocation results in a segmentation violation. This commit
> therefore adds this call.
>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>
Hi Paul,
This patch fixes the bug, and now "./hash_bkt_hazptr --schroedinger" works
correctly on my servers. Thanks a lot.
--Junchang
>diff --git a/CodeSamples/datastruct/hash/hashtorture.h b/CodeSamples/datastruct/hash/hashtorture.h
>index 6d30cb8..7d43e11 100644
>--- a/CodeSamples/datastruct/hash/hashtorture.h
>+++ b/CodeSamples/datastruct/hash/hashtorture.h
>@@ -1211,6 +1211,7 @@ void zoo_test(void)
> for (i = 0; i < nupdaters * elperupdater; i++) {
> sprintf(&zoo_names[ZOO_NAMELEN * i], "a%ld", i);
> }
>+ hash_register_thread();
>
> zhep = malloc(sizeof(*zhep));
> BUG_ON(!zhep);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-06 6:40 ` Junchang Wang
@ 2020-10-06 15:41 ` Paul E. McKenney
2020-10-07 1:50 ` Junchang Wang
0 siblings, 1 reply; 10+ messages in thread
From: Paul E. McKenney @ 2020-10-06 15:41 UTC (permalink / raw)
To: Junchang Wang; +Cc: Akira Yokosawa, perfbook
On Tue, Oct 06, 2020 at 02:40:20PM +0800, Junchang Wang wrote:
> On Mon, Oct 05, 2020 at 03:12:22PM -0700, Paul E. McKenney wrote:
> >On Sat, Oct 03, 2020 at 09:19:25AM -0700, Paul E. McKenney wrote:
> >> On Sat, Oct 03, 2020 at 12:28:47PM +0800, Junchang Wang wrote:
> >
> >[ . . . ]
> >
> >> But sadly, this change does not fix this bug:
> >>
> >> $ ./hash_bkt_hazptr --schroedinger --nreaders 1 --duration 1000 --updatewait 0 --nbuckets 8192 --elems/writer 8192
> >> Segmentation fault (core dumped)
> >>
> >> Can't have everything. ;-)
> >>
> >> It appears that hashtorture manages to fail to call the per-thread
> >> hazard-pointers initialization on some paths, so it is no surprise
> >> that Junchang's fix doesn't fix this bug.
> >
> > Thanx, Paul
> >
> >------------------------------------------------------------------------
> >
> >commit 2533decebb7fca65d2860174246966f5ed255dd0
> >Author: Paul E. McKenney <paulmck@kernel.org>
> >Date: Mon Oct 5 15:10:03 2020 -0700
> >
> > datastruct/hash: Add missing hash_register_thread()
> >
> > The zoo_test() function was missing the hash_register_thread() invocation
> > that is required to make hazard pointers work correctly. The lack
> > of this invocation results in a segmentation violation. This commit
> > therefore adds this call.
> >
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> >
>
> Hi Paul,
>
> This patch fixes the bug, and now "./hash_bkt_hazptr --schroedinger" works
> correctly on my servers. Thanks a lot.
Thank you for testing! May I apply your Tested-by?
Thanx, Paul
> --Junchang
>
> >diff --git a/CodeSamples/datastruct/hash/hashtorture.h b/CodeSamples/datastruct/hash/hashtorture.h
> >index 6d30cb8..7d43e11 100644
> >--- a/CodeSamples/datastruct/hash/hashtorture.h
> >+++ b/CodeSamples/datastruct/hash/hashtorture.h
> >@@ -1211,6 +1211,7 @@ void zoo_test(void)
> > for (i = 0; i < nupdaters * elperupdater; i++) {
> > sprintf(&zoo_names[ZOO_NAMELEN * i], "a%ld", i);
> > }
> >+ hash_register_thread();
> >
> > zhep = malloc(sizeof(*zhep));
> > BUG_ON(!zhep);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-06 15:41 ` Paul E. McKenney
@ 2020-10-07 1:50 ` Junchang Wang
2020-10-07 22:31 ` Paul E. McKenney
0 siblings, 1 reply; 10+ messages in thread
From: Junchang Wang @ 2020-10-07 1:50 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Akira Yokosawa, perfbook
On Tue, Oct 06, 2020 at 08:41:49AM -0700, Paul E. McKenney wrote:
>On Tue, Oct 06, 2020 at 02:40:20PM +0800, Junchang Wang wrote:
>> On Mon, Oct 05, 2020 at 03:12:22PM -0700, Paul E. McKenney wrote:
>> >On Sat, Oct 03, 2020 at 09:19:25AM -0700, Paul E. McKenney wrote:
>> >> On Sat, Oct 03, 2020 at 12:28:47PM +0800, Junchang Wang wrote:
>> >
>> >[ . . . ]
>> >
>> >> But sadly, this change does not fix this bug:
>> >>
>> >> $ ./hash_bkt_hazptr --schroedinger --nreaders 1 --duration 1000 --updatewait 0 --nbuckets 8192 --elems/writer 8192
>> >> Segmentation fault (core dumped)
>> >>
>> >> Can't have everything. ;-)
>> >>
>> >> It appears that hashtorture manages to fail to call the per-thread
>> >> hazard-pointers initialization on some paths, so it is no surprise
>> >> that Junchang's fix doesn't fix this bug.
>> >
>> > Thanx, Paul
>> >
>> >------------------------------------------------------------------------
>> >
>> >commit 2533decebb7fca65d2860174246966f5ed255dd0
>> >Author: Paul E. McKenney <paulmck@kernel.org>
>> >Date: Mon Oct 5 15:10:03 2020 -0700
>> >
>> > datastruct/hash: Add missing hash_register_thread()
>> >
>> > The zoo_test() function was missing the hash_register_thread() invocation
>> > that is required to make hazard pointers work correctly. The lack
>> > of this invocation results in a segmentation violation. This commit
>> > therefore adds this call.
>> >
>> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>> >
>>
>> Hi Paul,
>>
>> This patch fixes the bug, and now "./hash_bkt_hazptr --schroedinger" works
>> correctly on my servers. Thanks a lot.
>
>Thank you for testing! May I apply your Tested-by?
>
Hi Paul,
I'm very happy that I can help improve perfbook. Please add a
Tested-by: Junchang Wang <junchang2020@gmail.com>
Thanks,
--Junchang
> Thanx, Paul
>
>> --Junchang
>>
>> >diff --git a/CodeSamples/datastruct/hash/hashtorture.h b/CodeSamples/datastruct/hash/hashtorture.h
>> >index 6d30cb8..7d43e11 100644
>> >--- a/CodeSamples/datastruct/hash/hashtorture.h
>> >+++ b/CodeSamples/datastruct/hash/hashtorture.h
>> >@@ -1211,6 +1211,7 @@ void zoo_test(void)
>> > for (i = 0; i < nupdaters * elperupdater; i++) {
>> > sprintf(&zoo_names[ZOO_NAMELEN * i], "a%ld", i);
>> > }
>> >+ hash_register_thread();
>> >
>> > zhep = malloc(sizeof(*zhep));
>> > BUG_ON(!zhep);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
2020-10-07 1:50 ` Junchang Wang
@ 2020-10-07 22:31 ` Paul E. McKenney
0 siblings, 0 replies; 10+ messages in thread
From: Paul E. McKenney @ 2020-10-07 22:31 UTC (permalink / raw)
To: Junchang Wang; +Cc: Akira Yokosawa, perfbook
On Wed, Oct 07, 2020 at 09:50:35AM +0800, Junchang Wang wrote:
> On Tue, Oct 06, 2020 at 08:41:49AM -0700, Paul E. McKenney wrote:
> >On Tue, Oct 06, 2020 at 02:40:20PM +0800, Junchang Wang wrote:
> >> On Mon, Oct 05, 2020 at 03:12:22PM -0700, Paul E. McKenney wrote:
> >> >On Sat, Oct 03, 2020 at 09:19:25AM -0700, Paul E. McKenney wrote:
> >> >> On Sat, Oct 03, 2020 at 12:28:47PM +0800, Junchang Wang wrote:
> >> >
> >> >[ . . . ]
> >> >
> >> >> But sadly, this change does not fix this bug:
> >> >>
> >> >> $ ./hash_bkt_hazptr --schroedinger --nreaders 1 --duration 1000 --updatewait 0 --nbuckets 8192 --elems/writer 8192
> >> >> Segmentation fault (core dumped)
> >> >>
> >> >> Can't have everything. ;-)
> >> >>
> >> >> It appears that hashtorture manages to fail to call the per-thread
> >> >> hazard-pointers initialization on some paths, so it is no surprise
> >> >> that Junchang's fix doesn't fix this bug.
> >> >
> >> > Thanx, Paul
> >> >
> >> >------------------------------------------------------------------------
> >> >
> >> >commit 2533decebb7fca65d2860174246966f5ed255dd0
> >> >Author: Paul E. McKenney <paulmck@kernel.org>
> >> >Date: Mon Oct 5 15:10:03 2020 -0700
> >> >
> >> > datastruct/hash: Add missing hash_register_thread()
> >> >
> >> > The zoo_test() function was missing the hash_register_thread() invocation
> >> > that is required to make hazard pointers work correctly. The lack
> >> > of this invocation results in a segmentation violation. This commit
> >> > therefore adds this call.
> >> >
> >> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> >> >
> >>
> >> Hi Paul,
> >>
> >> This patch fixes the bug, and now "./hash_bkt_hazptr --schroedinger" works
> >> correctly on my servers. Thanks a lot.
> >
> >Thank you for testing! May I apply your Tested-by?
>
> Hi Paul,
>
> I'm very happy that I can help improve perfbook. Please add a
>
> Tested-by: Junchang Wang <junchang2020@gmail.com>
Applied, thank you!
Thanx, Paul
> Thanks,
> --Junchang
>
>
> > Thanx, Paul
> >
> >> --Junchang
> >>
> >> >diff --git a/CodeSamples/datastruct/hash/hashtorture.h b/CodeSamples/datastruct/hash/hashtorture.h
> >> >index 6d30cb8..7d43e11 100644
> >> >--- a/CodeSamples/datastruct/hash/hashtorture.h
> >> >+++ b/CodeSamples/datastruct/hash/hashtorture.h
> >> >@@ -1211,6 +1211,7 @@ void zoo_test(void)
> >> > for (i = 0; i < nupdaters * elperupdater; i++) {
> >> > sprintf(&zoo_names[ZOO_NAMELEN * i], "a%ld", i);
> >> > }
> >> >+ hash_register_thread();
> >> >
> >> > zhep = malloc(sizeof(*zhep));
> >> > BUG_ON(!zhep);
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] api-gcc.h: fix typos in the functions atomic_add_*
@ 2020-09-29 8:35 Junchang Wang
0 siblings, 0 replies; 10+ messages in thread
From: Junchang Wang @ 2020-09-29 8:35 UTC (permalink / raw)
To: paulmck; +Cc: perfbook
The typo in atomic_add_return() incurs tricky bugs. For example, the current
atomic_dec_return() is indeed an atomic_add_return(). A similar typo appears
in the function atomic_add_negative().
Signed-off-by: Junchang Wang <junchang2020@gmail.com>
---
CodeSamples/api-pthreads/api-gcc.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/CodeSamples/api-pthreads/api-gcc.h b/CodeSamples/api-pthreads/api-gcc.h
index fbbedf2..9ad8a68 100644
--- a/CodeSamples/api-pthreads/api-gcc.h
+++ b/CodeSamples/api-pthreads/api-gcc.h
@@ -140,7 +140,7 @@ static __inline__ int atomic_inc_and_test(atomic_t *v)
*/
static __inline__ int atomic_add_negative(int i, atomic_t *v)
{
- return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST) < 0;
+ return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST) < 0;
}
/**
@@ -152,7 +152,7 @@ static __inline__ int atomic_add_negative(int i, atomic_t *v)
*/
static __inline__ int atomic_add_return(int i, atomic_t *v)
{
- return __atomic_add_fetch(&v->counter, 1, __ATOMIC_SEQ_CST);
+ return __atomic_add_fetch(&v->counter, i, __ATOMIC_SEQ_CST);
}
static __inline__ int atomic_sub_return(int i, atomic_t *v)
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-07 22:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 8:40 [PATCH] api-gcc.h: fix typos in the functions atomic_add_* Junchang Wang
2020-10-03 2:24 ` Akira Yokosawa
2020-10-03 4:28 ` Junchang Wang
2020-10-03 16:19 ` Paul E. McKenney
2020-10-05 22:12 ` Paul E. McKenney
2020-10-06 6:40 ` Junchang Wang
2020-10-06 15:41 ` Paul E. McKenney
2020-10-07 1:50 ` Junchang Wang
2020-10-07 22:31 ` Paul E. McKenney
-- strict thread matches above, loose matches on Subject: below --
2020-09-29 8:35 Junchang Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.