Skip to content

fix FFT workarea typo leading to memory corruption - #1204

Merged
maleadt merged 1 commit into
JuliaGPU:masterfrom
marius311:workarea
Oct 18, 2021
Merged

maleadt merged 1 commit into
JuliaGPU:masterfrom
marius311:workarea

Conversation

@marius311

@marius311 marius311 commented Oct 16, 2021 •

Copy link
Copy Markdown
Contributor

Guessing this was just a typo? I was getting some memory corruption when using the same plan from different tasks before this fix which was fixed for me with this PR.

@maleadt

maleadt commented Oct 16, 2021

Copy link
Copy Markdown
Member

Yeah, that looks like an typo, weird how it never occurred on CI.

@maleadt maleadt added bugfix This gets something working again. cuda libraries Stuff about CUDA library wrappers. labels Oct 16, 2021
@codecov

codecov Bot commented Oct 16, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #1204 (ccb8511) into master (27c87a6) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1204   +/-   ##
=======================================
  Coverage   80.29%   80.29%           
=======================================
  Files         119      119           
  Lines        8306     8306           
=======================================
  Hits         6669     6669           
  Misses       1637     1637           
Impacted Files Coverage Δ
lib/cufft/fft.jl 86.95% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27c87a6...ccb8511. Read the comment docs.

@marius311

Copy link
Copy Markdown
Contributor Author

Not familiar with the test suite here but if its helpful the MWE I developed to track this down looked something like:

x = cu(rand(128,128,2))
plan = plan_rfft(x, (1,2))
n = norm(plan * x)
@sync @async plan * x # call plan from other task hence screwing up workarea
for i=1:10000
    @test norm(plan * x) ≈ n
end

and it did take alot of iterations for the memory corruption to reliably screw up the result, and even sometimes for some reason I didn't understand if wasn't triggered, so it was fairly "silent". Anyway, might be overkill for the tests here but that's what I did fwiw.

@maleadt

maleadt commented Oct 18, 2021

Copy link
Copy Markdown
Member

Yeah, the test suite takes too long already...

@maleadt
maleadt merged commit a9dd216 into JuliaGPU:master Oct 18, 2021
@marius311
marius311 deleted the workarea branch October 18, 2021 15:34
xaellison pushed a commit to xaellison/CUDA.jl that referenced this pull request Oct 20, 2021
simonbyrne pushed a commit to simonbyrne/CUDA.jl that referenced this pull request Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This gets something working again. cuda libraries Stuff about CUDA library wrappers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants