diff mbox

odp shm shared between processes

Message ID 1406636959-30239-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov July 29, 2014, 12:29 p.m. UTC
Add flag to odp_shm_reserve() to select if allocation memory is
visible by thread or it's visible by other processes.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 Proposed API change allows to allocate memory chunks like
 buffer pools and queue specific things in memory visible
 by other processes.

 BR,
 Maxim.

 example/generator/odp_generator.c          |  6 ++++--
 example/l2fwd/odp_l2fwd.c                  |  6 ++++--
 example/odp_example/odp_example.c          |  3 ++-
 example/packet/odp_pktio.c                 |  6 ++++--
 example/timer/odp_timer_test.c             |  3 ++-
 include/odp_shared_memory.h                | 11 +++++++++-
 platform/linux-generic/odp_buffer_pool.c   |  3 ++-
 platform/linux-generic/odp_packet_io.c     |  3 ++-
 platform/linux-generic/odp_queue.c         |  3 ++-
 platform/linux-generic/odp_ring.c          |  3 ++-
 platform/linux-generic/odp_schedule.c      |  6 ++++--
 platform/linux-generic/odp_shared_memory.c | 33 +++++++++++++++++++++++++++---
 test/api_test/odp_shm_test.c               |  3 ++-
 test/api_test/odp_timer_ping.c             |  3 ++-
 14 files changed, 72 insertions(+), 20 deletions(-)

Comments

Anders Roxell July 30, 2014, 7:53 p.m. UTC | #1
Hi, I think the concept is OK.
However, there are some coding style changes that has to be made.

See inline below.

On 2014-07-29 16:29, Maxim Uvarov wrote:
> Add flag to odp_shm_reserve() to select if allocation memory is
> visible by thread or it's visible by other processes.

Nit: "if allocated memory"

> 
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  Proposed API change allows to allocate memory chunks like
>  buffer pools and queue specific things in memory visible
>  by other processes.
> 
>  BR,
>  Maxim.
> 
>  example/generator/odp_generator.c          |  6 ++++--
>  example/l2fwd/odp_l2fwd.c                  |  6 ++++--
>  example/odp_example/odp_example.c          |  3 ++-
>  example/packet/odp_pktio.c                 |  6 ++++--
>  example/timer/odp_timer_test.c             |  3 ++-
>  include/odp_shared_memory.h                | 11 +++++++++-
>  platform/linux-generic/odp_buffer_pool.c   |  3 ++-
>  platform/linux-generic/odp_packet_io.c     |  3 ++-
>  platform/linux-generic/odp_queue.c         |  3 ++-
>  platform/linux-generic/odp_ring.c          |  3 ++-
>  platform/linux-generic/odp_schedule.c      |  6 ++++--
>  platform/linux-generic/odp_shared_memory.c | 33 +++++++++++++++++++++++++++---
>  test/api_test/odp_shm_test.c               |  3 ++-
>  test/api_test/odp_timer_ping.c             |  3 ++-
>  14 files changed, 72 insertions(+), 20 deletions(-)
> 
> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
> index e4a72fa..b1be789 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -542,7 +542,8 @@ int main(int argc, char *argv[])
>  	odp_atomic_init_u64(&counters.icmp);
>  
>  	/* Reserve memory for args from shared mem */
> -	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE);
> +	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE,
> +				ODP_SHM_THREAD);
>  	if (args == NULL) {
>  		ODP_ERR("Error: shared mem alloc failed.\n");
>  		exit(EXIT_FAILURE);
> @@ -587,7 +588,8 @@ int main(int argc, char *argv[])
>  
>  	/* Create packet pool */
>  	pool_base = odp_shm_reserve("shm_packet_pool",
> -				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +				    ODP_SHM_THREAD);
>  	if (pool_base == NULL) {
>  		ODP_ERR("Error: packet pool mem alloc failed.\n");
>  		exit(EXIT_FAILURE);
> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
> index e331ff2..c5f3725 100644
> --- a/example/l2fwd/odp_l2fwd.c
> +++ b/example/l2fwd/odp_l2fwd.c
> @@ -294,7 +294,8 @@ int main(int argc, char *argv[])
>  	}
>  
>  	/* Reserve memory for args from shared mem */
> -	gbl_args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE);
> +	gbl_args = odp_shm_reserve("shm_args", sizeof(args_t),
> +				 ODP_CACHE_LINE_SIZE, ODP_SHM_THREAD);
>  	if (gbl_args == NULL) {
>  		ODP_ERR("Error: shared mem alloc failed.\n");
>  		exit(EXIT_FAILURE);
> @@ -345,7 +346,8 @@ int main(int argc, char *argv[])
>  
>  	/* Create packet pool */
>  	pool_base = odp_shm_reserve("shm_packet_pool",
> -				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +				    ODP_SHM_THREAD);
>  	if (pool_base == NULL) {
>  		ODP_ERR("Error: packet pool mem alloc failed.\n");
>  		exit(EXIT_FAILURE);
> diff --git a/example/odp_example/odp_example.c b/example/odp_example/odp_example.c
> index be96093..6d075b2 100644
> --- a/example/odp_example/odp_example.c
> +++ b/example/odp_example/odp_example.c
> @@ -987,7 +987,8 @@ int main(int argc, char *argv[])
>  	 * Create message pool
>  	 */
>  	pool_base = odp_shm_reserve("msg_pool",
> -				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +				    ODP_SHM_THREAD);
>  
>  	pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
>  				      sizeof(test_message_t),
> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
> index edf8cfd..f5694c3 100644
> --- a/example/packet/odp_pktio.c
> +++ b/example/packet/odp_pktio.c
> @@ -291,7 +291,8 @@ int main(int argc, char *argv[])
>  	}
>  
>  	/* Reserve memory for args from shared mem */
> -	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE);
> +	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE,
> +				ODP_SHM_THREAD);
>  	if (args == NULL) {
>  		ODP_ERR("Error: shared mem alloc failed.\n");
>  		exit(EXIT_FAILURE);
> @@ -332,7 +333,8 @@ int main(int argc, char *argv[])
>  
>  	/* Create packet pool */
>  	pool_base = odp_shm_reserve("shm_packet_pool",
> -				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +				    ODP_SHM_THREAD);
>  	if (pool_base == NULL) {
>  		ODP_ERR("Error: packet pool mem alloc failed.\n");
>  		exit(EXIT_FAILURE);
> diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c
> index dbe0e5b..113200b 100644
> --- a/example/timer/odp_timer_test.c
> +++ b/example/timer/odp_timer_test.c
> @@ -260,7 +260,8 @@ int main(int argc, char *argv[])
>  	 * Create message pool
>  	 */
>  	pool_base = odp_shm_reserve("msg_pool",
> -				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +				    ODP_SHM_THREAD);
>  
>  	pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
>  				      0,
> diff --git a/include/odp_shared_memory.h b/include/odp_shared_memory.h
> index 8ac8847..10149ad 100644
> --- a/include/odp_shared_memory.h
> +++ b/include/odp_shared_memory.h
> @@ -24,6 +24,13 @@ extern "C" {
>  /** Maximum shared memory block name lenght in chars */
>  #define ODP_SHM_NAME_LEN 32
>  
> +typedef enum {
> +	ODP_SHM_THREAD = 0,    /**< Memory accessible by threads.  */

remove "= 0", starts on index 0 per default.

> +	ODP_SHM_PROC,	       /**< Memory accessible by processes.

Remove a tab after "," before "/**<"

> +				    Will be created if not exist.  */
> +	ODP_SHM_PROC_NOCREAT,  /**< Memory accessible by processes.
> +				    Has to be created before usage.*/
> +} odp_shm_e;
>  
>  /**
>   * Reserve a block of shared memory
> @@ -31,10 +38,12 @@ extern "C" {
>   * @param name   Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars)
>   * @param size   Block size in bytes
>   * @param align  Block alignment in bytes
> + * @param flags  Flags for shared memory creation, ODP_SHM_THREAD or ODP_SHM_PROC

flag not flags.

>   *
>   * @return Pointer to the reserved block, or NULL
>   */
> -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align);
> +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
> +		      odp_shm_e flags);

flag not flags.

>  
>  /**
>   * Lookup for a block of shared memory
> diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c
> index a48781a..9157994 100644
> --- a/platform/linux-generic/odp_buffer_pool.c
> +++ b/platform/linux-generic/odp_buffer_pool.c
> @@ -102,7 +102,8 @@ int odp_buffer_pool_init_global(void)
>  
>  	pool_tbl = odp_shm_reserve("odp_buffer_pools",
>  				   sizeof(pool_table_t),
> -				   sizeof(pool_entry_t));
> +				   sizeof(pool_entry_t),
> +				   ODP_SHM_THREAD);
>  
>  	if (pool_tbl == NULL)
>  		return -1;
> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
> index 33ade10..050ade4 100644
> --- a/platform/linux-generic/odp_packet_io.c
> +++ b/platform/linux-generic/odp_packet_io.c
> @@ -55,7 +55,8 @@ int odp_pktio_init_global(void)
>  
>  	pktio_tbl = odp_shm_reserve("odp_pktio_entries",
>  				    sizeof(pktio_table_t),
> -				    sizeof(pktio_entry_t));
> +				    sizeof(pktio_entry_t),
> +				    ODP_SHM_THREAD);
>  	if (pktio_tbl == NULL)
>  		return -1;
>  
> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c
> index c637bdf..507331f 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -99,7 +99,8 @@ int odp_queue_init_global(void)
>  
>  	queue_tbl = odp_shm_reserve("odp_queues",
>  				    sizeof(queue_table_t),
> -				    sizeof(queue_entry_t));
> +				    sizeof(queue_entry_t),
> +				    ODP_SHM_THREAD);
>  
>  	if (queue_tbl == NULL)
>  		return -1;
> diff --git a/platform/linux-generic/odp_ring.c b/platform/linux-generic/odp_ring.c
> index 25ff66a..ee8175e 100644
> --- a/platform/linux-generic/odp_ring.c
> +++ b/platform/linux-generic/odp_ring.c
> @@ -171,7 +171,8 @@ odp_ring_create(const char *name, unsigned count, unsigned flags)
>  
>  	odp_rwlock_write_lock(&qlock);
>  	/* reserve a memory zone for this ring.*/
> -	r = odp_shm_reserve(ring_name, ring_size, ODP_CACHE_LINE_SIZE);
> +	r = odp_shm_reserve(ring_name, ring_size, ODP_CACHE_LINE_SIZE,
> +			    ODP_SHM_THREAD);
>  
>  	if (r != NULL) {
>  		/* init the ring structure */
> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
> index 9e399f1..722e297 100644
> --- a/platform/linux-generic/odp_schedule.c
> +++ b/platform/linux-generic/odp_schedule.c
> @@ -89,7 +89,8 @@ int odp_schedule_init_global(void)
>  
>  	sched = odp_shm_reserve("odp_scheduler",
>  				sizeof(sched_t),
> -				ODP_CACHE_LINE_SIZE);
> +				ODP_CACHE_LINE_SIZE,
> +				ODP_SHM_THREAD);
>  
>  	if (sched == NULL) {
>  		ODP_ERR("Schedule init: Shm reserve failed.\n");
> @@ -98,7 +99,8 @@ int odp_schedule_init_global(void)
>  
>  
>  	pool_base = odp_shm_reserve("odp_sched_pool",
> -				    SCHED_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +			SCHED_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +			ODP_SHM_THREAD);

fix indentation

>  
>  	pool = odp_buffer_pool_create("odp_sched_pool", pool_base,
>  				      SCHED_POOL_SIZE, sizeof(queue_desc_t),
> diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c
> index 784f42b..ed94513 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -14,6 +14,8 @@
>  #include <sys/mman.h>
>  #include <asm/mman.h>
>  #include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/types.h>
>  
>  #include <stdio.h>
>  #include <string.h>
> @@ -95,11 +97,16 @@ static int find_block(const char *name)
>  }
>  
>  
> -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
> +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
> +		      odp_shm_e flags)
>  {
>  	int i;
> +	int shm = -1;
> +	int ret;
>  	odp_shm_block_t *block;
>  	void *addr;
> +	int shm_open_flags;
> +	int mmap_flags = MAP_SHARED;

Can't we put all int variables on the same line?

>  #ifdef MAP_HUGETLB
>  	uint64_t huge_sz, page_sz;
>  
> @@ -133,18 +140,38 @@ void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
>  	addr        = MAP_FAILED;
>  	block->huge = 0;
>  
> +	if (flags) {

flag not flags.

> +		shm_open_flags = O_RDWR;
> +		if (flags == ODP_SHM_PROC)
> +			shm_open_flags |= O_CREAT;
> +
> +		shm = shm_open(name, shm_open_flags, S_IRUSR | S_IWUSR);
> +		if (shm == -1) {
> +			ODP_ERR("shm_open failed");
> +			return  NULL;

Remove a space after "return".

> +		}
> +
> +		ret = ftruncate(shm, size + align);
> +		if (ret == -1) {
> +			ODP_ERR("ftruncate failed");
> +			return NULL;
> +		}
> +	} else {
> +		mmap_flags |= MAP_ANONYMOUS;
> +	}
> +
>  #ifdef MAP_HUGETLB
>  	/* Try first huge pages */
>  	if (huge_sz && (size + align) > page_sz) {
>  		addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
> -			    SHM_FLAGS | MAP_HUGETLB, -1, 0);
> +			mmap_flags | MAP_HUGETLB, shm, 0);

Fix indentation.

>  	}
>  #endif
>  
>  	/* Use normal pages for small or failed huge page allocations */
>  	if (addr == MAP_FAILED) {
>  		addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
> -			    SHM_FLAGS, -1, 0);
> +			mmap_flags, shm, 0);

Fix indentation.

