diff mbox series

[v3] kernel/linux: fix kernel dir for meson

Message ID 20191203155922.1565-1-xiaolong.ye@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] kernel/linux: fix kernel dir for meson |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Xiaolong Ye Dec. 3, 2019, 3:59 p.m. UTC 
  kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
system, for cross-compilation case, users would specify it as local
kernel src dir like

/<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/

Current meson build would fail to compile kernel module if user specify
kernel_dir as above, this patch fixes this issue.

After this change, for normal build case, user can specify
/lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build as
kernel_dir. For cross compilation case, user can specify any directory
that contains kernel source code as the kernel_dir.

Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
Cc: stable@dpdk.org
Cc: iryzhov@nfware.com

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---

V3 changes:

1. support setting both /lib/modules/<kernel_version> and
   /lib/modules/<kernel_version>/build as kernel_dir to keep backward
   compatibility

V2 changes:

1. handle both normal and cross-compilation cases

 kernel/linux/igb_uio/meson.build |  4 ++--
 kernel/linux/kni/meson.build     |  4 ++--
 kernel/linux/meson.build         | 19 +++++++++++++++----
 meson_options.txt                |  2 +-
 4 files changed, 20 insertions(+), 9 deletions(-)

Comments

Luca Boccassi Dec. 4, 2019, 1:51 p.m. UTC | #1 
On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
> make
> system, for cross-compilation case, users would specify it as local
> kernel src dir like
> 
> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> 
> Current meson build would fail to compile kernel module if user
> specify
> kernel_dir as above, this patch fixes this issue.
> 
> After this change, for normal build case, user can specify
> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
> as
> kernel_dir. For cross compilation case, user can specify any
> directory
> that contains kernel source code as the kernel_dir.
> 
> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> Cc: 
> stable@dpdk.org
> 
> Cc: 
> iryzhov@nfware.com
> 
> 
> Signed-off-by: Xiaolong Ye <
> xiaolong.ye@intel.com

The convention used by upstream and all distros is that kernel headers
are in <version>/build. Why can't the cross compilation case also
follow this convention, rather than adding complications to the
downstream build system?
Xiaolong Ye Dec. 4, 2019, 2:18 p.m. UTC | #2 
On 12/04, Luca Boccassi wrote:
>On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
>> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
>> make
>> system, for cross-compilation case, users would specify it as local
>> kernel src dir like
>> 
>> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> 
>> Current meson build would fail to compile kernel module if user
>> specify
>> kernel_dir as above, this patch fixes this issue.
>> 
>> After this change, for normal build case, user can specify
>> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
>> as
>> kernel_dir. For cross compilation case, user can specify any
>> directory
>> that contains kernel source code as the kernel_dir.
>> 
>> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> Cc: 
>> stable@dpdk.org
>> 
>> Cc: 
>> iryzhov@nfware.com
>> 
>> 
>> Signed-off-by: Xiaolong Ye <
>> xiaolong.ye@intel.com
>
>The convention used by upstream and all distros is that kernel headers
>are in <version>/build. Why can't the cross compilation case also
>follow this convention, rather than adding complications to the

