On 31-Oct-18 5:29 PM, Alejandro Lucero wrote:
During memory initialization calling rte_mem_check_dma_mask
leads to a deadlock because memory_hotplug_lock is locked by a
writer, the current code in execution, and rte_memseg_walk
tries to lock as a reader.
This patch adds safe and unsafe versions for invoking the final
function specifying if the memory_hotplug_lock needs to be
acquired, this is for the safe version, or not, the unsafe one.
PMDs should use the safe version and just internal EAL memory
code should use the unsafe one.
Fixes: 223b7f1d5ef6 ("mem: add function for checking memseg IOVA")
Signed-off-by: Alejandro Lucero <alejandro.luc...@netronome.com>
---
I don't think _safe and _unsafe are good names. _unsafe implies
something might blow up, which isn't the case :) I think following the
naming convention established by other functions (rte_mem_check_dma_mask
and rte_mem_check_dma_mask_thread_unsafe) is better. User/driver code is
only supposed to use rte_mem_check_dma_mask safe version anyway, so
there's no need to differentiate between the two if the other one is
never supposed to be used.
drivers/net/nfp/nfp_net.c | 2 +-
lib/librte_eal/common/eal_common_memory.c | 24 +++++++++++++++---
lib/librte_eal/common/include/rte_memory.h | 29 +++++++++++++++++++---
lib/librte_eal/common/malloc_heap.c | 2 +-
lib/librte_eal/rte_eal_version.map | 3 ++-
5 files changed, 51 insertions(+), 9 deletions(-)
<...>
-/* check memsegs iovas are within a range based on dma mask */
-int __rte_experimental rte_mem_check_dma_mask(uint8_t maskbits);
+/**
+ * * @warning
Here and in other places - same issue with extra star.
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Check memsegs iovas are within a range based on dma mask.
The comments make it seem like the parameter is an actual DMA mask,
rather than DMA mask *width*. In fact, you seem to have tripped yourself
up on that already :)
Suggested rewording:
Check if all currently allocated memory segments are compliant with
supplied DMA address width.
+ *
+ * @param maskbits
+ * Address width to check against.
+ */
+int __rte_experimental rte_mem_check_dma_mask_safe(uint8_t maskbits);
+
+/**
+ * * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Check memsegs iovas are within a range based on dma mask without acquiring
+ * memory_hotplug_lock first.
+ *
+ * This function is just for EAL core memory internal use. Drivers should
+ * use the previous safe one.
This is IMO too detailed. Suggested rewording:
Check if all currently allocated memory segments are compliant with
supplied DMA address width.
@warning This function is not thread-safe and is for internal use only.
+ *
+ * @param maskbits
+ * Address width to check against.
+ */
+int __rte_experimental rte_mem_check_dma_mask_unsafe(uint8_t maskbits);
/**
* * @warning
* @b EXPERIMENTAL: this API may change without prior notice
*
* Set dma mask to use once memory initialization is done.
- * Previous function rte_mem_check_dma_mask can not be used
+ * Previous functions rte_mem_check_dma_mask_safe/unsafe can not be used
* safely until memory has been initialized.
*/
void __rte_experimental rte_mem_set_dma_mask(uint8_t maskbits);
diff --git a/lib/librte_eal/common/malloc_heap.c
b/lib/librte_eal/common/malloc_heap.c
index 711622f19..dd8b983e7 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -335,7 +335,7 @@ alloc_pages_on_heap(struct malloc_heap *heap, uint64_t
pg_sz, size_t elt_size,
* executed. For 2) implies the new memory can not be added.
*/
if (mcfg->dma_maskbits) {
- if (rte_mem_check_dma_mask(mcfg->dma_maskbits)) {
+ if (rte_mem_check_dma_mask_unsafe(mcfg->dma_maskbits)) {
/* Currently this can only happen if IOMMU is enabled
* with RTE_ARCH_X86. It is not safe to use this memory
* so returning an error here.
diff --git a/lib/librte_eal/rte_eal_version.map
b/lib/librte_eal/rte_eal_version.map
index ae24b5c73..f863903b6 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -296,7 +296,8 @@ EXPERIMENTAL {
rte_devargs_remove;
rte_devargs_type_count;
rte_mem_check_dma_mask;
- rte_mem_set_dma_mask;
+ rte_mem_set_dma_mask_safe;
+ rte_mem_set_dma_mask_unsafe;
Again, alphabet :)
rte_eal_cleanup;
rte_fbarray_attach;
rte_fbarray_destroy;
--
Thanks,
Anatoly
