On 7 October 2016 at 12:37, Maxim Uvarov <[email protected]> wrote:
> On 10/07/16 13:35, Christophe Milard wrote:
>>
>> Implemented by calling the related functions from _ishm.
>>
>> Signed-off-by: Christophe Milard <[email protected]>
>> ---
>> platform/linux-generic/odp_shared_memory.c | 38
>> ++++++++++++++++++++++++++----
>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_shared_memory.c
>> b/platform/linux-generic/odp_shared_memory.c
>> index 30ef9d9..5439116 100644
>> --- a/platform/linux-generic/odp_shared_memory.c
>> +++ b/platform/linux-generic/odp_shared_memory.c
>> @@ -24,6 +24,23 @@ static inline odp_shm_t to_handle(uint32_t index)
>> return _odp_cast_scalar(odp_shm_t, index + 1);
>> }
>> +static uint32_t get_ishm_flags(uint32_t flags)
>> +{
>> + int flgs = 0; /* internal ishm flags */
>> +
>
> clang must warn that you return signed in unsigned function.
> Also name is not friendly for reading. Name it just 'f' or 'ret'.
Did not see the warning, but definetively correct =>V2
you'll get "f" in v2. Not sure why it is better, but I don't care =>V2
>
>> + /* set internal ishm flags according to API flags:
>> + * note that both ODP_SHM_PROC and ODP_SHM_EXPORT maps to
>> + * _ODP_ISHM_LINK as in the linux-gen implementation there is
>> + * no difference between exporting to another ODP instance or
>> + * another linux process */
>> + flgs |= (flags & ODP_SHM_PROC) ? _ODP_ISHM_LINK : 0;
>> + flgs |= (flags & ODP_SHM_EXPORT) ? _ODP_ISHM_LINK : 0;
>
> f |= (flags & (ODP_SHM_PROC | ODP_SHM_EXPORT) ? _ODP_ISHM_LINK : 0);
OK: If you want: =>V2
>
>> + flgs |= (flags & ODP_SHM_SINGLE_VA) ? _ODP_ISHM_SINGLE_VA : 0;
>> + flgs |= (flags & ODP_SHM_LOCK) ? _ODP_ISHM_LOCK : 0;
>> +
>
>
> Can duplication of flags be avoided? I.e. go with just ODP_SHM_ ?
No: The _ODP_ISHM flags are all the set of flag supported by ishm:
There is no reason why they would 100% match the API flags:
The driver interface has also his sets of flag: ishm flags will be the
superset of all flags: but we do not want dependency between the 2
interfaces (API and DRV), so I think we should have this flag set
separated.
>
>> + return flgs;
>> +}
>> +
>> int odp_shm_capability(odp_shm_capability_t *capa)
>> {
>> memset(capa, 0, sizeof(odp_shm_capability_t));
>> @@ -41,10 +58,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
>> size, uint64_t align,
>> int block_index;
>> int flgs = 0; /* internal ishm flags */
>> - /* set internal ishm flags according to API flags: */
>> - flgs |= (flags & ODP_SHM_PROC) ? _ODP_ISHM_LINK : 0;
>> - flgs |= (flags & ODP_SHM_SINGLE_VA) ? _ODP_ISHM_SINGLE_VA : 0;
>> - flgs |= (flags & ODP_SHM_LOCK) ? _ODP_ISHM_LOCK : 0;
>> + flgs = get_ishm_flags(flags);
>> block_index = _odp_ishm_reserve(name, size, -1, align, flgs,
>> flags);
>> if (block_index >= 0)
>> @@ -53,6 +67,22 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
>> size, uint64_t align,
>> return ODP_SHM_INVALID;
>> }
>> +odp_shm_t odp_shm_reserve_exported(const char *remote_name,
>> + odp_instance_t odp_inst,
>> + const char *local_name,
>> + uint64_t align, uint32_t flags)
>> +{
>> + int ret;
>> + int flgs = 0; /* internal ishm flags */
>
> no need to set to 0. flgs and flags hard to differ.
correct =>V2
name change to 'i_flgs' (for internal flags) in V2
/Christophe
>
>> +
>> + flgs = get_ishm_flags(flags);
>> +
>> + ret = _odp_ishm_reserve_exported(remote_name, (pid_t)odp_inst,
>> + local_name, align, flgs, flags);
>> +
>> + return to_handle(ret);
>> +}
>> +
>> int odp_shm_free(odp_shm_t shm)
>> {
>> return _odp_ishm_free_by_index(from_handle(shm));
>
>