All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
To: "OE Core (openembedded-core@lists.openembedded.org)"
	<openembedded-core@lists.openembedded.org>
Subject: Oddness regarding file locks in package.bbclass
Date: Tue, 3 Oct 2017 18:17:20 +0000	[thread overview]
Message-ID: <b720c2022a0a49b08ea3159ef3fdf083@XBOX02.axis.com> (raw)

I just stumbled upon something odd in package.bbclass. In commit 
ede381d5 from January 2011 (the code hasn't changed since), the 
use of the ${PACKAGELOCK} lock file was changed to shared to 
improve parallelism. However, when looking at the actual change 
it becomes confusing. I have included it below for reference.

> commit ede381d56b180b384fdad98d445e5430819cfade
> Author: Richard Purdie <richard.purdie@linuxfoundation.org>
> Date:   Wed Jan 19 11:04:15 2011 +0000
> 
>     package.bbclass: Take a shared lock when reading to improve do_package parallelism
>     
>     Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index d39c694de5..8e7fa26f72 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -497,7 +497,8 @@ python emit_pkgdata() {
>  	pkgdest = bb.data.getVar('PKGDEST', d, 1)
>  	pkgdatadir = bb.data.getVar('PKGDESTWORK', d, True)
>  
> -	lf = bb.utils.lockfile(bb.data.expand("${PACKAGELOCK}", d))
> +	# Take shared lock since we're only reading, not writing
> +	lf = bb.utils.lockfile(bb.data.expand("${PACKAGELOCK}", d), True)

Here the lock is changed to shared as per the commit message.

>  
>  	data_file = pkgdatadir + bb.data.expand("/${PN}" , d)
>  	f = open(data_file, 'w')
> @@ -649,6 +650,7 @@ python package_do_shlibs() {
>  	shlibs_dir = bb.data.getVar('SHLIBSDIR', d, True)
>  	shlibswork_dir = bb.data.getVar('SHLIBSWORKDIR', d, True)
>  
> +	# Take shared lock since we're only reading, not writing
>  	lf = bb.utils.lockfile(bb.data.expand("${PACKAGELOCK}", d))

Here, however, it is not changed, even though a comment is added to 
say that it is. Was this intentional, or just an oversight?

>  
>  	def linux_so(root, path, file):
> @@ -878,6 +880,7 @@ python package_do_pkgconfig () {
>  							if hdr == 'Requires':
>  								pkgconfig_needed[pkg] += exp.replace(',', ' ').split()
>  
> +	# Take shared lock since we're only reading, not writing
>  	lf = bb.utils.lockfile(bb.data.expand("${PACKAGELOCK}", d))

Here again a comment is added, but the code is not changed to match.

>  
>  	for pkg in packages.split():

Also, what is the ${PACKAGELOCK} lock file actually protecting? With 
the exception of the two questionable cases above, I cannot see that 
the lock is taken privately anywhere else. And since it looks as the 
code in package_do_shlibs() and package_do_pkgconfig() is not what 
needs protection (based on the added comments above), what is?

//Peter



             reply	other threads:[~2017-10-03 18:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-03 18:17 Peter Kjellerstedt [this message]
2017-10-04 15:10 ` Oddness regarding file locks in package.bbclass Burton, Ross
2018-03-23 15:05   ` Trevor Woerner
2018-04-19 22:03 ` Richard Purdie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b720c2022a0a49b08ea3159ef3fdf083@XBOX02.axis.com \
    --to=peter.kjellerstedt@axis.com \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.