All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jérôme Pouiller" <jezz@sysmic.org>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt' with TLP
Date: Fri, 10 Feb 2017 18:40:04 +0100	[thread overview]
Message-ID: <18141971.6S5PrfofuZ@sagittae> (raw)
In-Reply-To: <20170209232429.0c133424@free-electrons.com>

Hello Thomas,

On Thursday 9 February 2017 23:24:29 CET Thomas Petazzoni wrote:
> Hello,
> 
> Adding Gustavo in Cc. Gustavo, you are working on TLP support. Could
> you comment on the below patch?
> 
> See also my comments below.
> 
> On Mon, 14 Nov 2016 14:22:35 +0100, J?r?me Pouiller wrote:
> > Until now, `$(BUILD_DIR)/packages-file-list.txt' was not filled properly
> > when top level parallelization is enabled. Therefore, all scripts that
> > rely on packages-file-list.txt did not work.
> > 
> > In order to fix it,this patch place target installation task in a critical
> > section.
> > 
> > Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> > ---
> > 
> >  package/pkg-generic.mk | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 987efa6..c5f70e0 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -62,6 +62,9 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
> > 
> >  # files currently installed in the target. Note that the MD5 is also
> >  # stored, in order to identify if the files are overwritten.
> >  define step_pkg_size_start
> > 
> > +	while ! flock $(BUILD_DIR) -c "[ ! -e $(BUILD_DIR)/.target_lock ] &&
> > touch $(BUILD_DIR)/.target_lock"; do \ +		sleep 0.5; \
> > +	done
> 
> I personally don't really like this retry loop around flock, but since
> package-file-list.txt is a global file, I don't really see how to do
> otherwise. Unless storing a per-package file with just the time/date of
> the start/end of each step for this package, and then have a
> post-processing logic at the very end of the build that regroups all
> those per-package files into a single global file, ordering the entries
> by their timestamp. Don't know if it's really better.

I think that any tool involving parallel processing need one day or another
to protect code inside a critical section. Even if we find another way to 
solve current problem, I am pretty sure we will need it later, anyway.

However, I think I am going to modify my patch in order to provide a
generic mutex implementation that would be enabled iff TLP is enable.

I wonder where I should place lock file. In add, tools like fakedate,
check-shlibs-deps and other QA tools could also generate log or
temporary files. Currently, we place all of them in build/ without any
specific rules. Maybe we should have a naming policy for these files?
(prefix them with "BR_"?) or place them elsewhere than in build/?

[...]

-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

  reply	other threads:[~2017-02-10 17:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-14 13:22 [Buildroot] [PATCH 0/6] Warn user on missing optional dependencies Jérôme Pouiller
2016-11-14 13:22 ` [Buildroot] [PATCH 1/6] check-shlibs-deps: new script to check shared library dependencies Jérôme Pouiller
2017-02-06 21:04   ` Samuel Martin
2017-02-10 17:22     ` Jérôme Pouiller
2017-02-09 22:21   ` Thomas Petazzoni
2016-11-14 13:22 ` [Buildroot] [PATCH 2/6] pkg-generic: add check_shlibs_deps hooks Jérôme Pouiller
2017-02-06 21:04   ` Samuel Martin
2016-11-14 13:22 ` [Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt' with TLP Jérôme Pouiller
2017-02-09 22:24   ` Thomas Petazzoni
2017-02-10 17:40     ` Jérôme Pouiller [this message]
2017-04-01 14:43   ` Thomas Petazzoni
2016-11-14 13:22 ` [Buildroot] [PATCH 4/6] ntp: fix missing optional dependencies Jérôme Pouiller
2016-11-28 22:24   ` Thomas Petazzoni
2016-11-14 13:22 ` [Buildroot] [PATCH 5/6] xterm: depend on libXinerama if appropriate Jérôme Pouiller
2016-11-14 13:22 ` [Buildroot] [PATCH 6/6] xserver_xorg-server: fix dependency with dbus Jérôme Pouiller
2016-12-17 15:52   ` Thomas Petazzoni

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=18141971.6S5PrfofuZ@sagittae \
    --to=jezz@sysmic.org \
    --cc=buildroot@busybox.net \
    /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.