On Mon, Sep 01, 2014 at 12:53:04PM +0100, Petri Savolainen wrote:
> Add flags parameter to reserve call.
> * SW_ONLY: optimize memory usage when HW accelerators are not
>   involved.
> * PROC: for communication with other (non-ODP) processes in the
>   system

The non-ODP part isn't really a limitation as suggested here.

> * KERNEL: for communication with a kernel (OS) driver
> 
> Signed-off-by: Petri Savolainen <[email protected]>
> ---
>  .../linux-generic/include/api/odp_shared_memory.h  | 13 ++++++-
>  platform/linux-generic/odp_shared_memory.c         | 45 
> +++++++++++++++++-----
>  2 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/platform/linux-generic/include/api/odp_shared_memory.h 
> b/platform/linux-generic/include/api/odp_shared_memory.h
> index 8ac8847..814acdc 100644
> --- a/platform/linux-generic/include/api/odp_shared_memory.h
> +++ b/platform/linux-generic/include/api/odp_shared_memory.h
> @@ -24,6 +24,15 @@ extern "C" {
>  /** Maximum shared memory block name lenght in chars */

typo - length (I know it wasn't changed in this patch, but if you're
doing a v2 anyway..)

>  #define ODP_SHM_NAME_LEN 32
>  
> +/*
> + * Shared memory flags
> + */
> +
> +/* Share level */
> +#define ODP_SHM_SW_ONLY 0x1 /**< Internal to application SW, no HW access */
> +#define ODP_SHM_PROC    0x2 /**< Share with external processes */
> +#define ODP_SHM_KERNEL  0x4 /**< Share with kernel */

This seems a bit confused as to whether it's a level or a flag. If you
set only ODP_SHM_PROC the suggestion is that this means it wouldn't be
shared internal to the application.. but ODP_SHM_SW_ONLY|ODP_SHM_PROC
isn't logical either.

> +
>  
>  /**
>   * Reserve a block of shared memory
> @@ -31,10 +40,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  Block parameter 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,
> +                   uint32_t 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..4cb82b5 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -11,6 +11,7 @@
>  #include <odp_system_info.h>
>  #include <odp_debug.h>
>  
> +#include <unistd.h>
>  #include <sys/mman.h>
>  #include <asm/mman.h>
>  #include <fcntl.h>
> @@ -44,8 +45,6 @@ typedef struct {
>  #define MAP_ANONYMOUS MAP_ANON
>  #endif
>  
> -#define SHM_FLAGS (MAP_SHARED | MAP_ANONYMOUS)
> -
>  
>  /* Global shared memory table */
>  static odp_shm_table_t *odp_shm_tbl;
> @@ -60,7 +59,7 @@ int odp_shm_init_global(void)
>  #endif
>  
>       addr = mmap(NULL, sizeof(odp_shm_table_t),
> -                 PROT_READ | PROT_WRITE, SHM_FLAGS, -1, 0);
> +                 PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>  
>       if (addr == MAP_FAILED)
>               return -1;
> @@ -95,11 +94,17 @@ 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,
> +                   uint32_t flags)
>  {
>       int i;
>       odp_shm_block_t *block;
>       void *addr;
> +     int fd = -1;
> +     int map_flag = MAP_SHARED;
> +     /* O_EXCL: error, O_TRUNC: truncate to zero, if already exists */
> +     int oflag = O_RDWR | O_CREAT | O_TRUNC;

Not sure why the comment mentions O_EXCL as it isn't being set. I suppose
the expected behaviour is that if the shm already exists it's just opened,
so this is just a comment error.

-- 
Stuart.


_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to