| Message ID | 1409916551-24327-1-git-send-email-petri.savolainen@linaro.org |
|---|---|
| State | New |
| Headers | show |
Taras, Venky, Bala, Alex are you ok with that change? Maxim. On 09/05/2014 03:29 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 external (non-ODP) processes in the > system > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > .../linux-generic/include/api/odp_shared_memory.h | 15 +++++++- > platform/linux-generic/odp_shared_memory.c | 45 +++++++++++++++++----- > platform/linux-keystone2/odp_shared_memory.c | 5 ++- > 3 files changed, 53 insertions(+), 12 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..7d9fedd 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -21,9 +21,18 @@ extern "C" { > > #include <odp_std_types.h> > > -/** Maximum shared memory block name lenght in chars */ > +/** Maximum shared memory block name length in chars */ > #define ODP_SHM_NAME_LEN 32 > > +/* > + * Shared memory flags > + */ > + > +/* Share level */ > +#define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ > +#define ODP_SHM_PROC 0x2 /**< Share with external processes */ > + > + > > /** > * 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 Shared mem parameter flags (ODP_SHM_*). Default value is 0. > * > * @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..3d1e12a 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; > + /* If already exists: O_EXCL: error, O_TRUNC: truncate to zero */ > + 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; > } > > diff --git a/platform/linux-keystone2/odp_shared_memory.c b/platform/linux-keystone2/odp_shared_memory.c > index e595111..69ac34e 100644 > --- a/platform/linux-keystone2/odp_shared_memory.c > +++ b/platform/linux-keystone2/odp_shared_memory.c > @@ -208,8 +208,11 @@ void *_odp_shm_reserve(const char *name, uint64_t size, uint64_t align, > return block->addr; > } > > -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) > { > + (void)flags; > + > return _odp_shm_reserve(name, size, align, ODP_SHM_CMA); > } >
Hi, A Few queries from my side, * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process to be able to do a lookup of shared memory created by process outside of ODP. Current odp_shm_lookup only checks shm created by ODP process. * one use case which I can think of for ODP_SHM_PROC is for IPC across ODP and non-ODP process. if that is the case then we need to define a ODP message structure which will be inherited by all process which wants to communicate with ODP process. Regards, Bala On 7 September 2014 02:14, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Taras, Venky, Bala, Alex are you ok with that change? > > Maxim. > > > On 09/05/2014 03:29 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 external (non-ODP) processes in the >> system >> >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> >> --- >> .../linux-generic/include/api/odp_shared_memory.h | 15 +++++++- >> platform/linux-generic/odp_shared_memory.c | 45 >> +++++++++++++++++----- >> platform/linux-keystone2/odp_shared_memory.c | 5 ++- >> 3 files changed, 53 insertions(+), 12 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..7d9fedd 100644 >> --- a/platform/linux-generic/include/api/odp_shared_memory.h >> +++ b/platform/linux-generic/include/api/odp_shared_memory.h >> @@ -21,9 +21,18 @@ extern "C" { >> #include <odp_std_types.h> >> -/** Maximum shared memory block name lenght in chars */ >> +/** Maximum shared memory block name length in chars */ >> #define ODP_SHM_NAME_LEN 32 >> +/* >> + * Shared memory flags >> + */ >> + >> +/* Share level */ >> +#define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ >> +#define ODP_SHM_PROC 0x2 /**< Share with external processes */ >> + >> + >> /** >> * 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 Shared mem parameter flags (ODP_SHM_*). Default value >> is 0. >> * >> * @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..3d1e12a 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; >> + /* If already exists: O_EXCL: error, O_TRUNC: truncate to zero */ >> + 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; >> } >> diff --git a/platform/linux-keystone2/odp_shared_memory.c >> b/platform/linux-keystone2/odp_shared_memory.c >> index e595111..69ac34e 100644 >> --- a/platform/linux-keystone2/odp_shared_memory.c >> +++ b/platform/linux-keystone2/odp_shared_memory.c >> @@ -208,8 +208,11 @@ void *_odp_shm_reserve(const char *name, uint64_t >> size, uint64_t align, >> return block->addr; >> } >> -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) >> { >> + (void)flags; >> + >> return _odp_shm_reserve(name, size, align, ODP_SHM_CMA); >> } >> >> > >
On 09/08/2014 09:30 AM, Bala Manoharan wrote: > Hi, > > A Few queries from my side, > > * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process > to be able to do a lookup of shared memory created by process outside > of ODP. Current odp_shm_lookup only checks shm created by ODP process. Yes. And this is process odp_shm_reserve(). You can take a look how I do it for IPC in ipc patches. You need to call odp_shm_reserve() in 2 process with the same "name" argument. > > * one use case which I can think of for ODP_SHM_PROC is for IPC across > ODP and non-ODP process. if that is the case then we need to define a > ODP message structure which will be inherited by all process which > wants to communicate with ODP process. > For IPC in my IPC patches. I do transfer odp_buffer_t buffers. Standard odp_queue_create()/odp_queue_lookup() is used to create queues. Then standard queue_enqueue/queue_dequeue is used to queue packet to IPC queue. But this patch is related to shared memory between processes. And additional flags to odp_shm_reserve. It's not about IPC details. You can comment about IPC in "[PATCHv5 0/2] odp ipc implementation" thread. Thanks, Maxim. > > Regards, > Bala > > > On 7 September 2014 02:14, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Taras, Venky, Bala, Alex are you ok with that change? > > Maxim. > > > On 09/05/2014 03:29 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 external (non-ODP) processes in the > system > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org > <mailto:petri.savolainen@linaro.org>> > --- > .../linux-generic/include/api/odp_shared_memory.h | 15 +++++++- > platform/linux-generic/odp_shared_memory.c | 45 > +++++++++++++++++----- > platform/linux-keystone2/odp_shared_memory.c | 5 ++- > 3 files changed, 53 insertions(+), 12 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..7d9fedd 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -21,9 +21,18 @@ extern "C" { > #include <odp_std_types.h> > -/** Maximum shared memory block name lenght in chars */ > +/** Maximum shared memory block name length in chars */ > #define ODP_SHM_NAME_LEN 32 > +/* > + * Shared memory flags > + */ > + > +/* Share level */ > +#define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW > access */ > +#define ODP_SHM_PROC 0x2 /**< Share with external processes */ > + > + > /** > * 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 Shared mem parameter flags (ODP_SHM_*). > Default value is 0. > * > * @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..3d1e12a 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; > + /* If already exists: O_EXCL: error, O_TRUNC: truncate > to zero */ > + 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; > } > diff --git a/platform/linux-keystone2/odp_shared_memory.c > b/platform/linux-keystone2/odp_shared_memory.c > index e595111..69ac34e 100644 > --- a/platform/linux-keystone2/odp_shared_memory.c > +++ b/platform/linux-keystone2/odp_shared_memory.c > @@ -208,8 +208,11 @@ void *_odp_shm_reserve(const char *name, > uint64_t size, uint64_t align, > return block->addr; > } > -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) > { > + (void)flags; > + > return _odp_shm_reserve(name, size, align, ODP_SHM_CMA); > } > > >
> -----Original Message----- > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov > Sent: Monday, September 08, 2014 9:31 AM > To: Bala Manoharan > Cc: lng-odp-forward > Subject: Re: [lng-odp] [PATCH v2 1/2] Shared memory reserve flag > > On 09/08/2014 09:30 AM, Bala Manoharan wrote: > > Hi, > > > > A Few queries from my side, > > > > * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process > > to be able to do a lookup of shared memory created by process outside > > of ODP. Current odp_shm_lookup only checks shm created by ODP process. > > Yes. And this is process odp_shm_reserve(). You can take a look how I do > it for IPC in ipc patches. You need to call odp_shm_reserve() in 2 > process with the same "name" argument. Do we want to support shm created outside of ODP? Since ODP does not have full control over that memory, ODP cannot guarantee that application can use it in a portable way. Maybe it's better to operate such memory outside of ODP. If decided to support in ODP, it should be clearly separate from ODP controlled shm. -Petri
Yes. Support for shm created outside ODP will cause a lot of overhead. we can use IPC mechanism to check the validity of the allocated shm in ODP from a non-ODP process. @Maxim Sure.I will continue the IPC discussion in IPC patch. Regards, Bala On 8 September 2014 13:10, 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 Maxim Uvarov > > Sent: Monday, September 08, 2014 9:31 AM > > To: Bala Manoharan > > Cc: lng-odp-forward > > Subject: Re: [lng-odp] [PATCH v2 1/2] Shared memory reserve flag > > > > On 09/08/2014 09:30 AM, Bala Manoharan wrote: > > > Hi, > > > > > > A Few queries from my side, > > > > > > * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process > > > to be able to do a lookup of shared memory created by process outside > > > of ODP. Current odp_shm_lookup only checks shm created by ODP process. > > > > Yes. And this is process odp_shm_reserve(). You can take a look how I do > > it for IPC in ipc patches. You need to call odp_shm_reserve() in 2 > > process with the same "name" argument. > > Do we want to support shm created outside of ODP? Since ODP does not have > full control over that memory, ODP cannot guarantee that application can > use it in a portable way. Maybe it's better to operate such memory outside > of ODP. > > If decided to support in ODP, it should be clearly separate from ODP > controlled shm. > > -Petri > > > >
On 09/08/2014 11:40 AM, Savolainen, Petri (NSN - FI/Espoo) wrote: > >> -----Original Message----- >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov >> Sent: Monday, September 08, 2014 9:31 AM >> To: Bala Manoharan >> Cc: lng-odp-forward >> Subject: Re: [lng-odp] [PATCH v2 1/2] Shared memory reserve flag >> >> On 09/08/2014 09:30 AM, Bala Manoharan wrote: >>> Hi, >>> >>> A Few queries from my side, >>> >>> * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process >>> to be able to do a lookup of shared memory created by process outside >>> of ODP. Current odp_shm_lookup only checks shm created by ODP process. >> Yes. And this is process odp_shm_reserve(). You can take a look how I do >> it for IPC in ipc patches. You need to call odp_shm_reserve() in 2 >> process with the same "name" argument. > Do we want to support shm created outside of ODP? Since ODP does not have full control over that memory, ODP cannot guarantee that application can use it in a portable way. Maybe it's better to operate such memory outside of ODP. > > If decided to support in ODP, it should be clearly separate from ODP controlled shm. > > -Petri > I think it should be part of ODP. I.e. application developer should not care about allocation models, functions or any hw specific things like CMA. It should be platform specific thing. And application should be the same for all platforms. Maxim.
> -----Original Message----- > From: ext Maxim Uvarov [mailto:maxim.uvarov@linaro.org] > Sent: Monday, September 08, 2014 1:18 PM > To: Savolainen, Petri (NSN - FI/Espoo); Bala Manoharan > Cc: lng-odp-forward > Subject: Re: [lng-odp] [PATCH v2 1/2] Shared memory reserve flag > > On 09/08/2014 11:40 AM, Savolainen, Petri (NSN - FI/Espoo) wrote: > > > >> -----Original Message----- > >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > >> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov > >> Sent: Monday, September 08, 2014 9:31 AM > >> To: Bala Manoharan > >> Cc: lng-odp-forward > >> Subject: Re: [lng-odp] [PATCH v2 1/2] Shared memory reserve flag > >> > >> On 09/08/2014 09:30 AM, Bala Manoharan wrote: > >>> Hi, > >>> > >>> A Few queries from my side, > >>> > >>> * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process > >>> to be able to do a lookup of shared memory created by process outside > >>> of ODP. Current odp_shm_lookup only checks shm created by ODP process. > >> Yes. And this is process odp_shm_reserve(). You can take a look how I > do > >> it for IPC in ipc patches. You need to call odp_shm_reserve() in 2 > >> process with the same "name" argument. > > Do we want to support shm created outside of ODP? Since ODP does not > have full control over that memory, ODP cannot guarantee that application > can use it in a portable way. Maybe it's better to operate such memory > outside of ODP. > > > > If decided to support in ODP, it should be clearly separate from ODP > controlled shm. > > > > -Petri > > > > I think it should be part of ODP. I.e. application developer should not > care about allocation models, functions or any hw specific things like > CMA. It should be platform specific thing. And application should be the > same for all platforms. > > Maxim. > ODP won't be OS abstraction. If some shared memory was created outside of ODP (e.g. by a ctrl app), ODP implementation does not know about it or have control over it, so it should passed to the ODP app also outside of ODP. It's safe assumption, that ODP API can be used only for memory that is created/controlled through ODP. How and where non-ODP apps find that (ODP controlled) memory is implementation/system specific. Are we now ready to merge this patch? -Petri
On 09/10/2014 11:20 AM, Savolainen, Petri (NSN - FI/Espoo) wrote: > Are we now ready to merge this patch? > > -Petri I think we are ready. But merging it right now can break other apps. Because it's global api change. I will merge it after Connect to not break anyones demo, CI jobs and etc. Thanks, Maxim.
On 2014-09-10 13:14, Maxim Uvarov wrote: > On 09/10/2014 11:20 AM, Savolainen, Petri (NSN - FI/Espoo) wrote: > >Are we now ready to merge this patch? > > > >-Petri > > I think we are ready. But merging it right now can break other apps. > Because it's global api change. > I will merge it after Connect to not break anyones demo, CI jobs and etc. I can understand that we don't wont to break stuff for connect. However, why are we depending on the latest from ODP for the demo, and not another "demo" repo or branch? If the CI jobs breaks we have to fix that... Thats a minor, right? I think its wrong that we taking a "break" in the development because of this... Cheers, Anders > > Thanks, > Maxim. > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
> -----Original Message----- > From: ext Anders Roxell [mailto:anders.roxell@linaro.org] > Sent: Thursday, September 11, 2014 11:22 AM > To: Maxim Uvarov > Cc: Savolainen, Petri (NSN - FI/Espoo); Bala Manoharan; lng-odp-forward > Subject: Re: [lng-odp] [PATCH v2 1/2] Shared memory reserve flag > > On 2014-09-10 13:14, Maxim Uvarov wrote: > > On 09/10/2014 11:20 AM, Savolainen, Petri (NSN - FI/Espoo) wrote: > > >Are we now ready to merge this patch? > > > > > >-Petri > > > > I think we are ready. But merging it right now can break other apps. > > Because it's global api change. > > I will merge it after Connect to not break anyones demo, CI jobs and > etc. > > I can understand that we don't wont to break stuff for connect. > However, why are we depending on the latest from ODP for the demo, and > not another "demo" repo or branch? > If the CI jobs breaks we have to fix that... Thats a minor, right? > I think its wrong that we taking a "break" in the development because of > this... Agree. We don't force anybody to do "git pull" before starting a demo :) -Petri
On 09/08/2014 09:30 AM, Bala Manoharan wrote: > Hi, > > A Few queries from my side, > > * ODP_SHM_PROC: In this case we need a mechanism to allow ODP process > to be able to do a lookup of shared memory created by process outside > of ODP. Current odp_shm_lookup only checks shm created by ODP process. I use that flags in IPC. I do look up for queue by it's name. It looks like subject for other patch. > > * one use case which I can think of for ODP_SHM_PROC is for IPC across > ODP and non-ODP process. if that is the case then we need to define a > ODP message structure which will be inherited by all process which > wants to communicate with ODP process. > I agree with Petri here that no need to mix names and odp and non odp process. At least for now when we do not have real test case for that. In general what is reasonable it have some magic + memory size structure for each shm_opened memory block. I upped this patch to the latest ODP (added zeros to crypto app). And ready to merge it. If no objections. Maxim. > > Regards, > Bala > > > On 7 September 2014 02:14, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Taras, Venky, Bala, Alex are you ok with that change? > > Maxim. > > > On 09/05/2014 03:29 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 external (non-ODP) processes in the > system > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org > <mailto:petri.savolainen@linaro.org>> > --- > .../linux-generic/include/api/odp_shared_memory.h | 15 +++++++- > platform/linux-generic/odp_shared_memory.c | 45 > +++++++++++++++++----- > platform/linux-keystone2/odp_shared_memory.c | 5 ++- > 3 files changed, 53 insertions(+), 12 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..7d9fedd 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -21,9 +21,18 @@ extern "C" { > #include <odp_std_types.h> > -/** Maximum shared memory block name lenght in chars */ > +/** Maximum shared memory block name length in chars */ > #define ODP_SHM_NAME_LEN 32 > +/* > + * Shared memory flags > + */ > + > +/* Share level */ > +#define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW > access */ > +#define ODP_SHM_PROC 0x2 /**< Share with external processes */ > + > + > /** > * 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 Shared mem parameter flags (ODP_SHM_*). > Default value is 0. > * > * @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..3d1e12a 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; > + /* If already exists: O_EXCL: error, O_TRUNC: truncate > to zero */ > + 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; > } > diff --git a/platform/linux-keystone2/odp_shared_memory.c > b/platform/linux-keystone2/odp_shared_memory.c > index e595111..69ac34e 100644 > --- a/platform/linux-keystone2/odp_shared_memory.c > +++ b/platform/linux-keystone2/odp_shared_memory.c > @@ -208,8 +208,11 @@ void *_odp_shm_reserve(const char *name, > uint64_t size, uint64_t align, > return block->addr; > } > -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) > { > + (void)flags; > + > return _odp_shm_reserve(name, size, align, ODP_SHM_CMA); > } > > >
diff --git a/platform/linux-generic/include/api/odp_shared_memory.h b/platform/linux-generic/include/api/odp_shared_memory.h index 8ac8847..7d9fedd 100644 --- a/platform/linux-generic/include/api/odp_shared_memory.h +++ b/platform/linux-generic/include/api/odp_shared_memory.h @@ -21,9 +21,18 @@ extern "C" { #include <odp_std_types.h> -/** Maximum shared memory block name lenght in chars */ +/** Maximum shared memory block name length in chars */ #define ODP_SHM_NAME_LEN 32 +/* + * Shared memory flags + */ + +/* Share level */ +#define ODP_SHM_SW_ONLY 0x1 /**< Application SW only, no HW access */ +#define ODP_SHM_PROC 0x2 /**< Share with external processes */ + + /** * 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 Shared mem parameter flags (ODP_SHM_*). Default value is 0. * * @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..3d1e12a 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; + /* If already exists: O_EXCL: error, O_TRUNC: truncate to zero */ + 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; } diff --git a/platform/linux-keystone2/odp_shared_memory.c b/platform/linux-keystone2/odp_shared_memory.c index e595111..69ac34e 100644 --- a/platform/linux-keystone2/odp_shared_memory.c +++ b/platform/linux-keystone2/odp_shared_memory.c @@ -208,8 +208,11 @@ void *_odp_shm_reserve(const char *name, uint64_t size, uint64_t align, return block->addr; } -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) { + (void)flags; + return _odp_shm_reserve(name, size, align, ODP_SHM_CMA); }
Add flags parameter to reserve call. * SW_ONLY: optimize memory usage when HW accelerators are not involved. * PROC: for communication with external (non-ODP) processes in the system Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> --- .../linux-generic/include/api/odp_shared_memory.h | 15 +++++++- platform/linux-generic/odp_shared_memory.c | 45 +++++++++++++++++----- platform/linux-keystone2/odp_shared_memory.c | 5 ++- 3 files changed, 53 insertions(+), 12 deletions(-)