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.
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
