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));
>
>

Reply via email to