>  
>  	} else {
>  		block->huge = 1;
> diff --git a/test/api_test/odp_shm_test.c b/test/api_test/odp_shm_test.c
> index 318d662..194b4f7 100644
> --- a/test/api_test/odp_shm_test.c
> +++ b/test/api_test/odp_shm_test.c
> @@ -47,7 +47,8 @@ int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
>  	odp_print_system_info();
>  
>  	test_shared_data = odp_shm_reserve("test_shared_data",
> -					   sizeof(test_shared_data_t), 128);
> +			sizeof(test_shared_data_t), 128,
> +			ODP_SHM_THREAD);

Fix indentation.

>  	memset(test_shared_data, 0, sizeof(test_shared_data_t));
>  	printf("test shared data at %p\n\n", test_shared_data);
>  
> diff --git a/test/api_test/odp_timer_ping.c b/test/api_test/odp_timer_ping.c
> index cd67e0d..c31a850 100644
> --- a/test/api_test/odp_timer_ping.c
> +++ b/test/api_test/odp_timer_ping.c
> @@ -324,7 +324,8 @@ int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
>  	 * Create message pool
>  	 */
>  	pool_base = odp_shm_reserve("msg_pool",
> -				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> +					MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> +					ODP_SHM_THREAD);

Fix indentation.

Cheers,
Anders
Mike Holmes July 30, 2014, 8:10 p.m. UTC | #2
I think variables are much better on separate lines, especially if they are
assigned a value,  but now we ignite a style war. :)
On Jul 30, 2014 3:54 PM, "Anders Roxell" <anders.roxell@linaro.org> wrote:

