Date:	Mon, 11 May 2015 16:21:58 -0600
From:	Shuah Khan <shuahkh@....samsung.com>
To:	Darren Hart <dvhart@...ux.intel.com>, linux-api@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
CC:	Ingo Molnar <mingo@...e.hu>, Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Davidlohr Bueso <dave@...olabs.net>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH 1/5] selftests: Add futex functional tests

On 05/11/2015 04:21 PM, Shuah Khan wrote:
> On 05/11/2015 03:50 PM, Darren Hart wrote:
>> On 5/11/15, 1:51 PM, "Shuah Khan" <shuahkh@....samsung.com> wrote:
>>
>>> On 05/11/2015 01:47 PM, Darren Hart wrote:
>>>> On 5/11/15, 11:55 AM, "Shuah Khan" <shuahkh@....samsung.com> wrote:
>>>>
>>>>> On 05/11/2015 12:22 PM, Darren Hart wrote:
>>>>>> On 5/11/15, 11:06 AM, "Shuah Khan" <shuahkh@....samsung.com> wrote:
>>>>>>
>>>>>>> On 05/08/2015 04:09 PM, Darren Hart wrote:
>>>>>>>> The futextest testsuite [1] provides functional, stress, and
>>>>>>>> performance tests for the various futex op codes. Those tests will
>>>>>>>> be
>>>>>>>> of
>>>>>>>> more use to futex developers if they are included with the kernel
>>>>>>>> source.
>>>>>>>>
>>>>>>>> Copy the core infrastructure and the functional tests into
>>>>>>>> selftests,
>>>>>>>> but adapt them for inclusion in the kernel:
>>>>>>>>
>>>>>>>> - Update the Makefile to include the run_tests target, remove
>>>>>>>> reference
>>>>>>>>   to the performance and stress tests from the contributed sources.
>>>>>>>> - Replace my dead IBM email address with my current Intel email
>>>>>>>> address.
>>>>>>>> - Remove the warrantee and write-to paragraphs from the license
>>>>>>>> blurbs.
>>>>>>>> - Remove the NAME section as the filename is easily determined. ;-)
>>>>>>>> - Make the whitespace usage consistent in a couple of places.
>>>>>>>> - Cleanup various CodingStyle violations.
>>>>>>>>
>>>>>>>> A future effort will explore moving the performance and stress tests
>>>>>>>> into the kernel.
>>>>>>>>
>>>>>>>> 1. http://git.kernel.org/cgit/linux/kernel/git/dvhart/futextest.git
>>>>>>>>
>>>>>>>> Cc: Shuah Khan <shuahkh@....samsung.com>
>>>>>>>> Cc: linux-api@...r.kernel.org
>>>>>>>> Cc: Ingo Molnar <mingo@...e.hu>
>>>>>>>> Cc: Peter Zijlstra <peterz@...radead.org>
>>>>>>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>>>>>>> Cc: Davidlohr Bueso <dave@...olabs.net>
>>>>>>>> Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
>>>>>>>> Signed-off-by: Darren Hart <dvhart@...ux.intel.com>
>>>>>>>> ---
>>>>>>>
>>>>>>> Daren,
>>>>>>>
>>>>>>> I am seeing
>>>>>>>
>>>>>>> fatal: cannot convert from y to UTF-8
>>>>>>>
>>>>>>> when I try to apply the patch. Did you use git send-email?
>>>>>>
>>>>>> Hi Shuah,
>>>>>>
>>>>>> I've seen that from one of my contributors earlier this month as
>>>>>> well. I
>>>>>> dropped the Content... Header and it applied without problems.
>>>>>>
>>>>>> I created the patch using git format-patch and then sent them using
>>>>>> git
>>>>>> send-email - via a script I've been using for years now...
>>>>>>
>>>>>> Checking the files locally:
>>>>>> $ file *
>>>>>> 0000-cover-letter.patch:                                         ASCII
>>>>>> text
>>>>>> 0001-selftests-Add-futex-functional-tests.patch:
>>>>>> unified
>>>>>> diff output, UTF-8 Unicode text
>>>>>> 0002-selftest-futex-Update-Makefile-to-use-lib.mk.patch:
>>>>>> unified
>>>>>> diff output, ASCII text
>>>>>> 0003-selftest-futex-Increment-ksft-pass-and-fail-counters.patch:
>>>>>> unified
>>>>>> diff output, ASCII text
>>>>>> 0004-selftest-Add-futex-tests-to-the-top-level-Makefile.patch:
>>>>>> unified
>>>>>> diff output, ASCII text
>>>>>> 0005-kselftest-Add-exit-code-defines.patch:
>>>>>> unified
>>>>>> diff output, ASCII text
>>>>>>
>>>>>>
>>>>>> This shows that only the first in UTF-8 and the rest are ASCII. I
>>>>>> presume
>>>>>> this is due to the Copyright notices in the original files:
>>>>>>
>>>>>> Copyright © International Business Machines  Corp., 2006-2008
>>>>>>
>>>>>> Which use © instead of (C). I just checked and there are 545 instances
>>>>>> of
>>>>>> © in the kernel itself, so this should not present a problem.
>>>>>>
>>>>>> I apologize for the glitch in applying. If you use the pull request I
>>>>>> included that will avoid the mail transport issues, and I will be sure
>>>>>> to
>>>>>> fix my scripts to avoid the issue in the future.
>>>>>>
>>>>>> If you want to use the patches directly, please have a look at 1 of 5
>>>>>> and
>>>>>> just remove the "Content..." header, and I think you'll find "git am"
>>>>>> will
>>>>>> apply it without complaint.
>>>>>>
>>>>>
>>>>> Hi Daren,
>>>>>
>>>>> Removing the Content header got me past the utf error. However, git am
>>>>> complains:
>>>>>
>>>>> git am --signoff
>>>>>
>>>>> ../4.2_patches/futex_tests/PATCH_1_5selftestsAddfutexfunctionaltests.mbo
>>>>> x
>>>>> Applying: selftests: Add futex functional tests
>>>>> /mnt/data/lkml/linux-kselftest/.git/rebase-apply/patch:1457: new blank
>>>>> line at EOF.
>>>>> +
>>>>> warning: 1 line adds whitespace errors.
>>>>>
>>>>> Could you look into these. Rest of the patches applied fine.
>>>>
>>>> Hi Shuah,
>>>>
>>>> This was due to a newline at the end of:
>>>> tools/testing/selftests/futex/functional/run.sh
>>>>
>>>> Which unfortunately checkpatch.pl doesn't catch. My fault for not
>>>> applying
>>>> the series from the generated patches first. Sorry for the hassle.
>>>>
>>>> I have corrected this in the first patch and pushed v3 of the series to:
>>>>
>>>> git://git.infradead.org/users/dvhart/linux.git futextest-v3
>>>>
>>>>
>>>> $ git diff infradead/futextest-v2 infradead/futextest-v3
>>>> diff --git a/tools/testing/selftests/futex/functional/run.sh
>>>> b/tools/testing/selftests/futex/functional/run.sh
>>>> index 46827a8..e87dbe2 100755
>>>> --- a/tools/testing/selftests/futex/functional/run.sh
>>>> +++ b/tools/testing/selftests/futex/functional/run.sh
>>>> @@ -77,4 +77,3 @@ echo
>>>>  echo
>>>>  ./futex_wait_uninitialized_heap $COLOR
>>>>  ./futex_wait_private_mapped_file $COLOR
>>>> -
>>>>
>>>>
>>>
>>> No need for a new pull request. Have you seen these errors before:
>>>
>>> make[2]: Entering directory
>>> '/mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex'
>>> for DIR in functional; do make -C $DIR all ; done
>>> make[3]: Entering directory
>>> '/mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex/functional'
>>> gcc  -g -O2 -Wall -D_GNU_SOURCE -I../include -I../../   -lpthread -lrt
>>> futex_requeue_pi.c ../include/futextest.h   -o futex_requeue_pi
>>> /tmp/cc2UgUVs.o: In function `create_rt_thread':
>>> /mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex/functional/fu
>>> tex_requeue_pi.c:102:
>>> undefined reference to `pthread_create'
>>> /tmp/cc2UgUVs.o: In function `unit_test':
>>> /mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex/functional/fu
>>> tex_requeue_pi.c:342:
>>> undefined reference to `pthread_join'
>>> /mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex/functional/fu
>>> tex_requeue_pi.c:347:
>>> undefined reference to `pthread_join'
>>> /mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex/functional/fu
>>> tex_requeue_pi.c:346:
>>> undefined reference to `pthread_join'
>>> collect2: error: ld returned 1 exit status
>>> <builtin>: recipe for target 'futex_requeue_pi' failed
>>> make[3]: *** [futex_requeue_pi] Error 1
>>> make[3]: Leaving directory
>>> '/mnt/data/lkml/linux-kselftest/tools/testing/selftests/futex/functional'
>>> Makefile:7: recipe for target 'all' failed
>>>
>>> I am running make kselftest target when I saw the above build failures.
>>> kselftest run doesn't fail which is good, however futex tests won't
>>> run with this failure.
>>
>> I have not seen these errors whilst developing with futextest for the
>> kernel. That looks like you may be missing the pthread development headers
>> from your build machine.
>>
>> Those are provided by libc6-dev on my Debian systems.
>>
> 
> I do have lib6-dev installed. timers uses pthread compiles fine.
> So does mqueue. I am not sure what is going on with futex tests
> though. I am guessing it has to do with library link order.
> 
> The following fixed it for me:
> 
> -LDFLAGS := $(LDFLAGS) -lpthread -lrt
> +LDFLAGS := $(LDFLAGS) -pthread -lrt
> 
> Could you please make this change and resend the patch series.
> I prefer patch series over pull request.
> 
> thanks,
> -- Shuah
> 

Sorry full diff:


diff --git a/tools/testing/selftests/futex/functional/Makefile
b/tools/testing/selftests/futex/functional/Makefile
index e64d43b..b8a2e9b 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -1,6 +1,6 @@
 INCLUDES := -I../include -I../../
 CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE $(INCLUDES)
-LDFLAGS := $(LDFLAGS) -lpthread -lrt
+LDFLAGS := $(LDFLAGS) -pthread -lrt


-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@....samsung.com | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists