[lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
odph_linux_process_fork is not used by any ODP example or test, it is also
untested by the helper test suite.
odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
just delete this, the impact if there are any users is very small.
Signed-off-by: Mike Holmes
---
helper/include/odp/helper/linux.h | 18 --
helper/linux.c| 10 --
2 files changed, 28 deletions(-)
diff --git a/helper/include/odp/helper/linux.h
b/helper/include/odp/helper/linux.h
index 7a6504f..a68f269 100644
--- a/helper/include/odp/helper/linux.h
+++ b/helper/include/odp/helper/linux.h
@@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
*pthread_tbl,
*/
void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
-
-/**
- * Fork a process
- *
- * Forks and sets CPU affinity for the child process. Ignores 'start' and 'arg'
- * thread parameters.
- *
- * @param[out] procPointer to process state info (for output)
- * @param cpu Destination CPU for the child process
- * @param thr_params Linux helper thread parameters
- *
- * @return On success: 1 for the parent, 0 for the child
- * On failure: -1 for the parent, -2 for the child
- */
-int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
- const odph_linux_thr_params_t *thr_params);
-
-
/**
* Fork a number of processes
*
diff --git a/helper/linux.c b/helper/linux.c
index 24e243b..a181322 100644
--- a/helper/linux.c
+++ b/helper/linux.c
@@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
*proc_tbl,
return 1;
}
-int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
- const odph_linux_thr_params_t *thr_params)
-{
- odp_cpumask_t mask;
-
- odp_cpumask_zero(&mask);
- odp_cpumask_set(&mask, cpu);
- return odph_linux_process_fork_n(proc, &mask, thr_params);
-}
-
int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
{
pid_t pid;
--
2.7.4
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
On 2 May 2016 at 13:47, Mike Holmes wrote:
> odph_linux_process_fork is not used by any ODP example or test, it is also
> untested by the helper test suite.
>
Just for the note, we use this api currently in our nginx_ofp app.
>
> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
> just delete this, the impact if there are any users is very small.
>
Agree, need a minor update in the app.
/Krishna
>
> Signed-off-by: Mike Holmes
> ---
> helper/include/odp/helper/linux.h | 18 --
> helper/linux.c| 10 --
> 2 files changed, 28 deletions(-)
>
> diff --git a/helper/include/odp/helper/linux.h
> b/helper/include/odp/helper/linux.h
> index 7a6504f..a68f269 100644
> --- a/helper/include/odp/helper/linux.h
> +++ b/helper/include/odp/helper/linux.h
> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
> *pthread_tbl,
> */
> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
>
> -
> -/**
> - * Fork a process
> - *
> - * Forks and sets CPU affinity for the child process. Ignores 'start' and
> 'arg'
> - * thread parameters.
> - *
> - * @param[out] procPointer to process state info (for output)
> - * @param cpu Destination CPU for the child process
> - * @param thr_params Linux helper thread parameters
> - *
> - * @return On success: 1 for the parent, 0 for the child
> - * On failure: -1 for the parent, -2 for the child
> - */
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params);
> -
> -
> /**
> * Fork a number of processes
> *
> diff --git a/helper/linux.c b/helper/linux.c
> index 24e243b..a181322 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
> *proc_tbl,
> return 1;
> }
>
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params)
> -{
> - odp_cpumask_t mask;
> -
> - odp_cpumask_zero(&mask);
> - odp_cpumask_set(&mask, cpu);
> - return odph_linux_process_fork_n(proc, &mask, thr_params);
> -}
> -
> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
> {
> pid_t pid;
> --
> 2.7.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
Please don't do that: It will just create an extra rebase on my "running
things in process mode " patch series which replaces all this (and
therefore delete these also)
Christophe.
On 2 May 2016 at 14:01, Krishna Garapati
wrote:
>
>
> On 2 May 2016 at 13:47, Mike Holmes wrote:
>
>> odph_linux_process_fork is not used by any ODP example or test, it is also
>> untested by the helper test suite.
>>
> Just for the note, we use this api currently in our nginx_ofp app.
>
>>
>> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
>> just delete this, the impact if there are any users is very small.
>>
> Agree, need a minor update in the app.
>
> /Krishna
>
>>
>> Signed-off-by: Mike Holmes
>> ---
>> helper/include/odp/helper/linux.h | 18 --
>> helper/linux.c| 10 --
>> 2 files changed, 28 deletions(-)
>>
>> diff --git a/helper/include/odp/helper/linux.h
>> b/helper/include/odp/helper/linux.h
>> index 7a6504f..a68f269 100644
>> --- a/helper/include/odp/helper/linux.h
>> +++ b/helper/include/odp/helper/linux.h
>> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
>> *pthread_tbl,
>> */
>> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
>>
>> -
>> -/**
>> - * Fork a process
>> - *
>> - * Forks and sets CPU affinity for the child process. Ignores 'start'
>> and 'arg'
>> - * thread parameters.
>> - *
>> - * @param[out] procPointer to process state info (for output)
>> - * @param cpu Destination CPU for the child process
>> - * @param thr_params Linux helper thread parameters
>> - *
>> - * @return On success: 1 for the parent, 0 for the child
>> - * On failure: -1 for the parent, -2 for the child
>> - */
>> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
>> - const odph_linux_thr_params_t *thr_params);
>> -
>> -
>> /**
>> * Fork a number of processes
>> *
>> diff --git a/helper/linux.c b/helper/linux.c
>> index 24e243b..a181322 100644
>> --- a/helper/linux.c
>> +++ b/helper/linux.c
>> @@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
>> *proc_tbl,
>> return 1;
>> }
>>
>> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
>> - const odph_linux_thr_params_t *thr_params)
>> -{
>> - odp_cpumask_t mask;
>> -
>> - odp_cpumask_zero(&mask);
>> - odp_cpumask_set(&mask, cpu);
>> - return odph_linux_process_fork_n(proc, &mask, thr_params);
>> -}
>> -
>> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
>> {
>> pid_t pid;
>> --
>> 2.7.4
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
On 3 May 2016 at 07:32, Christophe Milard
wrote:
> Please don't do that: It will just create an extra rebase on my "running
> things in process mode " patch series which replaces all this (and
> therefore delete these also)
>
>
We need a test in helper for it then :) we dont want the full Monarch API
to go out with an untested API from the test suite perspective, Krishna,
Christoph any chance you can add one since you both care about the API?
Mike
> Christophe.
>
> On 2 May 2016 at 14:01, Krishna Garapati
> wrote:
>
>>
>>
>> On 2 May 2016 at 13:47, Mike Holmes wrote:
>>
>>> odph_linux_process_fork is not used by any ODP example or test, it is
>>> also
>>> untested by the helper test suite.
>>>
>> Just for the note, we use this api currently in our nginx_ofp app.
>>
>>>
>>> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
>>> just delete this, the impact if there are any users is very small.
>>>
>> Agree, need a minor update in the app.
>>
>> /Krishna
>>
>>>
>>> Signed-off-by: Mike Holmes
>>> ---
>>> helper/include/odp/helper/linux.h | 18 --
>>> helper/linux.c| 10 --
>>> 2 files changed, 28 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/linux.h
>>> b/helper/include/odp/helper/linux.h
>>> index 7a6504f..a68f269 100644
>>> --- a/helper/include/odp/helper/linux.h
>>> +++ b/helper/include/odp/helper/linux.h
>>> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
>>> *pthread_tbl,
>>> */
>>> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
>>>
>>> -
>>> -/**
>>> - * Fork a process
>>> - *
>>> - * Forks and sets CPU affinity for the child process. Ignores 'start'
>>> and 'arg'
>>> - * thread parameters.
>>> - *
>>> - * @param[out] procPointer to process state info (for output)
>>> - * @param cpu Destination CPU for the child process
>>> - * @param thr_params Linux helper thread parameters
>>> - *
>>> - * @return On success: 1 for the parent, 0 for the child
>>> - * On failure: -1 for the parent, -2 for the child
>>> - */
>>> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
>>> - const odph_linux_thr_params_t *thr_params);
>>> -
>>> -
>>> /**
>>> * Fork a number of processes
>>> *
>>> diff --git a/helper/linux.c b/helper/linux.c
>>> index 24e243b..a181322 100644
>>> --- a/helper/linux.c
>>> +++ b/helper/linux.c
>>> @@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
>>> *proc_tbl,
>>> return 1;
>>> }
>>>
>>> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
>>> - const odph_linux_thr_params_t *thr_params)
>>> -{
>>> - odp_cpumask_t mask;
>>> -
>>> - odp_cpumask_zero(&mask);
>>> - odp_cpumask_set(&mask, cpu);
>>> - return odph_linux_process_fork_n(proc, &mask, thr_params);
>>> -}
>>> -
>>> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
>>> {
>>> pid_t pid;
>>> --
>>> 2.7.4
>>>
>>> ___
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>
--
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
There is a test for these new functions in the patch series. Hope this is
what you meant by "it".
Christophe
On 3 May 2016 at 13:35, Mike Holmes wrote:
>
>
> On 3 May 2016 at 07:32, Christophe Milard
> wrote:
>
>> Please don't do that: It will just create an extra rebase on my "running
>> things in process mode " patch series which replaces all this (and
>> therefore delete these also)
>>
>>
> We need a test in helper for it then :) we dont want the full Monarch API
> to go out with an untested API from the test suite perspective, Krishna,
> Christoph any chance you can add one since you both care about the API?
>
> Mike
>
>
>
>> Christophe.
>>
>> On 2 May 2016 at 14:01, Krishna Garapati > > wrote:
>>
>>>
>>>
>>> On 2 May 2016 at 13:47, Mike Holmes wrote:
>>>
odph_linux_process_fork is not used by any ODP example or test, it is
also
untested by the helper test suite.
>>> Just for the note, we use this api currently in our nginx_ofp app.
>>>
odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
just delete this, the impact if there are any users is very small.
>>> Agree, need a minor update in the app.
>>>
>>> /Krishna
>>>
Signed-off-by: Mike Holmes
---
helper/include/odp/helper/linux.h | 18 --
helper/linux.c| 10 --
2 files changed, 28 deletions(-)
diff --git a/helper/include/odp/helper/linux.h
b/helper/include/odp/helper/linux.h
index 7a6504f..a68f269 100644
--- a/helper/include/odp/helper/linux.h
+++ b/helper/include/odp/helper/linux.h
@@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
*pthread_tbl,
*/
void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int
num);
-
-/**
- * Fork a process
- *
- * Forks and sets CPU affinity for the child process. Ignores 'start'
and 'arg'
- * thread parameters.
- *
- * @param[out] procPointer to process state info (for output)
- * @param cpu Destination CPU for the child process
- * @param thr_params Linux helper thread parameters
- *
- * @return On success: 1 for the parent, 0 for the child
- * On failure: -1 for the parent, -2 for the child
- */
-int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
- const odph_linux_thr_params_t *thr_params);
-
-
/**
* Fork a number of processes
*
diff --git a/helper/linux.c b/helper/linux.c
index 24e243b..a181322 100644
--- a/helper/linux.c
+++ b/helper/linux.c
@@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
*proc_tbl,
return 1;
}
-int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
- const odph_linux_thr_params_t *thr_params)
-{
- odp_cpumask_t mask;
-
- odp_cpumask_zero(&mask);
- odp_cpumask_set(&mask, cpu);
- return odph_linux_process_fork_n(proc, &mask, thr_params);
-}
-
int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
{
pid_t pid;
--
2.7.4
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>>
>>> ___
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>>
>>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
On 3 May 2016 at 07:39, Christophe Milard
wrote:
> There is a test for these new functions in the patch series. Hope this is
> what you meant by "it".
>
OK - if you will be adding a test that is perfect :), I am just going on
the current state
http://docs.opendataplane.org/snapshots/master/linux-generic-helper-lcov-html/helper/linux.c.func-sort-c.html
>
> Christophe
>
> On 3 May 2016 at 13:35, Mike Holmes wrote:
>
>>
>>
>> On 3 May 2016 at 07:32, Christophe Milard
>> wrote:
>>
>>> Please don't do that: It will just create an extra rebase on my "running
>>> things in process mode " patch series which replaces all this (and
>>> therefore delete these also)
>>>
>>>
>> We need a test in helper for it then :) we dont want the full Monarch API
>> to go out with an untested API from the test suite perspective, Krishna,
>> Christoph any chance you can add one since you both care about the API?
>>
>> Mike
>>
>>
>>
>>> Christophe.
>>>
>>> On 2 May 2016 at 14:01, Krishna Garapati <
>>> balakrishna.garap...@linaro.org> wrote:
>>>
On 2 May 2016 at 13:47, Mike Holmes wrote:
> odph_linux_process_fork is not used by any ODP example or test, it is
> also
> untested by the helper test suite.
>
Just for the note, we use this api currently in our nginx_ofp app.
>
> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
> just delete this, the impact if there are any users is very small.
>
Agree, need a minor update in the app.
/Krishna
>
> Signed-off-by: Mike Holmes
> ---
> helper/include/odp/helper/linux.h | 18 --
> helper/linux.c| 10 --
> 2 files changed, 28 deletions(-)
>
> diff --git a/helper/include/odp/helper/linux.h
> b/helper/include/odp/helper/linux.h
> index 7a6504f..a68f269 100644
> --- a/helper/include/odp/helper/linux.h
> +++ b/helper/include/odp/helper/linux.h
> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
> *pthread_tbl,
> */
> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int
> num);
>
> -
> -/**
> - * Fork a process
> - *
> - * Forks and sets CPU affinity for the child process. Ignores 'start'
> and 'arg'
> - * thread parameters.
> - *
> - * @param[out] procPointer to process state info (for output)
> - * @param cpu Destination CPU for the child process
> - * @param thr_params Linux helper thread parameters
> - *
> - * @return On success: 1 for the parent, 0 for the child
> - * On failure: -1 for the parent, -2 for the child
> - */
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params);
> -
> -
> /**
> * Fork a number of processes
> *
> diff --git a/helper/linux.c b/helper/linux.c
> index 24e243b..a181322 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -183,16 +183,6 @@ int
> odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
> return 1;
> }
>
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params)
> -{
> - odp_cpumask_t mask;
> -
> - odp_cpumask_zero(&mask);
> - odp_cpumask_set(&mask, cpu);
> - return odph_linux_process_fork_n(proc, &mask, thr_params);
> -}
> -
> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
> {
> pid_t pid;
> --
> 2.7.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>> --
>> Mike Holmes
>> Technical Manager - Linaro Networking Group
>> Linaro.org
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
Direct pthreads and process helpers should not be removed. Although our
validation tests and examples need to be thread type agnostic, there are a
bunch of other applications that will want to use either Linux pthread or
process (not opaque threads).
Tests should be added for direct pthread/process helpers, instead of removing
those.
-Petri
> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Mike
> Holmes
> Sent: Monday, May 02, 2016 2:47 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork
> API
>
> odph_linux_process_fork is not used by any ODP example or test, it is also
> untested by the helper test suite.
>
> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
> just delete this, the impact if there are any users is very small.
>
> Signed-off-by: Mike Holmes
> ---
> helper/include/odp/helper/linux.h | 18 --
> helper/linux.c| 10 --
> 2 files changed, 28 deletions(-)
>
> diff --git a/helper/include/odp/helper/linux.h
> b/helper/include/odp/helper/linux.h
> index 7a6504f..a68f269 100644
> --- a/helper/include/odp/helper/linux.h
> +++ b/helper/include/odp/helper/linux.h
> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
> *pthread_tbl,
> */
> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
>
> -
> -/**
> - * Fork a process
> - *
> - * Forks and sets CPU affinity for the child process. Ignores 'start' and
> 'arg'
> - * thread parameters.
> - *
> - * @param[out] procPointer to process state info (for output)
> - * @param cpu Destination CPU for the child process
> - * @param thr_params Linux helper thread parameters
> - *
> - * @return On success: 1 for the parent, 0 for the child
> - * On failure: -1 for the parent, -2 for the child
> - */
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params);
> -
> -
> /**
> * Fork a number of processes
> *
> diff --git a/helper/linux.c b/helper/linux.c
> index 24e243b..a181322 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
> *proc_tbl,
> return 1;
> }
>
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params)
> -{
> - odp_cpumask_t mask;
> -
> - odp_cpumask_zero(&mask);
> - odp_cpumask_set(&mask, cpu);
> - return odph_linux_process_fork_n(proc, &mask, thr_params);
> -}
> -
> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
> {
> pid_t pid;
> --
> 2.7.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
I am not sure I really agree with that, Petri:
as of today, none of the written validation tests, performance tests nor
example needed to do the difference: of course, that does not mean it will
never be needed, but I don't think we should be adding (nor keeping) unused
code promoting a behaviour we do not wish: If we want to be odpthread type
agnostic as much as possible should we really provide means to go the other
way in helpers?.
And if one piece of code does need to do the difference in the future,
shouldn't that specific code call pthread/fork directely then? should the
helper provide a wrapper for this specific case, really?
Christophe
On 6 May 2016 at 11:09, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:
> Direct pthreads and process helpers should not be removed. Although our
> validation tests and examples need to be thread type agnostic, there are a
> bunch of other applications that will want to use either Linux pthread or
> process (not opaque threads).
>
> Tests should be added for direct pthread/process helpers, instead of
> removing those.
>
> -Petri
>
> > -Original Message-
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> Mike
> > Holmes
> > Sent: Monday, May 02, 2016 2:47 PM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork
> > API
> >
> > odph_linux_process_fork is not used by any ODP example or test, it is
> also
> > untested by the helper test suite.
> >
> > odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
> > just delete this, the impact if there are any users is very small.
> >
> > Signed-off-by: Mike Holmes
> > ---
> > helper/include/odp/helper/linux.h | 18 --
> > helper/linux.c| 10 --
> > 2 files changed, 28 deletions(-)
> >
> > diff --git a/helper/include/odp/helper/linux.h
> > b/helper/include/odp/helper/linux.h
> > index 7a6504f..a68f269 100644
> > --- a/helper/include/odp/helper/linux.h
> > +++ b/helper/include/odp/helper/linux.h
> > @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
> > *pthread_tbl,
> > */
> > void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
> >
> > -
> > -/**
> > - * Fork a process
> > - *
> > - * Forks and sets CPU affinity for the child process. Ignores 'start'
> and
> > 'arg'
> > - * thread parameters.
> > - *
> > - * @param[out] procPointer to process state info (for output)
> > - * @param cpu Destination CPU for the child process
> > - * @param thr_params Linux helper thread parameters
> > - *
> > - * @return On success: 1 for the parent, 0 for the child
> > - * On failure: -1 for the parent, -2 for the child
> > - */
> > -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> > - const odph_linux_thr_params_t *thr_params);
> > -
> > -
> > /**
> > * Fork a number of processes
> > *
> > diff --git a/helper/linux.c b/helper/linux.c
> > index 24e243b..a181322 100644
> > --- a/helper/linux.c
> > +++ b/helper/linux.c
> > @@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
> > *proc_tbl,
> > return 1;
> > }
> >
> > -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> > - const odph_linux_thr_params_t *thr_params)
> > -{
> > - odp_cpumask_t mask;
> > -
> > - odp_cpumask_zero(&mask);
> > - odp_cpumask_set(&mask, cpu);
> > - return odph_linux_process_fork_n(proc, &mask, thr_params);
> > -}
> > -
> > int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
> > {
> > pid_t pid;
> > --
> > 2.7.4
> >
> > ___
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/lng-odp
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
The test exists already in the "running things in process mode" patch
series (reviewed by Brian). Hopefully I do test what I wrote there. However
his patch series does not add any test for the fork*() helper functions as
these functions are removed in the last patch of the series.
I still think these should be removed (see separate answer to Petri).
Christophe.
On 3 May 2016 at 14:27, Mike Holmes wrote:
>
>
> On 3 May 2016 at 07:39, Christophe Milard
> wrote:
>
>> There is a test for these new functions in the patch series. Hope this is
>> what you meant by "it".
>>
>
> OK - if you will be adding a test that is perfect :), I am just going on
> the current state
> http://docs.opendataplane.org/snapshots/master/linux-generic-helper-lcov-html/helper/linux.c.func-sort-c.html
>
>
>>
>> Christophe
>>
>> On 3 May 2016 at 13:35, Mike Holmes wrote:
>>
>>>
>>>
>>> On 3 May 2016 at 07:32, Christophe Milard
>>> wrote:
>>>
Please don't do that: It will just create an extra rebase on my
"running things in process mode " patch series which replaces all this (and
therefore delete these also)
>>> We need a test in helper for it then :) we dont want the full Monarch
>>> API to go out with an untested API from the test suite perspective,
>>> Krishna, Christoph any chance you can add one since you both care about the
>>> API?
>>>
>>> Mike
>>>
>>>
>>>
Christophe.
On 2 May 2016 at 14:01, Krishna Garapati <
balakrishna.garap...@linaro.org> wrote:
>
>
> On 2 May 2016 at 13:47, Mike Holmes wrote:
>
>> odph_linux_process_fork is not used by any ODP example or test, it is
>> also
>> untested by the helper test suite.
>>
> Just for the note, we use this api currently in our nginx_ofp app.
>
>>
>> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
>> just delete this, the impact if there are any users is very small.
>>
> Agree, need a minor update in the app.
>
> /Krishna
>
>>
>> Signed-off-by: Mike Holmes
>> ---
>> helper/include/odp/helper/linux.h | 18 --
>> helper/linux.c| 10 --
>> 2 files changed, 28 deletions(-)
>>
>> diff --git a/helper/include/odp/helper/linux.h
>> b/helper/include/odp/helper/linux.h
>> index 7a6504f..a68f269 100644
>> --- a/helper/include/odp/helper/linux.h
>> +++ b/helper/include/odp/helper/linux.h
>> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
>> *pthread_tbl,
>> */
>> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int
>> num);
>>
>> -
>> -/**
>> - * Fork a process
>> - *
>> - * Forks and sets CPU affinity for the child process. Ignores
>> 'start' and 'arg'
>> - * thread parameters.
>> - *
>> - * @param[out] procPointer to process state info (for output)
>> - * @param cpu Destination CPU for the child process
>> - * @param thr_params Linux helper thread parameters
>> - *
>> - * @return On success: 1 for the parent, 0 for the child
>> - * On failure: -1 for the parent, -2 for the child
>> - */
>> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
>> - const odph_linux_thr_params_t
>> *thr_params);
>> -
>> -
>> /**
>> * Fork a number of processes
>> *
>> diff --git a/helper/linux.c b/helper/linux.c
>> index 24e243b..a181322 100644
>> --- a/helper/linux.c
>> +++ b/helper/linux.c
>> @@ -183,16 +183,6 @@ int
>> odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
>> return 1;
>> }
>>
>> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
>> - const odph_linux_thr_params_t *thr_params)
>> -{
>> - odp_cpumask_t mask;
>> -
>> - odp_cpumask_zero(&mask);
>> - odp_cpumask_set(&mask, cpu);
>> - return odph_linux_process_fork_n(proc, &mask, thr_params);
>> -}
>> -
>> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int
>> num)
>> {
>> pid_t pid;
>> --
>> 2.7.4
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>>>
>>>
>>> --
>>> Mike Holmes
>>> Technical Manager - Linaro Networking Group
>>> Linaro.org
Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
It’s the other applications (like trex, etc … old and new external
applications) that will need to explicitly create N pthreads or M processes.
True - as with any helper function - an application can do the same sequence of
system and ODP call by itself, but it’s easier with helpers. With helpers, we
demonstrate how an application should create N pthreads or M processes (when
needed). Our own tests would be agnostic to the thread type and use the opaque
thread helper. If helpers offer only the opaque type, we make ODP harder to use
for all external apps.
-Petri
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Christophe
Milard
Sent: Monday, May 09, 2016 9:21 AM
To: Savolainen, Petri (Nokia - FI/Espoo)
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork API
I am not sure I really agree with that, Petri:
as of today, none of the written validation tests, performance tests nor
example needed to do the difference: of course, that does not mean it will
never be needed, but I don't think we should be adding (nor keeping) unused
code promoting a behaviour we do not wish: If we want to be odpthread type
agnostic as much as possible should we really provide means to go the other way
in helpers?.
And if one piece of code does need to do the difference in the future,
shouldn't that specific code call pthread/fork directely then? should the
helper provide a wrapper for this specific case, really?
Christophe
On 6 May 2016 at 11:09, Savolainen, Petri (Nokia - FI/Espoo)
mailto:petri.savolai...@nokia.com>> wrote:
Direct pthreads and process helpers should not be removed. Although our
validation tests and examples need to be thread type agnostic, there are a
bunch of other applications that will want to use either Linux pthread or
process (not opaque threads).
Tests should be added for direct pthread/process helpers, instead of removing
those.
-Petri
> -Original Message-
> From: lng-odp
> [mailto:lng-odp-boun...@lists.linaro.org<mailto:lng-odp-boun...@lists.linaro.org>]
> On Behalf Of Mike
> Holmes
> Sent: Monday, May 02, 2016 2:47 PM
> To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>
> Subject: [lng-odp] [PATCH] helper: remove unused odph_linux_process_fork
> API
>
> odph_linux_process_fork is not used by any ODP example or test, it is also
> untested by the helper test suite.
>
> odph_linux_process_fork is a wrapper for odph_linux_process_fork_n so
> just delete this, the impact if there are any users is very small.
>
> Signed-off-by: Mike Holmes
> mailto:mike.hol...@linaro.org>>
> ---
> helper/include/odp/helper/linux.h | 18 --
> helper/linux.c| 10 --
> 2 files changed, 28 deletions(-)
>
> diff --git a/helper/include/odp/helper/linux.h
> b/helper/include/odp/helper/linux.h
> index 7a6504f..a68f269 100644
> --- a/helper/include/odp/helper/linux.h
> +++ b/helper/include/odp/helper/linux.h
> @@ -80,24 +80,6 @@ int odph_linux_pthread_create(odph_linux_pthread_t
> *pthread_tbl,
> */
> void odph_linux_pthread_join(odph_linux_pthread_t *thread_tbl, int num);
>
> -
> -/**
> - * Fork a process
> - *
> - * Forks and sets CPU affinity for the child process. Ignores 'start' and
> 'arg'
> - * thread parameters.
> - *
> - * @param[out] procPointer to process state info (for output)
> - * @param cpu Destination CPU for the child process
> - * @param thr_params Linux helper thread parameters
> - *
> - * @return On success: 1 for the parent, 0 for the child
> - * On failure: -1 for the parent, -2 for the child
> - */
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params);
> -
> -
> /**
> * Fork a number of processes
> *
> diff --git a/helper/linux.c b/helper/linux.c
> index 24e243b..a181322 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -183,16 +183,6 @@ int odph_linux_process_fork_n(odph_linux_process_t
> *proc_tbl,
> return 1;
> }
>
> -int odph_linux_process_fork(odph_linux_process_t *proc, int cpu,
> - const odph_linux_thr_params_t *thr_params)
> -{
> - odp_cpumask_t mask;
> -
> - odp_cpumask_zero(&mask);
> - odp_cpumask_set(&mask, cpu);
> - return odph_linux_process_fork_n(proc, &mask, thr_params);
> -}
> -
> int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
> {
> pid_t pid;
> --
> 2.7.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>
> htt