> Hi, I think the concept is OK.
> However, there are some coding style changes that has to be made.
>
> See inline below.
>
> On 2014-07-29 16:29, Maxim Uvarov wrote:
> > Add flag to odp_shm_reserve() to select if allocation memory is
> > visible by thread or it's visible by other processes.
>
> Nit: "if allocated memory"
>
> >
> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > ---
> >  Proposed API change allows to allocate memory chunks like
> >  buffer pools and queue specific things in memory visible
> >  by other processes.
> >
> >  BR,
> >  Maxim.
> >
> >  example/generator/odp_generator.c          |  6 ++++--
> >  example/l2fwd/odp_l2fwd.c                  |  6 ++++--
> >  example/odp_example/odp_example.c          |  3 ++-
> >  example/packet/odp_pktio.c                 |  6 ++++--
> >  example/timer/odp_timer_test.c             |  3 ++-
> >  include/odp_shared_memory.h                | 11 +++++++++-
> >  platform/linux-generic/odp_buffer_pool.c   |  3 ++-
> >  platform/linux-generic/odp_packet_io.c     |  3 ++-
> >  platform/linux-generic/odp_queue.c         |  3 ++-
> >  platform/linux-generic/odp_ring.c          |  3 ++-
> >  platform/linux-generic/odp_schedule.c      |  6 ++++--
> >  platform/linux-generic/odp_shared_memory.c | 33
> +++++++++++++++++++++++++++---
> >  test/api_test/odp_shm_test.c               |  3 ++-
> >  test/api_test/odp_timer_ping.c             |  3 ++-
> >  14 files changed, 72 insertions(+), 20 deletions(-)
> >
> > diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> > index e4a72fa..b1be789 100644
> > --- a/example/generator/odp_generator.c
> > +++ b/example/generator/odp_generator.c
> > @@ -542,7 +542,8 @@ int main(int argc, char *argv[])
> >       odp_atomic_init_u64(&counters.icmp);
> >
> >       /* Reserve memory for args from shared mem */
> > -     args = odp_shm_reserve("shm_args", sizeof(args_t),
> ODP_CACHE_LINE_SIZE);
> > +     args = odp_shm_reserve("shm_args", sizeof(args_t),
> ODP_CACHE_LINE_SIZE,
> > +                             ODP_SHM_THREAD);
> >       if (args == NULL) {
> >               ODP_ERR("Error: shared mem alloc failed.\n");
> >               exit(EXIT_FAILURE);
> > @@ -587,7 +588,8 @@ int main(int argc, char *argv[])
> >
> >       /* Create packet pool */
> >       pool_base = odp_shm_reserve("shm_packet_pool",
> > -                                 SHM_PKT_POOL_SIZE,
> ODP_CACHE_LINE_SIZE);
> > +                                 SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                                 ODP_SHM_THREAD);
> >       if (pool_base == NULL) {
> >               ODP_ERR("Error: packet pool mem alloc failed.\n");
> >               exit(EXIT_FAILURE);
> > diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
> > index e331ff2..c5f3725 100644
> > --- a/example/l2fwd/odp_l2fwd.c
> > +++ b/example/l2fwd/odp_l2fwd.c
> > @@ -294,7 +294,8 @@ int main(int argc, char *argv[])
> >       }
> >
> >       /* Reserve memory for args from shared mem */
> > -     gbl_args = odp_shm_reserve("shm_args", sizeof(args_t),
> ODP_CACHE_LINE_SIZE);
> > +     gbl_args = odp_shm_reserve("shm_args", sizeof(args_t),
> > +                              ODP_CACHE_LINE_SIZE, ODP_SHM_THREAD);
> >       if (gbl_args == NULL) {
> >               ODP_ERR("Error: shared mem alloc failed.\n");
> >               exit(EXIT_FAILURE);
> > @@ -345,7 +346,8 @@ int main(int argc, char *argv[])
> >
> >       /* Create packet pool */
> >       pool_base = odp_shm_reserve("shm_packet_pool",
> > -                                 SHM_PKT_POOL_SIZE,
> ODP_CACHE_LINE_SIZE);
> > +                                 SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                                 ODP_SHM_THREAD);
> >       if (pool_base == NULL) {
> >               ODP_ERR("Error: packet pool mem alloc failed.\n");
> >               exit(EXIT_FAILURE);
> > diff --git a/example/odp_example/odp_example.c
> b/example/odp_example/odp_example.c
> > index be96093..6d075b2 100644
> > --- a/example/odp_example/odp_example.c
> > +++ b/example/odp_example/odp_example.c
> > @@ -987,7 +987,8 @@ int main(int argc, char *argv[])
> >        * Create message pool
> >        */
> >       pool_base = odp_shm_reserve("msg_pool",
> > -                                 MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> > +                                 MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                                 ODP_SHM_THREAD);
> >
> >       pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
> >                                     sizeof(test_message_t),
> > diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
> > index edf8cfd..f5694c3 100644
> > --- a/example/packet/odp_pktio.c
> > +++ b/example/packet/odp_pktio.c
> > @@ -291,7 +291,8 @@ int main(int argc, char *argv[])
> >       }
> >
> >       /* Reserve memory for args from shared mem */
> > -     args = odp_shm_reserve("shm_args", sizeof(args_t),
> ODP_CACHE_LINE_SIZE);
> > +     args = odp_shm_reserve("shm_args", sizeof(args_t),
> ODP_CACHE_LINE_SIZE,
> > +                             ODP_SHM_THREAD);
> >       if (args == NULL) {
> >               ODP_ERR("Error: shared mem alloc failed.\n");
> >               exit(EXIT_FAILURE);
> > @@ -332,7 +333,8 @@ int main(int argc, char *argv[])
> >
> >       /* Create packet pool */
> >       pool_base = odp_shm_reserve("shm_packet_pool",
> > -                                 SHM_PKT_POOL_SIZE,
> ODP_CACHE_LINE_SIZE);
> > +                                 SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                                 ODP_SHM_THREAD);
> >       if (pool_base == NULL) {
> >               ODP_ERR("Error: packet pool mem alloc failed.\n");
> >               exit(EXIT_FAILURE);
> > diff --git a/example/timer/odp_timer_test.c
> b/example/timer/odp_timer_test.c
> > index dbe0e5b..113200b 100644
> > --- a/example/timer/odp_timer_test.c
> > +++ b/example/timer/odp_timer_test.c
> > @@ -260,7 +260,8 @@ int main(int argc, char *argv[])
> >        * Create message pool
> >        */
> >       pool_base = odp_shm_reserve("msg_pool",
> > -                                 MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> > +                                 MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                                 ODP_SHM_THREAD);
> >
> >       pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
> >                                     0,
> > diff --git a/include/odp_shared_memory.h b/include/odp_shared_memory.h
> > index 8ac8847..10149ad 100644
> > --- a/include/odp_shared_memory.h
> > +++ b/include/odp_shared_memory.h
> > @@ -24,6 +24,13 @@ extern "C" {
> >  /** Maximum shared memory block name lenght in chars */
> >  #define ODP_SHM_NAME_LEN 32
> >
> > +typedef enum {
> > +     ODP_SHM_THREAD = 0,    /**< Memory accessible by threads.  */
>
> remove "= 0", starts on index 0 per default.
>
> > +     ODP_SHM_PROC,          /**< Memory accessible by processes.
>
> Remove a tab after "," before "/**<"
>
> > +                                 Will be created if not exist.  */
> > +     ODP_SHM_PROC_NOCREAT,  /**< Memory accessible by processes.
> > +                                 Has to be created before usage.*/
> > +} odp_shm_e;
> >
> >  /**
> >   * Reserve a block of shared memory
> > @@ -31,10 +38,12 @@ extern "C" {
> >   * @param name   Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars)
> >   * @param size   Block size in bytes
> >   * @param align  Block alignment in bytes
> > + * @param flags  Flags for shared memory creation, ODP_SHM_THREAD or
> ODP_SHM_PROC
>
> flag not flags.
>
> >   *
> >   * @return Pointer to the reserved block, or NULL
> >   */
> > -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align);
> > +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
> > +                   odp_shm_e flags);
>
> flag not flags.
>
> >
> >  /**
> >   * Lookup for a block of shared memory
> > diff --git a/platform/linux-generic/odp_buffer_pool.c
> b/platform/linux-generic/odp_buffer_pool.c
> > index a48781a..9157994 100644
> > --- a/platform/linux-generic/odp_buffer_pool.c
> > +++ b/platform/linux-generic/odp_buffer_pool.c
> > @@ -102,7 +102,8 @@ int odp_buffer_pool_init_global(void)
> >
> >       pool_tbl = odp_shm_reserve("odp_buffer_pools",
> >                                  sizeof(pool_table_t),
> > -                                sizeof(pool_entry_t));
> > +                                sizeof(pool_entry_t),
> > +                                ODP_SHM_THREAD);
> >
> >       if (pool_tbl == NULL)
> >               return -1;
> > diff --git a/platform/linux-generic/odp_packet_io.c
> b/platform/linux-generic/odp_packet_io.c
> > index 33ade10..050ade4 100644
> > --- a/platform/linux-generic/odp_packet_io.c
> > +++ b/platform/linux-generic/odp_packet_io.c
> > @@ -55,7 +55,8 @@ int odp_pktio_init_global(void)
> >
> >       pktio_tbl = odp_shm_reserve("odp_pktio_entries",
> >                                   sizeof(pktio_table_t),
> > -                                 sizeof(pktio_entry_t));
> > +                                 sizeof(pktio_entry_t),
> > +                                 ODP_SHM_THREAD);
> >       if (pktio_tbl == NULL)
> >               return -1;
> >
> > diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> > index c637bdf..507331f 100644
> > --- a/platform/linux-generic/odp_queue.c
> > +++ b/platform/linux-generic/odp_queue.c
> > @@ -99,7 +99,8 @@ int odp_queue_init_global(void)
> >
> >       queue_tbl = odp_shm_reserve("odp_queues",
> >                                   sizeof(queue_table_t),
> > -                                 sizeof(queue_entry_t));
> > +                                 sizeof(queue_entry_t),
> > +                                 ODP_SHM_THREAD);
> >
> >       if (queue_tbl == NULL)
> >               return -1;
> > diff --git a/platform/linux-generic/odp_ring.c
> b/platform/linux-generic/odp_ring.c
> > index 25ff66a..ee8175e 100644
> > --- a/platform/linux-generic/odp_ring.c
> > +++ b/platform/linux-generic/odp_ring.c
> > @@ -171,7 +171,8 @@ odp_ring_create(const char *name, unsigned count,
> unsigned flags)
> >
> >       odp_rwlock_write_lock(&qlock);
> >       /* reserve a memory zone for this ring.*/
> > -     r = odp_shm_reserve(ring_name, ring_size, ODP_CACHE_LINE_SIZE);
> > +     r = odp_shm_reserve(ring_name, ring_size, ODP_CACHE_LINE_SIZE,
> > +                         ODP_SHM_THREAD);
> >
> >       if (r != NULL) {
> >               /* init the ring structure */
> > diff --git a/platform/linux-generic/odp_schedule.c
> b/platform/linux-generic/odp_schedule.c
> > index 9e399f1..722e297 100644
> > --- a/platform/linux-generic/odp_schedule.c
> > +++ b/platform/linux-generic/odp_schedule.c
> > @@ -89,7 +89,8 @@ int odp_schedule_init_global(void)
> >
> >       sched = odp_shm_reserve("odp_scheduler",
> >                               sizeof(sched_t),
> > -                             ODP_CACHE_LINE_SIZE);
> > +                             ODP_CACHE_LINE_SIZE,
> > +                             ODP_SHM_THREAD);
> >
> >       if (sched == NULL) {
> >               ODP_ERR("Schedule init: Shm reserve failed.\n");
> > @@ -98,7 +99,8 @@ int odp_schedule_init_global(void)
> >
> >
> >       pool_base = odp_shm_reserve("odp_sched_pool",
> > -                                 SCHED_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> > +                     SCHED_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                     ODP_SHM_THREAD);
>
> fix indentation
>
> >
> >       pool = odp_buffer_pool_create("odp_sched_pool", pool_base,
> >                                     SCHED_POOL_SIZE,
> sizeof(queue_desc_t),
> > diff --git a/platform/linux-generic/odp_shared_memory.c
> b/platform/linux-generic/odp_shared_memory.c
> > index 784f42b..ed94513 100644
> > --- a/platform/linux-generic/odp_shared_memory.c
> > +++ b/platform/linux-generic/odp_shared_memory.c
> > @@ -14,6 +14,8 @@
> >  #include <sys/mman.h>
> >  #include <asm/mman.h>
> >  #include <fcntl.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> >
> >  #include <stdio.h>
> >  #include <string.h>
> > @@ -95,11 +97,16 @@ static int find_block(const char *name)
> >  }
> >
> >
> > -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
> > +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
> > +                   odp_shm_e flags)
> >  {
> >       int i;
> > +     int shm = -1;
> > +     int ret;
> >       odp_shm_block_t *block;
> >       void *addr;
> > +     int shm_open_flags;
> > +     int mmap_flags = MAP_SHARED;
>
> Can't we put all int variables on the same line?
>
> >  #ifdef MAP_HUGETLB
> >       uint64_t huge_sz, page_sz;
> >
> > @@ -133,18 +140,38 @@ void *odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align)
> >       addr        = MAP_FAILED;
> >       block->huge = 0;
> >
> > +     if (flags) {
>
> flag not flags.
>
> > +             shm_open_flags = O_RDWR;
> > +             if (flags == ODP_SHM_PROC)
> > +                     shm_open_flags |= O_CREAT;
> > +
> > +             shm = shm_open(name, shm_open_flags, S_IRUSR | S_IWUSR);
> > +             if (shm == -1) {
> > +                     ODP_ERR("shm_open failed");
> > +                     return  NULL;
>
> Remove a space after "return".
>
> > +             }
> > +
> > +             ret = ftruncate(shm, size + align);
> > +             if (ret == -1) {
> > +                     ODP_ERR("ftruncate failed");
> > +                     return NULL;
> > +             }
> > +     } else {
> > +             mmap_flags |= MAP_ANONYMOUS;
> > +     }
> > +
> >  #ifdef MAP_HUGETLB
> >       /* Try first huge pages */
> >       if (huge_sz && (size + align) > page_sz) {
> >               addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
> > -                         SHM_FLAGS | MAP_HUGETLB, -1, 0);
> > +                     mmap_flags | MAP_HUGETLB, shm, 0);
>
> Fix indentation.
>
> >       }
> >  #endif
> >
> >       /* Use normal pages for small or failed huge page allocations */
> >       if (addr == MAP_FAILED) {
> >               addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
> > -                         SHM_FLAGS, -1, 0);
> > +                     mmap_flags, shm, 0);
>
> Fix indentation.
>
> >
> >       } else {
> >               block->huge = 1;
> > diff --git a/test/api_test/odp_shm_test.c b/test/api_test/odp_shm_test.c
> > index 318d662..194b4f7 100644
> > --- a/test/api_test/odp_shm_test.c
> > +++ b/test/api_test/odp_shm_test.c
> > @@ -47,7 +47,8 @@ int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
> >       odp_print_system_info();
> >
> >       test_shared_data = odp_shm_reserve("test_shared_data",
> > -                                        sizeof(test_shared_data_t),
> 128);
> > +                     sizeof(test_shared_data_t), 128,
> > +                     ODP_SHM_THREAD);
>
> Fix indentation.
>
> >       memset(test_shared_data, 0, sizeof(test_shared_data_t));
> >       printf("test shared data at %p\n\n", test_shared_data);
> >
> > diff --git a/test/api_test/odp_timer_ping.c
> b/test/api_test/odp_timer_ping.c
> > index cd67e0d..c31a850 100644
> > --- a/test/api_test/odp_timer_ping.c
> > +++ b/test/api_test/odp_timer_ping.c
> > @@ -324,7 +324,8 @@ int main(int argc ODP_UNUSED, char *argv[]
> ODP_UNUSED)
> >        * Create message pool
> >        */
> >       pool_base = odp_shm_reserve("msg_pool",
> > -                                 MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
> > +                                     MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
> > +                                     ODP_SHM_THREAD);
>
> Fix indentation.
>
> Cheers,
> Anders
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Stuart Haslam July 30, 2014, 9:46 p.m. UTC | #3
On Tue, Jul 29, 2014 at 01:29:19PM +0100, Maxim Uvarov wrote:
> Add flag to odp_shm_reserve() to select if allocation memory is
> visible by thread or it's visible by other processes.
> 
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  Proposed API change allows to allocate memory chunks like
>  buffer pools and queue specific things in memory visible
>  by other processes.
> 
>  BR,
>  Maxim.
> 

