On Tue, Sep 15, 2015 at 11:50:16PM +0200, John Kacur wrote:
> On Mon, 31 Aug 2015, Josh Cartwright wrote:
>
> > The condition that numa_on_and_available() checks is impossible to hit,
> > as it's already being checked during process_options().
> >
> > Removal of numa_on_and_available() also has the side effect of removing
> > the following warning during build:
> >
> > src/cyclictest/rt_numa.h:259:15: warning: implicit declaration of function ???numa_available??? [-Wimplicit-function-declaration]
> >
> > Signed-off-by: Josh Cartwright <joshc@xxxxxx>
[..]
> We don't support building without numa libs anymore, although we of
> course support running on machines without numa. Never-the-less I
> created two versions of numa_on_and_available, one for if you build with
> the unsupported NUMA=0 and one for if you build with NUMA=1, which is
> the default.
:(. You risk plenty of embedded folks who don't much care about NUMA.
> I would prefer not to drop this function, since I think it cleanly
> documents the fact that numa_available must be called before any other
> numa library functions are defined.
>
> As Josh Cartwright reported though, there was no need to call it from
> main() since it was already tested in process_options(), so I tested it
> there.
>
> Tested by building with NUMA=0, NUMA=1 and with the non-standard
> -Wimplicit-function-declaration
>
> Reported-by: Signed-off-by: Josh Cartwright <joshc@xxxxxx>
s/Signed-off-by: /
:)
> Signed-off-by: John Kacur <jkacur@xxxxxxxxxx>
> ---
> src/cyclictest/cyclictest.c | 7 ++-----
> src/cyclictest/rt_numa.h | 18 +++++++++++++-----
> 2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 213c527fd714..b3abfcc67407 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -1451,12 +1451,11 @@ static void process_options (int argc, char *argv[], int max_cpus)
> setvbuf(stdout, NULL, _IONBF, 0); break;
> case 'U':
> case OPT_NUMA: /* NUMA testing */
> + numa = 1; /* Turn numa on */
> if (smp)
> fatal("numa and smp options are mutually exclusive\n");
> + numa_on_and_available();
...except numa is always on here. So why check if NUMA is enabled
again? Seems like a waste.
> #ifdef NUMA
> - if (numa_available() == -1)
> - fatal("NUMA functionality not available!");
Perhaps a middle ground should be to keep this check around and define a
static inline numa_available() in the NUMA:=0 case which unconditionally
returns -1.
Thanks for taking a look,
Josh
Attachment:
signature.asc
Description: PGP signature
