Re: [Fuego] [PATCH] Benchmark.lmbench2: fix lmbench compile issue

<[email protected]> Thu, 28 Oct 2021 08:42:47 +0000
Newsgroups dev.linux.lists.fuego
Message-ID <OSYPR01MB5542A8FEB19E106C76967BFBA4869@OSYPR01MB5542.jpnprd01.prod.outlook.com>
Thank you for applying the changes.

Please see comments below.

>-----Original Message-----
>From: [email protected] <[email protected]>
>Sent: 27 October 2021 04:32
>To: pyla venkata(TSIP) <[email protected]>
>Cc: sangorrin daniel(サンゴリン ダニエル □SWC◯ACT)
><[email protected]>; [email protected]; dinesh
>kumar(TSIP) <[email protected]>
>Subject: RE: [PATCH] Benchmark.lmbench2: fix lmbench compile issue
>
>See comments below.
>
>> -----Original Message-----
>> From: [email protected] <[email protected]>
>>
>> From: venkata pyla <[email protected]>
>>
>> The test is failing in fuego buster during build phase, it is due to
>> the llseek function is no more available from glibc version 2.28,
>> instead use the lseek64 function.
>>
>> ```
>> /usr/bin/ld: /tmp/ccUq4xrV.o: in function `seekto':
>> disk.c:(.text+0x2f): undefined reference to `llseek'
>> collect2: error: ld returned 1 exit status ``` created a patch for
>> lmbench3 source to fix the above issue, and applied during build
>> phase.
>
>Nice description!  It's very clear what you are fixing and why.
>
>I was worried that lseek64 might not be available for some of my older boards
>and build environments.  However, according to the man page, it was introduced
>in glibc version 2.1 The older version of glibc on any of my boards is 2.23, so it
>looks like I'm OK.

Yes, you are right, this fix should not give problem for the fuego supported versions (stretch, buster and henceforth)

>
>
>>
>> Signed-off-by: venkata pyla <[email protected]>
>> ---
>>  .../fix-undefined-reference-llseek.patch      | 24 +++++++++++++++++++
>>  tests/Benchmark.lmbench2/fuego_test.sh        |  5 ++++
>>  2 files changed, 29 insertions(+)
>>  create mode 100755
>> tests/Benchmark.lmbench2/fix-undefined-reference-llseek.patch
>>
>> diff --git
>> a/tests/Benchmark.lmbench2/fix-undefined-reference-llseek.patch
>> b/tests/Benchmark.lmbench2/fix-undefined-reference-
>> llseek.patch
>> new file mode 100755
>> index 0000000..99070f8
>> --- /dev/null
>> +++ b/tests/Benchmark.lmbench2/fix-undefined-reference-llseek.patch
>> @@ -0,0 +1,24 @@
>> +--- disk.c	2021-08-02 23:59:04.976884974 +0530
>> ++++ disk_new.c	2021-08-03 00:01:11.952696624 +0530
>> +@@ -7,6 +7,10 @@
>> +  * Bits of this are derived from work by Ethan Solomita.
>> +  */
>> +
>> ++#ifdef __linux__
>> ++#define _LARGEFILE64_SOURCE
>> ++#endif
>> ++
>> + #include	<stdio.h>
>> + #include	<sys/types.h>
>> + #include	<unistd.h>
>> +@@ -289,9 +293,7 @@
>> + seekto(int fd, uint64 off)
>> + {
>> + #ifdef	__linux__
>> +-	extern	loff_t llseek(int, loff_t, int);
>> +-
>> +-	if (llseek(fd, (loff_t)off, SEEK_SET) == (loff_t)-1) {
>> ++	if (lseek64(fd, (off64_t)off, SEEK_SET) == (off64_t)-1) {
>> + 		return(-1);
>> + 	}
>> + 	return (0);
>> diff --git a/tests/Benchmark.lmbench2/fuego_test.sh
>> b/tests/Benchmark.lmbench2/fuego_test.sh
>> index 9bdf0d2..d07606c 100755
>> --- a/tests/Benchmark.lmbench2/fuego_test.sh
>> +++ b/tests/Benchmark.lmbench2/fuego_test.sh
>> @@ -9,6 +9,7 @@ function test_build {
>>     patch -p0 < $TEST_HOME/lmbench3.mem64.patch
>>     cd ../src
>>     patch -p0 < $TEST_HOME/bench.h.patch
>> +   patch -p0 < $TEST_HOME/fix-undefined-reference-llseek.patch
>>     cd ..
>>     CFLAGS+=" -g -O"
>>     export OS=$PREFIX
>> @@ -25,7 +26,11 @@ function test_run {
>>     if [ -n "$PREFIX" ] ; then
>>        LMBENCH_OS=`ls ./bin`
>>     else
>> +      LMBENCH_OS=`ls ./bin`
>> +      echo "lmbench_os: $LMBENCH_OS"
>> +      echo "curr path: $(pwd)"
>>        LMBENCH_OS=$(scripts/os)
>> +      echo "lmbench_os: $LMBENCH_OS"
>
>Nice catch.  This `ls ./bin` construct is IMHO quite weird.
>

Sorry for my mistake, I have no intention to change this, I forgot to remove these debugging information while sending the patch.
I will send a patch to remove this unnecessary debugging information.

>>     fi
>>     safe_cmd "rm -rf $BOARD_TESTDIR/fuego.$TESTDIR/results"
>>     safe_cmd "cd $BOARD_TESTDIR/fuego.$TESTDIR/scripts;
>OS=$LMBENCH_OS ./config-run"
>> --
>> 2.20.1
>>
>
>Thanks for this fix.  This is the type of thing that is a real pain to hunt down and
>fix, and is nice to keep the test from bit-rotting.  So I appreciate this fix a lot.
>
>I found a couple of other issues with the lmbench build and results parsing, that
>I'll fix up in the next few days.  These were due to weird kernel configs and
>toolchain settings that caused certain bugs to surface.  When I'm done with the
>fixes, I'll let you know so you can re-test lmbench with your boards and make
>sure I didn't break anything.
>
>Thanks again.  This is applied and pushed.
> -- Tim