[..]

> diff --git a/include/odp_shared_memory.h b/include/odp_shared_memory.h
> index 8ac8847..10149ad 100644
> --- a/include/odp_shared_memory.h
> +++ b/include/odp_shared_memory.h
> @@ -24,6 +24,13 @@ extern "C" {
>  /** Maximum shared memory block name lenght in chars */
>  #define ODP_SHM_NAME_LEN 32
> 
> +typedef enum {
> +       ODP_SHM_THREAD = 0,    /**< Memory accessible by threads.  */
> +       ODP_SHM_PROC,          /**< Memory accessible by processes.
> +                                   Will be created if not exist.  */
> +       ODP_SHM_PROC_NOCREAT,  /**< Memory accessible by processes.
> +                                   Has to be created before usage.*/
> +} odp_shm_e;
> 

It's not clear when you'd use ODP_SHM_PROC_NOCREAT. Is the expectation
that the first process uses ODP_SHM_PROC and subsequent processes use
ODP_SHM_PROC_NOCREAT, or is it OK to use ODP_SHM_PROC in all cases? (I
think it would be OK in this implementation, but is it defined
behaviour?)

>  /**
>   * Reserve a block of shared memory
> @@ -31,10 +38,12 @@ extern "C" {
>   * @param name   Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars)
>   * @param size   Block size in bytes
>   * @param align  Block alignment in bytes
> + * @param flags  Flags for shared memory creation, ODP_SHM_THREAD or ODP_SHM_PROC

Or ODP_SHM_PROC_NOCREAT?.. just remove the list from this comment, if
all values of odp_shm_e are valid then it's easy enough to look at the
enum itself.

>   *
>   * @return Pointer to the reserved block, or NULL
>   */
> -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align);
> +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
> +                     odp_shm_e flags);
> 
>  /**
>   * Lookup for a block of shared memory

[..]

> diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c
> index 784f42b..ed94513 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -14,6 +14,8 @@
>  #include <sys/mman.h>
>  #include <asm/mman.h>
>  #include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> 
>  #include <stdio.h>
>  #include <string.h>
> @@ -95,11 +97,16 @@ static int find_block(const char *name)
>  }
> 
> 
> -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
> +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
> +                     odp_shm_e flags)
>  {
>         int i;
> +       int shm = -1;
> +       int ret;
>         odp_shm_block_t *block;
>         void *addr;
> +       int shm_open_flags;
> +       int mmap_flags = MAP_SHARED;
>  #ifdef MAP_HUGETLB
>         uint64_t huge_sz, page_sz;
> 
> @@ -133,18 +140,38 @@ void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
>         addr        = MAP_FAILED;
>         block->huge = 0;
> 
> +       if (flags) {

Should be flags != ODP_SHM_THREAD

> +               shm_open_flags = O_RDWR;
> +               if (flags == ODP_SHM_PROC)
> +                       shm_open_flags |= O_CREAT;
> +
> +               shm = shm_open(name, shm_open_flags, S_IRUSR | S_IWUSR);
> +               if (shm == -1) {
> +                       ODP_ERR("shm_open failed");

Missing an odp_spinlock_unlock here.

> +                       return  NULL;
> +               }
> +
> +               ret = ftruncate(shm, size + align);
> +               if (ret == -1) {
> +                       ODP_ERR("ftruncate failed");

Missing odp_spinlock_unlock and close/unlink of shm.

> +                       return NULL;
> +               }
> +       } else {
> +               mmap_flags |= MAP_ANONYMOUS;
> +       }
> +
>  #ifdef MAP_HUGETLB
>         /* Try first huge pages */
>         if (huge_sz && (size + align) > page_sz) {
>                 addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
> -                           SHM_FLAGS | MAP_HUGETLB, -1, 0);
> +                       mmap_flags | MAP_HUGETLB, shm, 0);
>         }
>  #endif
> 
>         /* Use normal pages for small or failed huge page allocations */
>         if (addr == MAP_FAILED) {
>                 addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
> -                           SHM_FLAGS, -1, 0);
> +                       mmap_flags, shm, 0);
> 
>         } else {
>                 block->huge = 1;

Also missing clean up of shm if the mmap fails.
Savolainen, Petri (NSN - FI/Espoo) Aug. 13, 2014, 11:29 a.m. UTC | #4
> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Stuart Haslam
> Sent: Thursday, July 31, 2014 12:47 AM
> To: Maxim Uvarov
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH] odp shm shared between processes
> 
> On Tue, Jul 29, 2014 at 01:29:19PM +0100, Maxim Uvarov wrote:
> > Add flag to odp_shm_reserve() to select if allocation memory is
> > visible by thread or it's visible by other processes.
> >
> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> > ---
> >  Proposed API change allows to allocate memory chunks like
> >  buffer pools and queue specific things in memory visible
> >  by other processes.
> >
> >  BR,
> >  Maxim.
> >
> 
> [..]
> 
> > diff --git a/include/odp_shared_memory.h
> b/include/odp_shared_memory.h
> > index 8ac8847..10149ad 100644
> > --- a/include/odp_shared_memory.h
> > +++ b/include/odp_shared_memory.h
> > @@ -24,6 +24,13 @@ extern "C" {
> >  /** Maximum shared memory block name lenght in chars */
> >  #define ODP_SHM_NAME_LEN 32
> >
> > +typedef enum {
> > +       ODP_SHM_THREAD = 0,    /**< Memory accessible by threads.  */
> > +       ODP_SHM_PROC,          /**< Memory accessible by processes.
> > +                                   Will be created if not exist.  */
> > +       ODP_SHM_PROC_NOCREAT,  /**< Memory accessible by processes.
> > +                                   Has to be created before usage.*/
> > +} odp_shm_e;
> >
> 
> It's not clear when you'd use ODP_SHM_PROC_NOCREAT. Is the expectation
> that the first process uses ODP_SHM_PROC and subsequent processes use
> ODP_SHM_PROC_NOCREAT, or is it OK to use ODP_SHM_PROC in all cases? (I
> think it would be OK in this implementation, but is it defined
> behaviour?)
> 
> >  /**
> >   * Reserve a block of shared memory
> > @@ -31,10 +38,12 @@ extern "C" {
> >   * @param name   Name of the block (maximum ODP_SHM_NAME_LEN - 1
> chars)
> >   * @param size   Block size in bytes
> >   * @param align  Block alignment in bytes
> > + * @param flags  Flags for shared memory creation, ODP_SHM_THREAD or
> ODP_SHM_PROC
> 
> Or ODP_SHM_PROC_NOCREAT?.. just remove the list from this comment, if
> all values of odp_shm_e are valid then it's easy enough to look at the
> enum itself.
> 
> >   *
> >   * @return Pointer to the reserved block, or NULL
> >   */
> > -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t
> align);
> > +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t
> align,
> > +                     odp_shm_e flags);
> >


