Skip to contentSkip to conversation

kernel.bbclass: Fix race condition

PATCH In review Ed Bartosh 4 participants 11 messages Apr 14, 2015 – May 6, 2015

lore
Share & tools
lore

Ed Bartosh

author
#0 lore raw

PATCH 1/1 kernel.bbclass: Fix race condition

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]

---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +11 -1
index 125ed88..aad430a 100644
@@ -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 () {
 	:
Signature · 1 line
2.1.4
#1 lore raw
Quoted 7 lines · 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.

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

Quoted 48 lines

[YOCTO #7321]

---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +11 -1
index 125ed88..aad430a 100644
@@ -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 () {
 	:
Signature · 1 line
2.1.4
#2 lore raw
Quoted 5 lines · Richard Purdie
Quoted 4 lines · 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.

Quoted 8 lines
Quoted 3 lines

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

Quoted 60 lines

Cheers,

Richard

Quoted 46 lines

[YOCTO #7321]

---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +11 -2
index 125ed88..aad430a 100644
@@ -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 () {
      :
--
Quoted 2 lines

2.1.4

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"
author
#3 lore raw

PATCH 1/1 kernel.bbclass: Fix race condition

In reply to Richard Purdie

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]

---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +11 -1
index 125ed88..13c0180 100644
@@ -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 () {
 	:
Signature · 1 line
2.1.4
#4 lore raw
Quoted 7 lines · 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.

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

Quoted 47 lines

[YOCTO #7321]

---
  meta/classes/kernel.bbclass | 12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +11 -1
index 125ed88..13c0180 100644
@@ -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 () {
  	:
#5 lore raw
In reply to Ed Bartosh
Quoted 43 lines · 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]

---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +4 -1
index 125ed88..13c0180 100644
@@ -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

meta/classes/kernel.bbclass do_shared_workdir () { · line 304

Quoted 10 lines
meta/classes/kernel.bbclass +7 -0
+# 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

author
#6 lore raw

PATCH 1/1 kernel.bbclass: Fix race condition

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]

---
 meta/classes/kernel.bbclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
meta/classes/kernel.bbclass +11 -1
index 125ed88..7878a73 100644
@@ -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 () {
 	:
Signature · 1 line
2.1.4
author
#7 lore raw

PATCH 1/1 kernel.bbclass: Fix race condition

In reply to Richard Purdie

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]

---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/kernel.bbclass +1 -1
index 125ed88..9207ae4 100644
@@ -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
Signature · 1 line
2.1.4
#8 lore raw
Quoted 6 lines · 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.

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

Quoted 29 lines

[YOCTO #7321]

---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/kernel.bbclass +1 -2
index 125ed88..9207ae4 100644
@@ -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
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"
#9 lore raw
In reply to Ed Bartosh
Quoted 19 lines · 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]

---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/kernel.bbclass +0 -0
index 125ed88..9207ae4 100644

meta/classes/kernel.bbclass kernel_do_install() { · line 237

Quoted 5 lines
meta/classes/kernel.bbclass +1 -1
@@ -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

author
#10 lore raw

PATCH 1/1 kernel.bbclass: Fix race condition

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]

---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/kernel.bbclass +1 -1
index 125ed88..74092e9 100644
@@ -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
Signature · 1 line
2.1.4