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
