Re: [lng-odp] [PATCH 1/2] Shared memory reserve flag

Maxim Uvarov Tue, 02 Sep 2014 08:44:24 -0700


On 09/01/2014 03:53 PM, 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
* KERNEL: for communication with a kernel (OS) driver

Hi Petri,

this patch is about the same which I sent.

1. One difference is KERNEL flag. How if KERNEL flag will be used? Isthere is specific need for that? What will happen if

app will set this flag but flag is not supported by platform.

2. If public API changed then you need to fix all existence platforms,to not break their compilation/work.

3. What I don't like in current odp_shm_reserve() is that it'sdifficult to understand what is the reason of the error.

I think it's reasonable to change that function from:
void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
              odp_shm_e flag)
to

int odp_shm_reserve(void **addr, const char *name, uint64_t size,uint64_t align,

              odp_shm_e flag)

where return codes are: 1) chunk with that name already exist. addr isset. 2) mmap failed 3) exceed number of ODP_SHM_NUM_BLOCKS and etc.

I.e. not just fail with ODP_ERR() but return some reasonable value.

Thanks,
Maxim.

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 */
  #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 */
+

/**

   * 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;
+       uint64_t alloc_size = size + align;
  #ifdef MAP_HUGETLB
        uint64_t huge_sz, page_sz;

@@ -107,11 +112,31 @@ void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)

        page_sz = odp_sys_page_size();
  #endif

+ if (flags & ODP_SHM_PROC) {

+               /* Creates a file to /dev/shm */
+               fd = shm_open(name, oflag,
+                             S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+               if (fd == -1) {
+                       ODP_DBG("odp_shm_reserve: shm_open failed\n");
+                       return NULL;
+               }
+
+               if (ftruncate(fd, alloc_size) == -1) {
+                       ODP_DBG("odp_shm_reserve: ftruncate failed\n");
+                       return NULL;
+               }
+
+       } else {
+               map_flag |= MAP_ANONYMOUS;
+       }
+
        odp_spinlock_lock(&odp_shm_tbl->lock);

if (find_block(name) >= 0) {

                /* Found a block with the same name */
                odp_spinlock_unlock(&odp_shm_tbl->lock);
+               ODP_DBG("odp_shm_reserve: name already used\n");
                return NULL;
        }

@@ -125,6 +150,7 @@ void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)

        if (i > ODP_SHM_NUM_BLOCKS - 1) {
                /* Table full */
                odp_spinlock_unlock(&odp_shm_tbl->lock);
+               ODP_DBG("odp_shm_reserve: no more blocks\n");
                return NULL;
        }

@@ -135,16 +161,16 @@ void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align)#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);
+       if (huge_sz && alloc_size > page_sz) {
+               addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
+                           map_flag | MAP_HUGETLB, fd, 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);
+               addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
+                           map_flag, fd, 0);

} else {

                block->huge = 1;
@@ -153,6 +179,7 @@ void *odp_shm_reserve(const char *name, uint64_t size, 
uint64_t align)
        if (addr == MAP_FAILED) {
                /* Alloc failed */
                odp_spinlock_unlock(&odp_shm_tbl->lock);
+               ODP_DBG("odp_shm_reserve: mmap failed\n");
                return NULL;
        }



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

Re: [lng-odp] [PATCH 1/2] Shared memory reserve flag

Reply via email to