These changes are not needed for sharing memory within an ODP instance. An implementation should map shared memory correctly regardless of ODP application is running on threads/processes/bare metal.

It's another thing to set up shared memory between two ODP instances (launched separately on the same or different VM), or between an ODP application and non-ODP application (or e.g. Linux kernel). Also there ODP application should not see the difference whether itself or the other party is composed from threads/processes/kernel driver/bare metal. Instead, I'd call this "shared memory for external communication", or even leave reservation of that memory to the application (it's system level problem to locate and set up that memory). 

Another way forward for IPC could be - not to create new ODP IPC, but add support for typical Linux mechanisms like IVSHMEM, KNI, VIRTIO. On ODP side those could managed like packet interfaces, and connected similarly to input/output queues.

-Petri
Ciprian Barbu Aug. 13, 2014, 1:05 p.m. UTC | #5
On Wed, Aug 13, 2014 at 2:29 PM, Savolainen, Petri (NSN - FI/Espoo)
<petri.savolainen@nsn.com> wrote:
>
>
>> -----Original Message-----
>> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
>> bounces@lists.linaro.org] On Behalf Of ext Stuart Haslam
>> Sent: Thursday, July 31, 2014 12:47 AM
>> To: Maxim Uvarov
>> Cc: lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [PATCH] odp shm shared between processes
>>
>> On Tue, Jul 29, 2014 at 01:29:19PM +0100, Maxim Uvarov wrote:
>> > Add flag to odp_shm_reserve() to select if allocation memory is
>> > visible by thread or it's visible by other processes.
>> >
>> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> > ---
>> >  Proposed API change allows to allocate memory chunks like
>> >  buffer pools and queue specific things in memory visible
>> >  by other processes.
>> >
>> >  BR,
>> >  Maxim.
>> >
>>
>> [..]
>>
>> > diff --git a/include/odp_shared_memory.h
>> b/include/odp_shared_memory.h
>> > index 8ac8847..10149ad 100644
>> > --- a/include/odp_shared_memory.h
>> > +++ b/include/odp_shared_memory.h
>> > @@ -24,6 +24,13 @@ extern "C" {
>> >  /** Maximum shared memory block name lenght in chars */
>> >  #define ODP_SHM_NAME_LEN 32
>> >
>> > +typedef enum {
>> > +       ODP_SHM_THREAD = 0,    /**< Memory accessible by threads.  */
>> > +       ODP_SHM_PROC,          /**< Memory accessible by processes.
>> > +                                   Will be created if not exist.  */
>> > +       ODP_SHM_PROC_NOCREAT,  /**< Memory accessible by processes.
>> > +                                   Has to be created before usage.*/
>> > +} odp_shm_e;
>> >
>>
>> It's not clear when you'd use ODP_SHM_PROC_NOCREAT. Is the expectation
>> that the first process uses ODP_SHM_PROC and subsequent processes use
>> ODP_SHM_PROC_NOCREAT, or is it OK to use ODP_SHM_PROC in all cases? (I
>> think it would be OK in this implementation, but is it defined
>> behaviour?)
>>
>> >  /**
>> >   * Reserve a block of shared memory
>> > @@ -31,10 +38,12 @@ extern "C" {
>> >   * @param name   Name of the block (maximum ODP_SHM_NAME_LEN - 1
>> chars)
>> >   * @param size   Block size in bytes
>> >   * @param align  Block alignment in bytes
>> > + * @param flags  Flags for shared memory creation, ODP_SHM_THREAD or
>> ODP_SHM_PROC
>>
>> Or ODP_SHM_PROC_NOCREAT?.. just remove the list from this comment, if
>> all values of odp_shm_e are valid then it's easy enough to look at the
>> enum itself.
>>
>> >   *
>> >   * @return Pointer to the reserved block, or NULL
>> >   */
>> > -void *odp_shm_reserve(const char *name, uint64_t size, uint64_t
>> align);
>> > +void *odp_shm_reserve(const char *name, uint64_t size, uint64_t
>> align,
>> > +                     odp_shm_e flags);
>> >
>
>
> These changes are not needed for sharing memory within an ODP instance. An implementation should map shared memory correctly regardless of ODP application is running on threads/processes/bare metal.
>
> It's another thing to set up shared memory between two ODP instances (launched separately on the same or different VM), or between an ODP application and non-ODP application (or e.g. Linux kernel). Also there ODP application should not see the difference whether itself or the other party is composed from threads/processes/kernel driver/bare metal. Instead, I'd call this "shared memory for external communication", or even leave reservation of that memory to the application (it's system level problem to locate and set up that memory).

I agree with Petri here, the whole concept of sharing a pool between
proceses or threads seems to be linux-generic specific, it doesn't
play well with bare-metal and possibly even with other arch
implementations.

But making a pool that's created over shared memory made available to
other processes by the implementation, seems like it would be
optional, unless we decide otherwise. Or to put in in other words, do
we impose that pools are visible between different ODP instances where
such a concept as a thread or a process exists?

/Ciprian

>
> Another way forward for IPC could be - not to create new ODP IPC, but add support for typical Linux mechanisms like IVSHMEM, KNI, VIRTIO. On ODP side those could managed like packet interfaces, and connected similarly to input/output queues.
>
> -Petri
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
index e4a72fa..b1be789 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -542,7 +542,8 @@  int main(int argc, char *argv[])
 	odp_atomic_init_u64(&counters.icmp);
 
 	/* Reserve memory for args from shared mem */
-	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE);
+	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE,
+				ODP_SHM_THREAD);
 	if (args == NULL) {
 		ODP_ERR("Error: shared mem alloc failed.\n");
 		exit(EXIT_FAILURE);
@@ -587,7 +588,8 @@  int main(int argc, char *argv[])
 
 	/* Create packet pool */
 	pool_base = odp_shm_reserve("shm_packet_pool",
-				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+				    ODP_SHM_THREAD);
 	if (pool_base == NULL) {
 		ODP_ERR("Error: packet pool mem alloc failed.\n");
 		exit(EXIT_FAILURE);
diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
index e331ff2..c5f3725 100644
--- a/example/l2fwd/odp_l2fwd.c
+++ b/example/l2fwd/odp_l2fwd.c
@@ -294,7 +294,8 @@  int main(int argc, char *argv[])
 	}
 
 	/* Reserve memory for args from shared mem */
-	gbl_args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE);
+	gbl_args = odp_shm_reserve("shm_args", sizeof(args_t),
+				 ODP_CACHE_LINE_SIZE, ODP_SHM_THREAD);
 	if (gbl_args == NULL) {
 		ODP_ERR("Error: shared mem alloc failed.\n");
 		exit(EXIT_FAILURE);
@@ -345,7 +346,8 @@  int main(int argc, char *argv[])
 
 	/* Create packet pool */
 	pool_base = odp_shm_reserve("shm_packet_pool",
-				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+				    ODP_SHM_THREAD);
 	if (pool_base == NULL) {
 		ODP_ERR("Error: packet pool mem alloc failed.\n");
 		exit(EXIT_FAILURE);
diff --git a/example/odp_example/odp_example.c b/example/odp_example/odp_example.c
index be96093..6d075b2 100644
--- a/example/odp_example/odp_example.c
+++ b/example/odp_example/odp_example.c
@@ -987,7 +987,8 @@  int main(int argc, char *argv[])
 	 * Create message pool
 	 */
 	pool_base = odp_shm_reserve("msg_pool",
-				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+				    ODP_SHM_THREAD);
 
 	pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
 				      sizeof(test_message_t),
diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
index edf8cfd..f5694c3 100644
--- a/example/packet/odp_pktio.c
+++ b/example/packet/odp_pktio.c
@@ -291,7 +291,8 @@  int main(int argc, char *argv[])
 	}
 
 	/* Reserve memory for args from shared mem */
-	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE);
+	args = odp_shm_reserve("shm_args", sizeof(args_t), ODP_CACHE_LINE_SIZE,
+				ODP_SHM_THREAD);
 	if (args == NULL) {
 		ODP_ERR("Error: shared mem alloc failed.\n");
 		exit(EXIT_FAILURE);
@@ -332,7 +333,8 @@  int main(int argc, char *argv[])
 
 	/* Create packet pool */
 	pool_base = odp_shm_reserve("shm_packet_pool",
-				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+				    SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+				    ODP_SHM_THREAD);
 	if (pool_base == NULL) {
 		ODP_ERR("Error: packet pool mem alloc failed.\n");
 		exit(EXIT_FAILURE);
diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c
index dbe0e5b..113200b 100644
--- a/example/timer/odp_timer_test.c
+++ b/example/timer/odp_timer_test.c
@@ -260,7 +260,8 @@  int main(int argc, char *argv[])
 	 * Create message pool
 	 */
 	pool_base = odp_shm_reserve("msg_pool",
-				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+				    ODP_SHM_THREAD);
 
 	pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
 				      0,
diff --git a/include/odp_shared_memory.h b/include/odp_shared_memory.h
index 8ac8847..10149ad 100644
--- a/include/odp_shared_memory.h
+++ b/include/odp_shared_memory.h
@@ -24,6 +24,13 @@  extern "C" {
 /** Maximum shared memory block name lenght in chars */
 #define ODP_SHM_NAME_LEN 32
 
+typedef enum {
+	ODP_SHM_THREAD = 0,    /**< Memory accessible by threads.  */
+	ODP_SHM_PROC,	       /**< Memory accessible by processes.
+				    Will be created if not exist.  */
+	ODP_SHM_PROC_NOCREAT,  /**< Memory accessible by processes.
+				    Has to be created before usage.*/
+} odp_shm_e;
 
 /**
  * Reserve a block of shared memory
@@ -31,10 +38,12 @@  extern "C" {
  * @param name   Name of the block (maximum ODP_SHM_NAME_LEN - 1 chars)
  * @param size   Block size in bytes
  * @param align  Block alignment in bytes
+ * @param flags  Flags for shared memory creation, ODP_SHM_THREAD or ODP_SHM_PROC
  *
  * @return Pointer to the reserved block, or NULL
  */
-void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align);
+void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
+		      odp_shm_e flags);
 
 /**
  * Lookup for a block of shared memory
diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c
index a48781a..9157994 100644
--- a/platform/linux-generic/odp_buffer_pool.c
+++ b/platform/linux-generic/odp_buffer_pool.c
@@ -102,7 +102,8 @@  int odp_buffer_pool_init_global(void)
 
 	pool_tbl = odp_shm_reserve("odp_buffer_pools",
 				   sizeof(pool_table_t),
-				   sizeof(pool_entry_t));
+				   sizeof(pool_entry_t),
+				   ODP_SHM_THREAD);
 
 	if (pool_tbl == NULL)
 		return -1;
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c
index 33ade10..050ade4 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -55,7 +55,8 @@  int odp_pktio_init_global(void)
 
 	pktio_tbl = odp_shm_reserve("odp_pktio_entries",
 				    sizeof(pktio_table_t),
-				    sizeof(pktio_entry_t));
+				    sizeof(pktio_entry_t),
+				    ODP_SHM_THREAD);
 	if (pktio_tbl == NULL)
 		return -1;
 
diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-generic/odp_queue.c
index c637bdf..507331f 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -99,7 +99,8 @@  int odp_queue_init_global(void)
 
 	queue_tbl = odp_shm_reserve("odp_queues",
 				    sizeof(queue_table_t),
-				    sizeof(queue_entry_t));
+				    sizeof(queue_entry_t),
+				    ODP_SHM_THREAD);
 
 	if (queue_tbl == NULL)
 		return -1;
diff --git a/platform/linux-generic/odp_ring.c b/platform/linux-generic/odp_ring.c
index 25ff66a..ee8175e 100644
--- a/platform/linux-generic/odp_ring.c
+++ b/platform/linux-generic/odp_ring.c
@@ -171,7 +171,8 @@  odp_ring_create(const char *name, unsigned count, unsigned flags)
 
 	odp_rwlock_write_lock(&qlock);
 	/* reserve a memory zone for this ring.*/
-	r = odp_shm_reserve(ring_name, ring_size, ODP_CACHE_LINE_SIZE);
+	r = odp_shm_reserve(ring_name, ring_size, ODP_CACHE_LINE_SIZE,
+			    ODP_SHM_THREAD);
 
 	if (r != NULL) {
 		/* init the ring structure */
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-generic/odp_schedule.c
index 9e399f1..722e297 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -89,7 +89,8 @@  int odp_schedule_init_global(void)
 
 	sched = odp_shm_reserve("odp_scheduler",
 				sizeof(sched_t),
-				ODP_CACHE_LINE_SIZE);
+				ODP_CACHE_LINE_SIZE,
+				ODP_SHM_THREAD);
 
 	if (sched == NULL) {
 		ODP_ERR("Schedule init: Shm reserve failed.\n");
@@ -98,7 +99,8 @@  int odp_schedule_init_global(void)
 
 
 	pool_base = odp_shm_reserve("odp_sched_pool",
-				    SCHED_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+			SCHED_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+			ODP_SHM_THREAD);
 
 	pool = odp_buffer_pool_create("odp_sched_pool", pool_base,
 				      SCHED_POOL_SIZE, sizeof(queue_desc_t),
diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c
index 784f42b..ed94513 100644
--- a/platform/linux-generic/odp_shared_memory.c
+++ b/platform/linux-generic/odp_shared_memory.c
@@ -14,6 +14,8 @@ 
 #include <sys/mman.h>
 #include <asm/mman.h>
 #include <fcntl.h>
+#include <unistd.h>
+#include <sys/types.h>
 
 #include <stdio.h>
 #include <string.h>
@@ -95,11 +97,16 @@  static int find_block(const char *name)
 }
 
 
-void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
+void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
+		      odp_shm_e flags)
 {
 	int i;
+	int shm = -1;
+	int ret;
 	odp_shm_block_t *block;
 	void *addr;
+	int shm_open_flags;
+	int mmap_flags = MAP_SHARED;
 #ifdef MAP_HUGETLB
 	uint64_t huge_sz, page_sz;
 
@@ -133,18 +140,38 @@  void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)
 	addr        = MAP_FAILED;
 	block->huge = 0;
 
+	if (flags) {
+		shm_open_flags = O_RDWR;
+		if (flags == ODP_SHM_PROC)
+			shm_open_flags |= O_CREAT;
+
+		shm = shm_open(name, shm_open_flags, S_IRUSR | S_IWUSR);
+		if (shm == -1) {
+			ODP_ERR("shm_open failed");
+			return  NULL;
+		}
+
+		ret = ftruncate(shm, size + align);
+		if (ret == -1) {
+			ODP_ERR("ftruncate failed");
+			return NULL;
+		}
+	} else {
+		mmap_flags |= MAP_ANONYMOUS;
+	}
+
 #ifdef MAP_HUGETLB
 	/* Try first huge pages */
 	if (huge_sz && (size + align) > page_sz) {
 		addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
-			    SHM_FLAGS | MAP_HUGETLB, -1, 0);
+			mmap_flags | MAP_HUGETLB, shm, 0);
 	}
 #endif
 
 	/* Use normal pages for small or failed huge page allocations */
 	if (addr == MAP_FAILED) {
 		addr = mmap(NULL, size + align, PROT_READ | PROT_WRITE,
-			    SHM_FLAGS, -1, 0);
+			mmap_flags, shm, 0);
 
 	} else {
 		block->huge = 1;
diff --git a/test/api_test/odp_shm_test.c b/test/api_test/odp_shm_test.c
index 318d662..194b4f7 100644
--- a/test/api_test/odp_shm_test.c
+++ b/test/api_test/odp_shm_test.c
@@ -47,7 +47,8 @@  int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
 	odp_print_system_info();
 
 	test_shared_data = odp_shm_reserve("test_shared_data",
-					   sizeof(test_shared_data_t), 128);
+			sizeof(test_shared_data_t), 128,
+			ODP_SHM_THREAD);
 	memset(test_shared_data, 0, sizeof(test_shared_data_t));
 	printf("test shared data at %p\n\n", test_shared_data);
 
diff --git a/test/api_test/odp_timer_ping.c b/test/api_test/odp_timer_ping.c
index cd67e0d..c31a850 100644
--- a/test/api_test/odp_timer_ping.c
+++ b/test/api_test/odp_timer_ping.c
@@ -324,7 +324,8 @@  int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
 	 * Create message pool
 	 */
 	pool_base = odp_shm_reserve("msg_pool",
-				    MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE);
+					MSG_POOL_SIZE, ODP_CACHE_LINE_SIZE,
+					ODP_SHM_THREAD);
 
 	pool = odp_buffer_pool_create("msg_pool", pool_base, MSG_POOL_SIZE,
 				      BUF_SIZE,