On 2024-10-14 10:17, Morten Brørup wrote:
From: Mattias Rönnblom [mailto:mattias.ronnb...@ericsson.com]
Sent: Monday, 14 October 2024 09.44


+struct lcore_var_buffer {
+       char data[RTE_MAX_LCORE_VAR * RTE_MAX_LCORE];
+       struct lcore_var_buffer *prev;
+};

In relation to Jerin's request for using hugepages when available, the "data" 
field should be a pointer to the memory allocated from either the heap or through 
rte_malloc. You would also need to add a flag to indicate which it is, so the correct 
deallocation function can be used to free it on cleanup.


The typing (glibc heap or DPDK heap) could be in the buffers themselves, no?

<feature creep>
Here's another (nice to have) idea, which does not need to be part of this 
series, but can be implemented in a separate patch:
If you move "offset" into this structure, new lcore variables can be allocated 
from any buffer, instead of only the most recently allocated buffer.
There might even be gains by picking the "optimal" buffer to allocate different 
size variables from.
</feature creep>


If the max lcore variable size is much greater than the actual variable sizes, the amount of fragmentation (i.e., the space at the end) will be very small.

I don't think we should use huge pages for this facility, since they don't support demand paging.

The day we have a DPDK heap which support lcore-affinitized allocations, then potentially eal_common_lcore_var.c could use that, provided it's available (and there is a proper way to check [or get notified] if it is available or not).

+
+static struct lcore_var_buffer *current_buffer;
+
+/* initialized to trigger buffer allocation on first allocation */
+static size_t offset = RTE_MAX_LCORE_VAR;


+void *
+rte_lcore_var_alloc(size_t size, size_t align)
+{
+       /* Having the per-lcore buffer size aligned on cache lines
+        * assures as well as having the base pointer aligned on cache
+        * size assures that aligned offsets also translate to alipgned
+        * pointers across all values.
+        */
+       RTE_BUILD_BUG_ON(RTE_MAX_LCORE_VAR % RTE_CACHE_LINE_SIZE != 0);
+       RTE_ASSERT(align <= RTE_CACHE_LINE_SIZE);
+       RTE_ASSERT(size <= RTE_MAX_LCORE_VAR);

This is very slow path, please RTE_VERIFY instead of RTE_ASSERT in this 
function.


Sure. (I think I rejected that before, but now I don't agree with my old self.)


+/**
+ * Get pointer to lcore variable instance with the specified lcore id.
+ *
+ * @param lcore_id
+ *   The lcore id specifying which of the @c RTE_MAX_LCORE value
+ *   instances should be accessed. The lcore id need not be valid
+ *   (e.g., may be @ref LCORE_ID_ANY), but in such a case, the pointer
+ *   is also not valid (and thus should not be dereferenced).
+ * @param handle
+ *   The lcore variable handle.
+ */
+#define RTE_LCORE_VAR_LCORE_VALUE(lcore_id, handle)                    \
+       ((typeof(handle))rte_lcore_var_lcore_ptr(lcore_id, handle))

Please remove the _VALUE suffix.


You changed your mind? I'm missing the rationale here.

+
+/**
+ * Get pointer to lcore variable instance of the current thread.
+ *
+ * May only be used by EAL threads and registered non-EAL threads.
+ */
+#define RTE_LCORE_VAR_VALUE(handle) \
+       RTE_LCORE_VAR_LCORE_VALUE(rte_lcore_id(), handle)

Please remove the _VALUE suffix.

+
+/**
+ * Iterate over each lcore id's value for an lcore variable.
+ *
+ * @param lcore_id
+ *   An <code>unsigned int</code> variable successively set to the
+ *   lcore id of every valid lcore id (up to @c RTE_MAX_LCORE).
+ * @param value
+ *   A pointer variable successively set to point to lcore variable
+ *   value instance of the current lcore id being processed.
+ * @param handle
+ *   The lcore variable handle.
+ */
+#define RTE_LCORE_VAR_FOREACH_VALUE(lcore_id, value, handle)

Please remove the _VALUE suffix.

        \
+       for ((lcore_id) =                                               \
+                    (((value) = RTE_LCORE_VAR_LCORE_VALUE(0, handle)), 0);
\
+            (lcore_id) < RTE_MAX_LCORE;                             \
+            (lcore_id)++, (value) = RTE_LCORE_VAR_LCORE_VALUE(lcore_id,
\


Reply via email to