On 09/04/2014 02:18 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
Let's use mine. Added shm features are not IPC specific. Also...
+typedef enum {
+ ODP_SHM_THREAD = 1, /**< Memory accessible by threads. */
+ ODP_SHM_PROC = 2, /**< Memory accessible by processes.
+ Will be created if not exist. */
+ ODP_SHM_PROC_NOCREAT = 3, /**< Memory accessible by processes.
+ Has to be created before usage.*/
+} odp_shm_e;
... this exposes too much (OS) details to the application. Application wants to
share data with an entity outside of ODP - no need to specify if the other
entity id a thread or process.
NOCREAT not needed, it's a look up.
It's not standard look up. It also needs to create entry in
odp_shm_tbl. But this thing in platform implementation.
Will think how to move it there.
Maxim.
-Petri
-----Original Message-----
From: ext Maxim Uvarov [mailto:[email protected]]
Sent: Wednesday, September 03, 2014 4:59 PM
To: Savolainen, Petri (NSN - FI/Espoo); [email protected]
Subject: Re: [lng-odp] [PATCH 1/2] Shared memory reserve flag
Perti, can you please take a look at "linux-generic: odp ipc
implementation v4" patch?
We need to define how to pass flags to odp_shm_reserve. Or we use mine
variant or yours.
Thank you,
Maxim.
On 09/03/2014 03:56 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
-----Original Message-----
From: ext Maxim Uvarov [mailto:[email protected]]
Sent: Wednesday, September 03, 2014 1:37 PM
To: Savolainen, Petri (NSN - FI/Espoo); [email protected]
Subject: Re: [lng-odp] [PATCH 1/2] Shared memory reserve flag
On 09/03/2014 11:36 AM, Savolainen, Petri (NSN - FI/Espoo) wrote:
-----Original Message-----
From: [email protected] [mailto:lng-odp-
[email protected]] On Behalf Of ext Maxim Uvarov
Sent: Tuesday, September 02, 2014 6:43 PM
To: [email protected]
Subject: Re: [lng-odp] [PATCH 1/2] Shared memory reserve flag
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.
Didn't realize that. This is not only for IPC, but the shm flag was on
TODO list from last sprint to help Taras to optimize shared memory
allocation (flag SW only vs. SW+HW accelerator access).
Yes, it's patch for reserving memory. Because it's general api I need
the same for IPC.
My patch was here:
https://patches.linaro.org/35037/
1. One difference is KERNEL flag. How if KERNEL flag will be used? Is
there is specific need for that? What will happen if
app will set this flag but flag is not supported by platform.
I was thinking to use that to flag memory that will be shared with a
kernel driver (but not with external processes). It can be dropped now
if
not needed. Flags are additional information to the implementation
what's
the intended memory usage, so that they can optimize how HW resource
are
used.
Lets do that. Drop this flag for now. And if Taras needs this flag he
will add it with KS2 implementation.
2. If public API changed then you need to fix all existence
platforms,
to not break their compilation/work.
I changed the reference API and implementation (== linux-generic).
Maintainers of other implementations are free to decide when (and more
importantly who) to follow. We cannot keep all implementations in 100%
sync. It's better to break a build (if they are reusing linux-generic
code), than try to fix all implementations at once (with potentially
untested, buggy code).
You should not break other platforms when changing api for
linux-generic. You added new flag to odp_shm_reserve() for example to
example/generator/odp_generator.c but ks2 and dpdk still have on one
argument less. So their compilation will fail. And this things have to
be avoided. We also decided that git history
have to be 'git bisect' friendly. That rule is also acceptable on
other arches.
Maybe we need to start versioning the API and test apps, so that it's
possible to have different implementations out-of-sync. Otherwise after
couple of more platforms, it's pretty impossible to modify the (reference)
API if all implementations must be modified at once. Who can master
HW/SDK/implementation details of all the platforms? Also it would require
pretty tight lock-step development (not very agile).
A new argument is trivial to add... but if implementation is missing,
test apps would fail still... if you implement you should test it... does
everybody have access to all HW platforms...
-Petri
3. What I don't like in current odp_shm_reserve() is that it's
difficult 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
is
set. 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.
This goes back to errno discussion... I think it should return NULL
and
set errno. Errno/abort()/etc changes will be done in sync with all
other
APIs.
errno is ok for me.
Maxim.
-Petri
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
