pmempool_rm not thread safe

[Daniel Waddington]

Hi,

We've had issues around calling pmempool_rm concurrently from multiple threads.  It appears not to be thread safe.  Is this correct, an oversight or intentional?

Daniel

[Piotr Balcer]

Hi Daniel,

All of PMDK APIs are thread-safe unless otherwise noted in the man page for the particular function.

In the case of pmempool_rm, the function should be thread-safe (in the same way as unlink is threadsafe). If it's not then that's a bug that needs fixing ;)

FYI: For device-dax, `pmempool_rm` simply memsets the first 2 megabytes of the device.

Can you elaborate on the issues you are observing?

Piotr

[Daniel Waddington]

Looking further, we think it is pmemobj_open, not pmempool_rm, which causes the trouble. And for whatever reason, our code requires that a pool be opened before it can be removed.

pmemobj_open calls util_map_hdr to map the file header. A comment in that function reads:

         * According to mmap(2), system should automatically align mapping
         * length to be a multiple of the underlying page size, but it's
         * not true for Device DAX.

So the work-around for DAX is to call util_map_hint to find a suitable address "hint," then call mmap with that hint. But if two calls to util_map_hdr interleave (or if another process changes the page tables during execution of util_map_hint), the call to mmap may fail, or at least may map to an improperly-aligned address. In other words, the read/update of the page map attempted by util_map_hint is not transactional, and needs to be in order to avoid returning spurious failure indications.

Daniel

[Piotr Balcer]

That makes more sense. There are only 3 non-thread-safe functions in libpmemobj, and these are pmemobj_open, pmemobj_close, pmemobj_create. See `man 3 pmemobj_open` for more information.

We intend on addressing that in 1.6 release.

Pool mapping does not rely on the hint address actually being respected by the kernel, we are also not using MAP_FIXED. It's fine if the actual mapping resides in a different address, the hint is there

to make usage of huge pages more likely.

The actual problem is explained here: https://github.com/pmem/issues/issues/872

Piotr