On 09/04/2014 04:27 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
-----Original Message-----
From: ext Stuart Haslam [mailto:[email protected]]
Sent: Thursday, September 04, 2014 2:19 PM
To: Petri Savolainen
Cc: [email protected]
Subject: Re: [lng-odp] [PATCH 1/2] Shared memory reserve flag
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.
I think it is, since ODP API calls should be the same regardless of the
application is running as threads/processes/bare metal. That's supported by
default. Here the PROC flag says that external processes should see the memory
also.
Why not put this to enum? I have:
typedef enum {
ODP_SHM_THREAD = 1, /**< Memory accessible by threads. */
ODP_SHM_PROC = 2,
}
It might be ODP_SHM_THREAD is not the good name. And we can change it to
ODP_SHM_DEFAULT. Which is more readable then 0 flag.
Or we can go with enum and bind default to 0.
Maxim.
* 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.
ODP_SHM_SW_ONLY == HW accelerators will _not_ access this memory
define ODP_SHM_PROC == external (user space) processes access this memory
For a SW only struct, ODP_SHM_SW_ONLY|ODP_SHM_PROC would be logical choice.
+
/**
* 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.
O_EXCL is there to remain, that it should be used after we have proper clean up
implemented. I choose to O_TRUNC now so that user do not have to delete the the
/dev/shm/foobar file between application runs.
-Petri
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
