[PATCH] kernel.bbclass: Fix race condition

[PATCH] kernel.bbclass: Fix race condition

[Ed Bartosh]

Race condition between do_compile_kernelmodules and do_shared_workdir
tasks occurs when do_compilemodules changes files in include/generated/*
while do_shared_workdir tries to copy them to shared working directory.

Functionality of do_shared_workdir has been moved to separate function
mk_shared_workdir and called from do_kernel_compile. do_shared_workdir
left in the code as it may be used in many recipes.

[YOCTO #7321]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 125ed88..aad430a 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -231,6 +231,8 @@ kernel_do_install() {
 	[ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/boot/Module.symvers-${KERNEL_VERSION}
 	install -d ${D}${sysconfdir}/modules-load.d
 	install -d ${D}${sysconfdir}/modprobe.d
+
+	mk_shared_workdir
 }
 do_install[prefuncs] += "package_get_auto_pr"
 
@@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
 
 PACKAGEFUNCS += "emit_depmod_pkgdata"
 
-do_shared_workdir () {
+mk_shared_workdir () {
 	cd ${B}
 
 	kerneldir=${STAGING_KERNEL_BUILDDIR}
@@ -289,6 +291,14 @@ do_shared_workdir () {
 	fi
 }
 
+# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
+# and called from kernel_do_compile.
+# It caused race condition with do_compile_kernelmodules when it runs
+# in parallel with do_compile_kernelmodules
+do_shared_workdir () {
+	:
+}
+
 # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
 sysroot_stage_all () {
 	:
-- 
2.1.4

Re: [PATCH] kernel.bbclass: Fix race condition

[Richard Purdie]

On Tue, 2015-04-14 at 19:56 +0300, Ed Bartosh wrote:
> Race condition between do_compile_kernelmodules and do_shared_workdir
> tasks occurs when do_compilemodules changes files in include/generated/*
> while do_shared_workdir tries to copy them to shared working directory.
> 
> Functionality of do_shared_workdir has been moved to separate function
> mk_shared_workdir and called from do_kernel_compile. do_shared_workdir
> left in the code as it may be used in many recipes.

Its called from do_kernel_install according to the patch?

This leaves a race since some code depends on the do_shared_work task
and shared_work is added after compile before install.

Cheers,

Richard

> [YOCTO #7321]
> 
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
> ---
>  meta/classes/kernel.bbclass | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 125ed88..aad430a 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -231,6 +231,8 @@ kernel_do_install() {
>  	[ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/boot/Module.symvers-${KERNEL_VERSION}
>  	install -d ${D}${sysconfdir}/modules-load.d
>  	install -d ${D}${sysconfdir}/modprobe.d
> +
> +	mk_shared_workdir
>  }
>  do_install[prefuncs] += "package_get_auto_pr"
>  
> @@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
>  
>  PACKAGEFUNCS += "emit_depmod_pkgdata"
>  
> -do_shared_workdir () {
> +mk_shared_workdir () {
>  	cd ${B}
>  
>  	kerneldir=${STAGING_KERNEL_BUILDDIR}
> @@ -289,6 +291,14 @@ do_shared_workdir () {
>  	fi
>  }
>  
> +# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
> +# and called from kernel_do_compile.
> +# It caused race condition with do_compile_kernelmodules when it runs
> +# in parallel with do_compile_kernelmodules
> +do_shared_workdir () {
> +	:
> +}
> +
>  # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
>  sysroot_stage_all () {
>  	:
> -- 
> 2.1.4
> 

Re: [PATCH] kernel.bbclass: Fix race condition

[Bruce Ashfield]

On Fri, May 1, 2015 at 10:28 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Tue, 2015-04-14 at 19:56 +0300, Ed Bartosh wrote:
>> Race condition between do_compile_kernelmodules and do_shared_workdir
>> tasks occurs when do_compilemodules changes files in include/generated/*
>> while do_shared_workdir tries to copy them to shared working directory.
>>
>> Functionality of do_shared_workdir has been moved to separate function
>> mk_shared_workdir and called from do_kernel_compile. do_shared_workdir
>> left in the code as it may be used in many recipes.
>
> Its called from do_kernel_install according to the patch?
>
> This leaves a race since some code depends on the do_shared_work task
> and shared_work is added after compile before install.

And the autobuilder picked up the failure as well. I have a variant
staged here, and
will shake it out now that my other kernel series is out.

Bruce

>
> Cheers,
>
> Richard
>
>> [YOCTO #7321]
>>
>> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
>> Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
>> ---
>>  meta/classes/kernel.bbclass | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>> index 125ed88..aad430a 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -231,6 +231,8 @@ kernel_do_install() {
>>       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/boot/Module.symvers-${KERNEL_VERSION}
>>       install -d ${D}${sysconfdir}/modules-load.d
>>       install -d ${D}${sysconfdir}/modprobe.d
>> +
>> +     mk_shared_workdir
>>  }
>>  do_install[prefuncs] += "package_get_auto_pr"
>>
>> @@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
>>
>>  PACKAGEFUNCS += "emit_depmod_pkgdata"
>>
>> -do_shared_workdir () {
>> +mk_shared_workdir () {
>>       cd ${B}
>>
>>       kerneldir=${STAGING_KERNEL_BUILDDIR}
>> @@ -289,6 +291,14 @@ do_shared_workdir () {
>>       fi
>>  }
>>
>> +# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
>> +# and called from kernel_do_compile.
>> +# It caused race condition with do_compile_kernelmodules when it runs
>> +# in parallel with do_compile_kernelmodules
>> +do_shared_workdir () {
>> +     :
>> +}
>> +
>>  # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
>>  sysroot_stage_all () {
>>       :
>> --
>> 2.1.4
>>
>
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core



-- 
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"

[PATCH] kernel.bbclass: Fix race condition

[Ed Bartosh]

Race condition between do_compile_kernelmodules and do_shared_workdir
tasks occurs when do_compilemodules changes files in include/generated/*
while do_shared_workdir tries to copy them to shared working directory.

Functionality of do_shared_workdir has been moved to separate function
mk_shared_workdir and called from do_kernel_compile. do_shared_workdir
left in the code as it may be used in many recipes.

[YOCTO #7321]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 125ed88..13c0180 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -192,6 +192,8 @@ kernel_do_compile() {
 	if test "${KERNEL_IMAGETYPE_FOR_MAKE}.gz" = "${KERNEL_IMAGETYPE}"; then
 		gzip -9c < "${KERNEL_IMAGETYPE_FOR_MAKE}" > "${KERNEL_OUTPUT}"
 	fi
+
+	mk_shared_workdir
 }
 
 do_compile_kernelmodules() {
@@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
 
 PACKAGEFUNCS += "emit_depmod_pkgdata"
 
-do_shared_workdir () {
+mk_shared_workdir () {
 	cd ${B}
 
 	kerneldir=${STAGING_KERNEL_BUILDDIR}
@@ -289,6 +291,14 @@ do_shared_workdir () {
 	fi
 }
 
+# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
+# and called from kernel_do_compile.
+# It caused race condition with do_compile_kernelmodules when it runs
+# in parallel with do_compile_kernelmodules
+do_shared_workdir () {
+	:
+}
+
 # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
 sysroot_stage_all () {
 	:
-- 
2.1.4

Re: [PATCH] kernel.bbclass: Fix race condition

[Bruce Ashfield]

On 2015-05-04 10:11 AM, Ed Bartosh wrote:
> Race condition between do_compile_kernelmodules and do_shared_workdir
> tasks occurs when do_compilemodules changes files in include/generated/*
> while do_shared_workdir tries to copy them to shared working directory.
>
> Functionality of do_shared_workdir has been moved to separate function
> mk_shared_workdir and called from do_kernel_compile. do_shared_workdir
> left in the code as it may be used in many recipes.

Looks good. Thanks for the respin of the patch! I'll put my
variant in the attic, since this looks like it will solve
the issue.

Cheers,

Bruce

>
> [YOCTO #7321]
>
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
> ---
>   meta/classes/kernel.bbclass | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 125ed88..13c0180 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -192,6 +192,8 @@ kernel_do_compile() {
>   	if test "${KERNEL_IMAGETYPE_FOR_MAKE}.gz" = "${KERNEL_IMAGETYPE}"; then
>   		gzip -9c < "${KERNEL_IMAGETYPE_FOR_MAKE}" > "${KERNEL_OUTPUT}"
>   	fi
> +
> +	mk_shared_workdir
>   }
>
>   do_compile_kernelmodules() {
> @@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
>
>   PACKAGEFUNCS += "emit_depmod_pkgdata"
>
> -do_shared_workdir () {
> +mk_shared_workdir () {
>   	cd ${B}
>
>   	kerneldir=${STAGING_KERNEL_BUILDDIR}
> @@ -289,6 +291,14 @@ do_shared_workdir () {
>   	fi
>   }
>
> +# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
> +# and called from kernel_do_compile.
> +# It caused race condition with do_compile_kernelmodules when it runs
> +# in parallel with do_compile_kernelmodules
> +do_shared_workdir () {
> +	:
> +}
> +
>   # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
>   sysroot_stage_all () {
>   	:
>

Re: [PATCH] kernel.bbclass: Fix race condition

[Richard Purdie]

On Mon, 2015-05-04 at 17:11 +0300, Ed Bartosh wrote:
> Race condition between do_compile_kernelmodules and do_shared_workdir
> tasks occurs when do_compilemodules changes files in include/generated/*
> while do_shared_workdir tries to copy them to shared working directory.
> 
> Functionality of do_shared_workdir has been moved to separate function
> mk_shared_workdir and called from do_kernel_compile. do_shared_workdir
> left in the code as it may be used in many recipes.
> 
> [YOCTO #7321]
> 
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
> ---
>  meta/classes/kernel.bbclass | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 125ed88..13c0180 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -192,6 +192,8 @@ kernel_do_compile() {
>  	if test "${KERNEL_IMAGETYPE_FOR_MAKE}.gz" = "${KERNEL_IMAGETYPE}"; then
>  		gzip -9c < "${KERNEL_IMAGETYPE_FOR_MAKE}" > "${KERNEL_OUTPUT}"
>  	fi
> +
> +	mk_shared_workdir
>  }
>  
>  do_compile_kernelmodules() {
> @@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
>  
>  PACKAGEFUNCS += "emit_depmod_pkgdata"
>  
> -do_shared_workdir () {
> +mk_shared_workdir () {
>  	cd ${B}
>  
>  	kerneldir=${STAGING_KERNEL_BUILDDIR}
> @@ -289,6 +291,14 @@ do_shared_workdir () {
>  	fi
>  }
>  
> +# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
> +# and called from kernel_do_compile.
> +# It caused race condition with do_compile_kernelmodules when it runs
> +# in parallel with do_compile_kernelmodules
> +do_shared_workdir () {
> +	:
> +}
> +
>  # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
>  sysroot_stage_all () {
>  	:

This fails on the autobuilder with a missing kernel-None package.
Building lttng-modules, then looking at the package dependencies 100%
reproduces. The problem is this line:

echo "${KERNEL_VERSION}" > $kerneldir/kernel-abiversion

since KERNEL_VERSION is obtained from ${B} via:

KERNEL_VERSION = "${@get_kernelversion_headers('${B}')}"

and ${B} isn't populated when the do_compile shell script is generated
resulting in the file containing "None".

Cheers,

Richard

[PATCH] kernel.bbclass: Fix race condition

[Ed Bartosh]

Race condition between do_compile_kernelmodules and do_shared_workdir
tasks occurs when do_compile_kernelmodules changes files in
include/generated/* while do_shared_workdir tries to copy them to
shared working directory.

Functionality of do_shared_workdir has been moved to separate function
mk_shared_workdir and called from do_compile_kernelmodules.
do_shared_workdir is left in the code as it may be used in many recipes.

[YOCTO #7321]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
Signed-off-by: Bruce Ashfield <bruce.ashfield@windriver.com>
---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 125ed88..7878a73 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -201,6 +201,8 @@ do_compile_kernelmodules() {
 	else
 		bbnote "no modules to compile"
 	fi
+
+	mk_shared_workdir
 }
 addtask compile_kernelmodules after do_compile before do_strip
 
@@ -245,7 +247,7 @@ emit_depmod_pkgdata() {
 
 PACKAGEFUNCS += "emit_depmod_pkgdata"
 
-do_shared_workdir () {
+mk_shared_workdir () {
 	cd ${B}
 
 	kerneldir=${STAGING_KERNEL_BUILDDIR}
@@ -289,6 +291,14 @@ do_shared_workdir () {
 	fi
 }
 
+# NOTE!!! Functionality of do_shared_workdir has been moved to mk_shared_workdir
+# and called from kernel_do_compile.
+# It caused race condition with do_compile_kernelmodules when it runs
+# in parallel with do_compile_kernelmodules
+do_shared_workdir () {
+	:
+}
+
 # We don't need to stage anything, not the modules/firmware since those would clash with linux-firmware
 sysroot_stage_all () {
 	:
-- 
2.1.4

[PATCH v.4] kernel.bbclass: Fix race condition

[Ed Bartosh]

Race condition between do_compile_kernelmodules and do_shared_workdir
tasks occurs when do_compile_kernelmodules changes files in
include/generated/* while do_shared_workdir tries to copy them to
shared working directory.

Fixed race by moving do_shared_workdir after do_compile_kernelmodules.

[YOCTO #7321]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 125ed88..9207ae4 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -234,7 +234,7 @@ kernel_do_install() {
 }
 do_install[prefuncs] += "package_get_auto_pr"
 
-addtask shared_workdir after do_compile before do_install
+addtask shared_workdir after do_compile_kernelmodules before do_install
 
 emit_depmod_pkgdata() {
 	# Stash data for depmod
-- 
2.1.4

Re: [PATCH v.4] kernel.bbclass: Fix race condition

[Bruce Ashfield]

On Wed, May 6, 2015 at 10:03 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> Race condition between do_compile_kernelmodules and do_shared_workdir
> tasks occurs when do_compile_kernelmodules changes files in
> include/generated/* while do_shared_workdir tries to copy them to
> shared working directory.
>
> Fixed race by moving do_shared_workdir after do_compile_kernelmodules.

I assume that this supersedes all the old versions.

I had a comment typed for the first, but got distracted and didn't send it
before this one came out!

I was concerned in the earlier revision that the race still existed, or
more precisely that we had things in a non-obvious place if a recipe
needed to depend on the artifacts in the shared directory.

Will this variant fix the issue that Richard pointed out with lttng-modules ?
(It should, since lttng-modules does depend on
'virtual/kernel:do_shared_workdir'
and that will now run after the kernel modules are installed.

We are forcing a bit of serialization of the module build for anything external
that depends on the artifacts, but I'd rather get this working .. and
worry about
any optimizations and further tweaks later.

Bruce


>
> [YOCTO #7321]
>
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> ---
>  meta/classes/kernel.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 125ed88..9207ae4 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -234,7 +234,7 @@ kernel_do_install() {
>  }
>  do_install[prefuncs] += "package_get_auto_pr"
>
> -addtask shared_workdir after do_compile before do_install
> +addtask shared_workdir after do_compile_kernelmodules before do_install
>
>  emit_depmod_pkgdata() {
>         # Stash data for depmod
> --
> 2.1.4
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core



-- 
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"

Re: [PATCH v.4] kernel.bbclass: Fix race condition

[Richard Purdie]

On Wed, 2015-05-06 at 17:03 +0300, Ed Bartosh wrote:
> Race condition between do_compile_kernelmodules and do_shared_workdir
> tasks occurs when do_compile_kernelmodules changes files in
> include/generated/* while do_shared_workdir tries to copy them to
> shared working directory.
> 
> Fixed race by moving do_shared_workdir after do_compile_kernelmodules.
> 
> [YOCTO #7321]
> 
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> ---
>  meta/classes/kernel.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 125ed88..9207ae4 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -234,7 +234,7 @@ kernel_do_install() {
>  }
>  do_install[prefuncs] += "package_get_auto_pr"
>  
> -addtask shared_workdir after do_compile before do_install
> +addtask shared_workdir after do_compile_kernelmodules before do_install

Sorry to further discuss this, however I was thinking about something
Mike was saying about external kernel module builds causing kernel
builds in a different thread. 

The above will mean that external module builds depend on
do_compile_kernelmodules.

I therefore think we should do:

addtask shared_workdir after do_compile before do_compile_kernelmodules

since that will inflict less overhead on module builds.

Cheers,

Richard

[PATCH v.5] kernel.bbclass: Fix race condition

[Ed Bartosh]

Race condition between do_compile_kernelmodules and do_shared_workdir
tasks occurs when do_compile_kernelmodules changes files in
include/generated/* while do_shared_workdir tries to copy them to
shared working directory.

Fixed race by moving do_shared_workdir after do_compile but before
do_compile_kernelmodules.

[YOCTO #7321]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 125ed88..74092e9 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -234,7 +234,7 @@ kernel_do_install() {
 }
 do_install[prefuncs] += "package_get_auto_pr"
 
-addtask shared_workdir after do_compile before do_install
+addtask shared_workdir after do_compile before do_compile_kernelmodules
 
 emit_depmod_pkgdata() {
 	# Stash data for depmod
-- 
2.1.4