He Yi(heyi-linaro) replied on github web page:
platform/linux-generic/include/odp_pktio_ops_subsystem.h
@@ -88,10 +89,40 @@ typedef ODP_MODULE_CLASS(pktio_ops) {
/* Maximum size of pktio specific ops data.*/
#define ODP_PKTIO_ODPS_DATA_MAX_SIZE 80000
+/* Pktio ops data pool */
+#define ODP_PKTIO_OPS_DATA_POOL_SIZE 160000
+#define ODP_PKTIO_OPS_DATA_POOL_NAME "ODP_PKTIO_OPS_DATA"
+
/* Extract pktio ops data from pktio entry structure */
#define odp_ops_data(_p, _mod) \
Comment:
Look good no more comments from me to this commit after Honnappa and Josep's,
this is a step forward for the pktio ops data
This commit also reveals how complex in ODP to allocate an arbitrary sized
piece of memory, need to prepare a pool (and guess the largest usage), lookup
this pool in every allocation/free by name, and do the allocation/free after
then.
> He Yi(heyi-linaro) wrote:
> seems no need to add an extra macro here?
>
> ODP_SUBSYSTEM_FOREACH_TEMPLATE(...) is extern
> we can just use to generate a static function use the same macro:
> static ODP_SUBSYSTEM_FOREACH_TEMPLATE(...)
>> nagarahalli wrote
>> Temporarily, roundup the size to cache line size. This way all the memory
>> allocations will be cache line aligned.
>>> nagarahalli wrote
>>> Should be odp_ops_data_alloc(_p, size).
>>>
>>> As per the pkt I/O changes document, _p (pktio_entry_t) is not required to
>>> be exposed to drivers. Do you plan to do it as part of this PR?
>>>> nagarahalli wrote
>>>> Same here, this can be part of odp_pktio_term_global
>>>>> nagarahalli wrote
>>>>> This functionality can be done in odp_pktio_global_init function. This
>>>>> will avoid the changes to modular framework as well.
>>>>>
>>>>> When we have done additional enhancements to shared memory, this code
>>>>> will be deleted. So, can be part of odp_pktio_global_init without
>>>>> affecting the modular framework.
>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>> ah, yes, true, I didn't think about this detail...
>>>>>>> bogdanPricope wrote
>>>>>>> @joseppc Btw, 'odp_ops_data_alloc(_p, _mod)' vs.
>>>>>>> odp_ops_data_alloc(_p, _size) ?
>>>>>>>> bogdanPricope wrote
>>>>>>>> No, this pool is used to allocate packets (for recv side).
>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>> Ok, I may be splitting hairs now, but I thought we were just checking
>>>>>>>>> whether the pool parameter passed to pktio_open was valid, and bail
>>>>>>>>> out if not. We are not actually using this pool to allocate this
>>>>>>>>> pktio's private data, right?
>>>>>>>>>> bogdanPricope wrote
>>>>>>>>>> Alloc/free vs. array has this disadvantage: you need to allocate the
>>>>>>>>>> memory at some point and free it if the operation fails for any
>>>>>>>>>> reason. It is better to delay the allocation until after some common
>>>>>>>>>> checks.
>>>>>>>>>>
>>>>>>>>>> stncmp: open calls are not on fast path ... no reason to optimize
>>>>>>>>>> the performance ... but repeated memory alloc/free may affect some
>>>>>>>>>> pool implementations
>>>>>>>>>>> bogdanPricope wrote
>>>>>>>>>>> It will require a cast when is called.
>>>>>>>>>>> A pktio_type may implement another way to allocate memory starting
>>>>>>>>>>> form the name of the pool / is not mandatory to use those macros
>>>>>>>>>>> but are helpful for existing pktio types.
>>>>>>>>>>>> bogdanPricope wrote
>>>>>>>>>>>> _p comes form (pktio_entry_t *)
>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>> It probably belongs to its own patch, but now that you are at it,
>>>>>>>>>>>>> it could even be moved even further up, as it is probably faster
>>>>>>>>>>>>> than checking for "tap:" in the device string.
>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>> (_p)? There are a couple more, also in odp_ops_data_free.
>>>>>>>>>>>>>>> Josep Puigdemont(joseppc) wrote:
>>>>>>>>>>>>>>> Maybe we can return (void *)? This way we would not care if
>>>>>>>>>>>>>>> pktios name (or define) their private structures according to
>>>>>>>>>>>>>>> the naming conventions implicit in the macro.
https://github.com/Linaro/odp/pull/297#discussion_r151591533
updated_at 2017-11-17 16:48:29
