diff mbox

[API-NEXT,11/14] linux-gen: shm: add flag and function to share memory between ODP instances

Message ID 1475836517-60484-12-git-send-email-christophe.milard@linaro.org
State Superseded
Headers show

Commit Message

Christophe Milard Oct. 7, 2016, 10:35 a.m. UTC
Implemented by calling the related functions from _ishm.

Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

---
 platform/linux-generic/odp_shared_memory.c | 38 ++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

-- 
2.7.4

Comments

Maxim Uvarov Oct. 7, 2016, 10:37 a.m. UTC | #1
On 10/07/16 13:35, Christophe Milard wrote:
> Implemented by calling the related functions from _ishm.

>

> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

> ---

>   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'.

> +	/* 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);

> +	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_ ?

> +	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.
> +

> +	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));
Christophe Milard Oct. 7, 2016, 10:58 a.m. UTC | #2
On 7 October 2016 at 12:37, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 10/07/16 13:35, Christophe Milard wrote:

>>

>> Implemented by calling the related functions from _ishm.

>>

>> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

>> ---

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

>

>
diff mbox

Patch

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 */
+
+	/* 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;
+	flgs |= (flags & ODP_SHM_SINGLE_VA) ? _ODP_ISHM_SINGLE_VA : 0;
+	flgs |= (flags & ODP_SHM_LOCK) ? _ODP_ISHM_LOCK : 0;
+
+	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 */
+
+	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));