Re: [PATCH] eal: default to one memory channel if not specified

[Panu Matilainen <pmatilai@redhat.com>]

On 10/14/2015 02:45 PM, David Marchand wrote:
> Hello Panu,
>
> On Wed, Oct 14, 2015 at 12:22 PM, Panu Matilainen <pmatilai@redhat.com
> <mailto:pmatilai@redhat.com>> wrote:
>
>     Obtaining the correct value, especially from a running system, can
>     be anything from difficult to plain impossible.  Since the value is
>     merely an optimization and does not affect functionality otherwise,
>     its pointless to force such a guess on users initially, such things
>     belong to performance tuning phase.
>
>     Signed-off-by: Panu Matilainen <pmatilai@redhat.com
>     <mailto:pmatilai@redhat.com>>
>     ---
>       lib/librte_eal/common/eal_common_options.c | 10 ++--------
>       1 file changed, 2 insertions(+), 8 deletions(-)
>
>     diff --git a/lib/librte_eal/common/eal_common_options.c
>     b/lib/librte_eal/common/eal_common_options.c
>     index 1f459ac..28f10a2 100644
>     --- a/lib/librte_eal/common/eal_common_options.c
>     +++ b/lib/librte_eal/common/eal_common_options.c
>     @@ -104,7 +104,7 @@ eal_reset_internal_config(struct internal_config
>     *internal_cfg)
>
>              internal_cfg->memory = 0;
>              internal_cfg->force_nrank = 0;
>     -       internal_cfg->force_nchannel = 0;
>     +       internal_cfg->force_nchannel = 1;
>
>
> Well, not too sure about this default value.
>
> - mempool code is already checking for the 0 value.

Yeah, I noticed it already handles the zero case.

> - API already tells for rte_memory_get_nchannel() :
>   * @return
>   *   The number of memory channels on the system. The value is 0 if
> unknown
>   *   or not the same on all devices.

...but missed this one, and thought it'd be "safer" to return some 
non-zero value since callers might be expecting it to be a valid -n value.

>
> So, I would let it 0.

Right, so just drop the default value, reword commit message accordingly 
and resend. Will do unless there are other objections.

	- Panu -