Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Panu Matilainen]

Thanks for the patch!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-647424064___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Panu Matilainen]

Merged #1264 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#event-3467630072___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[ニール・ゴンパ]

This patch is now running in OpenMandriva Cooker and seems to be working okay 
so far...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-647325614___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Panu Matilainen]

If you don't mind... yeah, please do :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-645818174___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Tom Stellard]

@pmatilai Do you want me to re-submit this pull request with your suggested fix?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-645700387___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Tom Stellard]

> Okay, the problem is fairly obvious, pkg being declared way outside the 
> parallel block.
> But rather than sprinkling more OpenMP magic dust to solve it, can you verify 
> that this (ie just declare the variable inside the parallel block) solves the 
> problem?
> 
> ```
>  #pragma omp single
>  for (int i = 0; i < npkgs; i++) {
> -   pkg = tasks[i];
> +   Package pkg = tasks[i];

I just checked and doing this also fixes the bug.

> #pragma omp task untied priority(i)
> {
> pkg->rc = packageBinary(spec, pkg, cookie, cheating, >filename);
> ```



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-644201067___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Panu Matilainen]

Okay, the problem is fairly obvious, pkg being declared way outside the 
parallel block.
But rather than sprinkling more OpenMP magic dust to solve it, can you verify 
that this (ie just declare the variable inside the parallel block) solves the 
problem?

``` #pragma omp parallel
 #pragma omp single
 for (int i = 0; i < npkgs; i++) {
-   pkg = tasks[i];
+   Package pkg = tasks[i];
#pragma omp task untied priority(i)
{
pkg->rc = packageBinary(spec, pkg, cookie, cheating, >filename);

```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-643998971___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[ニール・ゴンパ]

@ffesti I'd appreciate this being pulled into rpm-4.16 since this is going to 
be needed for OpenMandriva.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264#issuecomment-641615329___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [rpm-software-management/rpm] Fix data race in packageBinaries() function (#1264)

[Tom Stellard]

The pkg variable used in the parallel loop was declared outside
of the omp parallel construct, so it was shared among tasks.  This
had the potential to cause a data race.  The gcc openmp implementation
did not hit this problem, but I uncovered it while trying to compile with
clang.  My best guess as to what was happening is that after the last
task was launched, all tasks had the same value of pkg and were operating
on the same data at the same time.

This patch marks the variable as firstprivate, so each tasks gets its
own copy of the variable.
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1264

-- Commit Summary --

  * Fix data race in packageBinaries() function

-- File Changes --

M build/pack.c (2)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1264.patch
https://github.com/rpm-software-management/rpm/pull/1264.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1264
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint