From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754660AbbIHJyQ (ORCPT ); Tue, 8 Sep 2015 05:54:16 -0400 Received: from ozlabs.org ([103.22.144.67]:39052 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753171AbbIHJyP (ORCPT ); Tue, 8 Sep 2015 05:54:15 -0400 Message-ID: <1441706053.7601.9.camel@ellerman.id.au> Subject: Re: [PATCH 6/7] selftests: only compile userfaultfd for x86 and powperpc From: Michael Ellerman To: Bamvor Zhang Jian Cc: linux-kernel@vger.kernel.org, Mark Brown , khilman@linaro.org, tyler.baker@linaro.org, shuahkh@osg.samsung.com, Andrea Arcangeli Date: Tue, 08 Sep 2015 19:54:13 +1000 In-Reply-To: <55EEA731.5090708@linaro.org> References: <1439559818-21666-1-git-send-email-bamvor.zhangjian@linaro.org> <1439559818-21666-7-git-send-email-bamvor.zhangjian@linaro.org> <1440991580.5735.4.camel@ellerman.id.au> <55EEA731.5090708@linaro.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2015-09-08 at 17:15 +0800, Bamvor Zhang Jian wrote: > Hi, Michael > > I thought I reply to you, but ... > > On 08/31/2015 11:26 AM, Michael Ellerman wrote: > > On Fri, 2015-08-14 at 21:43 +0800, Bamvor Jian Zhang wrote: > >> Signed-off-by: Bamvor Jian Zhang > >> --- > >> tools/testing/selftests/vm/Makefile | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > >> > >> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile > >> index bb888c6..4dd6e4f 100644 > >> --- a/tools/testing/selftests/vm/Makefile > >> +++ b/tools/testing/selftests/vm/Makefile > >> @@ -1,5 +1,15 @@ > >> # Makefile for vm selftests > >> > >> +uname_M := $(shell uname -m 2>/dev/null || echo not) > >> +ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/) > >> + > >> +ifeq ($(ARCH),powerpc) > >> +support_userfaultfd = yes > >> +endif > >> +ifeq ($(ARCH),x86) > >> +support_userfaultfd = yes > >> +endif > >> + > >> CFLAGS = -Wall > >> BINARIES = compaction_test > >> BINARIES += hugepage-mmap > >> @@ -9,7 +19,9 @@ BINARIES += mlock2-tests > >> BINARIES += on-fault-limit > >> BINARIES += thuge-gen > >> BINARIES += transhuge-stress > >> +ifdef support_userfaultfd > >> BINARIES += userfaultfd > >> +endif > >> > >> all: $(BINARIES) > >> %: %.c > > > > > > This is nasty. It means when userfaultfd gets implemented for other arches > > someone has to remember to update the logic here, which they won't. > > > > Instead the C program should just do nothing when __NR_userfaultfd is not defined, eg: > > > > #ifdef __NR_userfaultfd > > > > int main(int argc, char **argv) > > { > > ... > > } > > > > #else > > > > int main(void) > > { > > printf("skip: Skipping userfaultfd test\n"); > > return 0; > > } > > #endif > > > > > > This way when the syscall is implemented for other arches the test will just > > start working. > > > > cheers > > > > > When read the following code, It seems that sometimes __NR_userfaultfd is not > defined but the syscall is exist. I am not sure why these piece is needed. > cc'd c > > #ifndef __NR_userfaultfd > #ifdef __x86_64__ > #define __NR_userfaultfd 323 > #elif defined(__i386__) > #define __NR_userfaultfd 374 > #elif defined(__powewrpc__) > #define __NR_userfaultfd 364 > #else > #error "missing __NR_userfaultfd definition" > #endif > #endif > > Do you mean that we should remove the above code? Well yes, it would need to be removed to make the logic I suggested work. I'm not sure those #defines actually help in practice, because if the syscall number is not defined then linux/userfaultfd.h will not exist and the whole test will not compile anyway. I was suggesting something like this, which has the properties of: - not breaking the build on arches that don't have the syscall - still printing a notice on arches that don't have the syscall, both at build time and runtime. - building correctly on an arch as soon as that arch implements the syscall, with no extra changes required. cheers diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c index 2bf1fc3f562b..652c9d805006 100644 --- a/tools/testing/selftests/vm/userfaultfd.c +++ b/tools/testing/selftests/vm/userfaultfd.c @@ -64,19 +64,10 @@ #include #include #include -#include -#ifndef __NR_userfaultfd -#ifdef __x86_64__ -#define __NR_userfaultfd 323 -#elif defined(__i386__) -#define __NR_userfaultfd 374 -#elif defined(__powewrpc__) -#define __NR_userfaultfd 364 -#else -#error "missing __NR_userfaultfd definition" -#endif -#endif +#ifdef __NR_userfaultfd + +#include static unsigned long nr_cpus, nr_pages, nr_pages_per_cpu, page_size; @@ -636,3 +627,15 @@ int main(int argc, char **argv) nr_pages, nr_pages_per_cpu); return userfaultfd_stress(); } + +#else /* ! __NR_userfaultfd */ + +#warning "missing __NR_userfaultfd definition" + +int main(void) +{ + printf("skip: Skipping userfaultfd test (missing __NR_userfaultfd)\n"); + return 0; +} + +#endif