Yes, cross-compilation can follow this convention, but one common case is that
users download and put kernel src (the same kernel that's running in the target machine)
to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
it's extra burden for users to create extra build dir to hold the kernel headers.

Thanks,
Xiaolong

>downstream build system?
>
>-- 
>Kind regards,
>Luca Boccassi
Bruce Richardson Dec. 4, 2019, 3:12 p.m. UTC | #3 
On Wed, Dec 04, 2019 at 10:18:21PM +0800, Ye Xiaolong wrote:
> On 12/04, Luca Boccassi wrote:
> >On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
> >> make
> >> system, for cross-compilation case, users would specify it as local
> >> kernel src dir like
> >> 
> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >> 
> >> Current meson build would fail to compile kernel module if user
> >> specify
> >> kernel_dir as above, this patch fixes this issue.
> >> 
> >> After this change, for normal build case, user can specify
> >> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
> >> as
> >> kernel_dir. For cross compilation case, user can specify any
> >> directory
> >> that contains kernel source code as the kernel_dir.
> >> 
> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> Cc: 
> >> stable@dpdk.org
> >> 
> >> Cc: 
> >> iryzhov@nfware.com
> >> 
> >> 
> >> Signed-off-by: Xiaolong Ye <
> >> xiaolong.ye@intel.com
> >
> >The convention used by upstream and all distros is that kernel headers
> >are in <version>/build. Why can't the cross compilation case also
> >follow this convention, rather than adding complications to the
> 
> Yes, cross-compilation can follow this convention, but one common case is that
> users download and put kernel src (the same kernel that's running in the target machine)
> to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
> it's extra burden for users to create extra build dir to hold the kernel headers.
> 

As part of the build of the kernel, do you not do a "modules_install" step,
which should set up things correctly for later builds?

/Bruce
Xiaolong Ye Dec. 8, 2019, 1:26 a.m. UTC | #4 
On 12/04, Bruce Richardson wrote:
>On Wed, Dec 04, 2019 at 10:18:21PM +0800, Ye Xiaolong wrote:
>> On 12/04, Luca Boccassi wrote:
>> >On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
>> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
>> >> make
>> >> system, for cross-compilation case, users would specify it as local
>> >> kernel src dir like
>> >> 
>> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> >> 
>> >> Current meson build would fail to compile kernel module if user
>> >> specify
>> >> kernel_dir as above, this patch fixes this issue.
>> >> 
>> >> After this change, for normal build case, user can specify
>> >> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
>> >> as
>> >> kernel_dir. For cross compilation case, user can specify any
>> >> directory
>> >> that contains kernel source code as the kernel_dir.
>> >> 
>> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> >> Cc: 
>> >> stable@dpdk.org
>> >> 
>> >> Cc: 
>> >> iryzhov@nfware.com
>> >> 
>> >> 
>> >> Signed-off-by: Xiaolong Ye <
>> >> xiaolong.ye@intel.com
>> >
>> >The convention used by upstream and all distros is that kernel headers
>> >are in <version>/build. Why can't the cross compilation case also
>> >follow this convention, rather than adding complications to the
>> 
>> Yes, cross-compilation can follow this convention, but one common case is that
>> users download and put kernel src (the same kernel that's running in the target machine)
>> to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
>> it's extra burden for users to create extra build dir to hold the kernel headers.
>> 
>
>As part of the build of the kernel, do you not do a "modules_install" step,
>which should set up things correctly for later builds?

Yes, this cmd helps. But for make build, user could specify both
/lib/modules/<kernelversion>/build and any kernel src dir as RTE_KERNELDIR, I
think this patch can help give user consistent experience when they migrate from make 
to meson build.

Thanks,
Xiaolong

>
>/Bruce
Bruce Richardson Dec. 9, 2019, 12:12 p.m. UTC | #5 
On Sun, Dec 08, 2019 at 09:26:57AM +0800, Ye Xiaolong wrote:
> On 12/04, Bruce Richardson wrote:
> >On Wed, Dec 04, 2019 at 10:18:21PM +0800, Ye Xiaolong wrote:
> >> On 12/04, Luca Boccassi wrote:
> >> >On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
> >> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
> >> >> make
> >> >> system, for cross-compilation case, users would specify it as local
> >> >> kernel src dir like
> >> >> 
> >> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >> >> 
> >> >> Current meson build would fail to compile kernel module if user
> >> >> specify
> >> >> kernel_dir as above, this patch fixes this issue.
> >> >> 
> >> >> After this change, for normal build case, user can specify
> >> >> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
> >> >> as
> >> >> kernel_dir. For cross compilation case, user can specify any
> >> >> directory
> >> >> that contains kernel source code as the kernel_dir.
> >> >> 
> >> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> >> Cc: 
> >> >> stable@dpdk.org
> >> >> 
> >> >> Cc: 
> >> >> iryzhov@nfware.com
> >> >> 
> >> >> 
> >> >> Signed-off-by: Xiaolong Ye <
> >> >> xiaolong.ye@intel.com
> >> >
> >> >The convention used by upstream and all distros is that kernel headers
> >> >are in <version>/build. Why can't the cross compilation case also
> >> >follow this convention, rather than adding complications to the
> >> 
> >> Yes, cross-compilation can follow this convention, but one common case is that
> >> users download and put kernel src (the same kernel that's running in the target machine)
> >> to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
> >> it's extra burden for users to create extra build dir to hold the kernel headers.
> >> 
> >
> >As part of the build of the kernel, do you not do a "modules_install" step,
> >which should set up things correctly for later builds?
> 
> Yes, this cmd helps. But for make build, user could specify both
> /lib/modules/<kernelversion>/build and any kernel src dir as RTE_KERNELDIR, I
> think this patch can help give user consistent experience when they migrate from make 
> to meson build.
> 

Issues like specifying the wrong directory when switching over are
a) short term, once off issue in the conversion from make to meson and
b) fixed by just looking at the documentation for the build option.

On the other hand, maintaining code supporting both options is complexity
that, if added to meson builds, needs to be maintained going forward. While
we can expect users to have to make changes when switching build system, we
should not require them later to make changes based on us arbitrarily
changing the allowed values of options.

Therefore, unless your build system absolutely cannot be set up to support
the existing build configuration, I would rather we not accept changes
here, and we try and keep the code simpler.

Regards,
/Bruce
diff mbox series

Patch

diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
index fac404f07..63990372e 100644
--- a/kernel/linux/igb_uio/meson.build
+++ b/kernel/linux/igb_uio/meson.build
@@ -8,7 +8,7 @@  mkfile = custom_target('igb_uio_makefile',
 custom_target('igb_uio',
 	input: ['igb_uio.c', 'Kbuild'],
 	output: 'igb_uio.ko',
-	command: ['make', '-C', kernel_dir + '/build',
+	command: ['make', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
@@ -16,5 +16,5 @@  custom_target('igb_uio',
 		'modules'],
 	depends: mkfile,
 	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install_dir: install_base + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index 955eec949..04c817e8a 100644
--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -13,7 +13,7 @@  kni_sources = files(
 custom_target('rte_kni',
 	input: kni_sources,
 	output: 'rte_kni.ko',
-	command: ['make', '-j4', '-C', kernel_dir + '/build',
+	command: ['make', '-j4', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' +
@@ -25,5 +25,5 @@  custom_target('rte_kni',
 	depends: kni_mkfile,
 	console: true,
 	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install_dir: install_base + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
index 1796cc686..0568ed7e1 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -13,15 +13,26 @@  kernel_dir = get_option('kernel_dir')
 if kernel_dir == ''
 	# use default path for native builds
 	kernel_version = run_command('uname', '-r').stdout().strip()
-	kernel_dir = '/lib/modules/' + kernel_version
+	kernel_dir = '/lib/modules/' + kernel_version + '/build'
 endif
 
 # test running make in kernel directory, using "make kernelversion"
-make_returncode = run_command('make', '-sC', kernel_dir + '/build',
+make_returncode = run_command('make', '-sC', kernel_dir,
 		'kernelversion').returncode()
 if make_returncode != 0
-	warning('Cannot compile kernel modules as requested - are kernel headers installed?')
-	subdir_done()
+	kernel_dir = kernel_dir + '/build'
+	make_returncode = run_command('make', '-sC', kernel_dir,
+			'kernelversion').returncode()
+	if make_returncode != 0
+		warning('Cannot compile kernel modules as requested - are kernel headers installed?')
+		subdir_done()
+	endif
+endif
+
+if kernel_dir.endswith('build') or kernel_dir.endswith('build/')
+	install_base = kernel_dir.split('build')[0]
+else
+	install_base = kernel_dir
 endif
 
 # DO ACTUAL MODULE BUILDING
diff --git a/meson_options.txt b/meson_options.txt
index bc369d06c..7eba3b720 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -17,7 +17,7 @@  option('ibverbs_link', type: 'combo', choices : ['shared', 'dlopen'], value: 'sh
 option('include_subdir_arch', type: 'string', value: '',
 	description: 'subdirectory where to install arch-dependent headers')
 option('kernel_dir', type: 'string', value: '',
-	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
+	description: 'Path to the kernel for building kernel modules. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
 option('lib_musdk_dir', type: 'string', value: '',
 	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',