rt-tests v1.9 fails to compile on architectures that aren't x86_64, i386 or PPC64

rt-tests v1.9 fails to compile on architectures that aren't x86_64, i386 or PPC64

[Randy Witt]

With the addition of the oslat test in v1.9, "make" with no arguments will fail 
on any architecture that isn't x86_64, i386 or PPC64. This is due to the #error 
added here 
https://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git/tree/src/oslat/oslat.c?h=v1.9#n72.

Since there is no build time configuration to turn off particular tests, is the 
expected workflow to specify the set of desired tests as make arguments?

In the absence of some sort of configure script, should there be a variable that 
allows for tests to be removed from the set of TARGETS?

Re: rt-tests v1.9 fails to compile on architectures that aren't x86_64, i386 or PPC64

[Peter Xu]

[-- Attachment #1: Type: text/plain, Size: 1375 bytes --]

Hi, Randy,

On Mon, Sep 28, 2020 at 03:14:23PM -0700, Randy Witt wrote:
> With the addition of the oslat test in v1.9, "make" with no arguments will
> fail on any architecture that isn't x86_64, i386 or PPC64. This is due to
> the #error added here https://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git/tree/src/oslat/oslat.c?h=v1.9#n72.
> 
> Since there is no build time configuration to turn off particular tests, is
> the expected workflow to specify the set of desired tests as make arguments?
> 
> In the absence of some sort of configure script, should there be a variable
> that allows for tests to be removed from the set of TARGETS?

I think we can, but instead of maintaining this knowledge both in the build
system and in oslat.c, I'm thinking maybe we can simply do that in oslat.c, let
it build but dump an error message for any unsupported archs.  Patch attached.

Another solution (as mentioned by John offlist) is we implement a common frc()
like fetching the time.  However since the __measure_cpu_hz() will always try
to calculate the cpu HZ, then the HZ could be confusing then.  Considering the
"read the processor clock" operation should still be the ideal implementation
for frc() (and I really doubt whether there'll be real need to run oslat on
other archs...), maybe we can use the simple way for now.

What do you think?

Thanks,

-- 
Peter Xu

[-- Attachment #2: 0001-rt-tests-oslat-Allow-build-for-not-supported-archs.patch --]
[-- Type: text/plain, Size: 1450 bytes --]

From 40eeae690c1560632ac6160ad8056960216a9a43 Mon Sep 17 00:00:00 2001
From: Peter Xu <peterx@redhat.com>
Date: Wed, 30 Sep 2020 11:30:15 -0400
Subject: [PATCH] rt-tests: oslat: Allow build for not supported archs

Now rt-tests won't build for archs other than x86/i386/ppc64 after oslat is
merged.  Instead of failing the build, let's make it pass.  However, whenever
oslat is executed, instead of running the real program, dump an error message,
so that people can try to implement the frc() function for it when there's a
real need for the new arch.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 src/oslat/oslat.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/oslat/oslat.c b/src/oslat/oslat.c
index f1a82f2..a8b6155 100644
--- a/src/oslat/oslat.c
+++ b/src/oslat/oslat.c
@@ -69,7 +69,9 @@ static inline void frc(uint64_t *pval)
 	__asm__ __volatile__("mfspr %0, 268\n" : "=r" (*pval));
 }
 # else
-#  error Need frc() for this platform.
+#  define relax()          do { } while (0)
+#  define frc(x)
+#  define FRC_MISSING
 # endif
 #else
 # error Need to add support for this compiler.
@@ -810,6 +812,12 @@ int main(int argc, char *argv[])
 	int i, n_cores;
 	cpu_set_t cpu_set;
 
+#ifdef FRC_MISSING
+	printf("This architecture is not yet supported. "
+	       "Please implement frc() function first for %s.\n", argv[0]);
+	return 0;
+#endif
+
 	CPU_ZERO(&cpu_set);
 
 	g.app_name = argv[0];
-- 
2.26.2

Re: rt-tests v1.9 fails to compile on architectures that aren't x86_64, i386 or PPC64

[Randy Witt]

On 9/30/20 8:45 AM, Peter Xu wrote:
> Hi, Randy,
> 
> On Mon, Sep 28, 2020 at 03:14:23PM -0700, Randy Witt wrote:
>> With the addition of the oslat test in v1.9, "make" with no arguments will
>> fail on any architecture that isn't x86_64, i386 or PPC64. This is due to
>> the #error added here https://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git/tree/src/oslat/oslat.c?h=v1.9#n72.
>>
>> Since there is no build time configuration to turn off particular tests, is
>> the expected workflow to specify the set of desired tests as make arguments?
>>
>> In the absence of some sort of configure script, should there be a variable
>> that allows for tests to be removed from the set of TARGETS?
> 
> I think we can, but instead of maintaining this knowledge both in the build
> system and in oslat.c, I'm thinking maybe we can simply do that in oslat.c, let
> it build but dump an error message for any unsupported archs.  Patch attached.

Sorry I didn't have time to respond yesterday. I think this patch is a good 
solution. It's much better than having to know the set of compatible tests.

> Another solution (as mentioned by John offlist) is we implement a common frc()
> like fetching the time.  However since the __measure_cpu_hz() will always try
> to calculate the cpu HZ, then the HZ could be confusing then.  Considering the
> "read the processor clock" operation should still be the ideal implementation
> for frc() (and I really doubt whether there'll be real need to run oslat on
> other archs...), maybe we can use the simple way for now.

Would you be willing to help me understand why it wouldn't be useful for other 
archs? I am most likely missing something obvious, and apologize if that's the case.

> What do you think?
> 
> Thanks,
> 

Re: rt-tests v1.9 fails to compile on architectures that aren't x86_64, i386 or PPC64

[Peter Xu]

On Thu, Oct 01, 2020 at 12:00:20PM -0700, Randy Witt wrote:
> > Another solution (as mentioned by John offlist) is we implement a common frc()
> > like fetching the time.  However since the __measure_cpu_hz() will always try
> > to calculate the cpu HZ, then the HZ could be confusing then.  Considering the
> > "read the processor clock" operation should still be the ideal implementation
> > for frc() (and I really doubt whether there'll be real need to run oslat on
> > other archs...), maybe we can use the simple way for now.
> 
> Would you be willing to help me understand why it wouldn't be useful for
> other archs?

Sorry if I confused you... I never meant that. :)

I tried to dump that error message just to make sure when some other arch
wanted to run this program, then people will know what they need to do is
simply to implement the frc() function on that arch for oslat.c.  I just feel
like providing a common implementation for frc() could be slightly confusing
(as mentioned above, e.g. on hz information).  So maybe it's better to wait for
someone to implement that (I believe shouldn't be hard for a genuine RT user
anyways...).

Thanks,

-- 
Peter Xu