Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Add memfence before unsetting bit #283

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/mallocMC/creationPolicies/FlatterScatter/AccessBlock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,16 @@ namespace mallocMC::CreationPolicies::FlatterScatterAlloc
template<typename TAcc>
ALPAKA_FN_INLINE ALPAKA_FN_ACC auto destroy(TAcc const& acc, void* const pointer) -> void
{
// CAUTION: This memfence is of utmost importance! As we are allowing a re-use of the chunk we're about to
// free, we need to make sure that any memory operation from the previous thread is executed before we can
// safely consider it free. If this is missing, an extended (non-atomic) write operation might not yet have
// finished when we unset the bit. In such a case, another thread might start using the memory while we're
// still writing to it, thus corrupting the new thread's data. It might even lead to us overwriting the
// bitmask itself, if the chunk size (and thereby the extent of the bitmask) changes before we finish.
// (The latter scenario might be excluded by other mem_fences in the code.) If a read is pending, the old
// thread might read data from the new thread leading to inconsistent information in the first thread.
alpaka::mem_fence(acc, alpaka::memory_scope::Device{});

auto const index = pageIndex(pointer);
if(index >= static_cast<int32_t>(numPages()) || index < 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ namespace mallocMC::CreationPolicies::FlatterScatterAlloc
"to a valid chunk or it is not marked as allocated."};
}
#endif // NDEBUG

bitField().unset(acc, chunkIndex);
}

Expand Down
12 changes: 12 additions & 0 deletions include/mallocMC/creationPolicies/Scatter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,18 @@ namespace mallocMC
{
if(mem == 0)
return;

// CAUTION: This memfence is of utmost importance! As we are allowing a re-use of the chunk we're about
// to free, we need to make sure that any memory operation from the previous thread is executed before
// we can safely consider it free. If this is missing, an extended (non-atomic) write operation might
// not yet have finished when we unset the bit. In such a case, another thread might start using the
// memory while we're still writing to it, thus corrupting the new thread's data. It might even lead to
// us overwriting the bitmask itself, if the chunk size (and thereby the extent of the bitmask) changes
// before we finish. (The latter scenario might be excluded by other mem_fences in the code.) If a read
// is pending, the old thread might read data from the new thread leading to inconsistent information
// in the first thread.
alpaka::mem_fence(acc, alpaka::memory_scope::Device{});

// lets see on which page we are on
auto const page = static_cast<uint32>(((char*) mem - (char*) _page) / pagesize);
/* Emulate atomic read.
Expand Down