Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1428092 > unrolled thread
| Started by | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| First post | 2016-06-21 21:30 +0200 |
| Last post | 2016-06-21 23:40 +0200 |
| Articles | 20 on this page of 35 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v3 0/8] coccicheck: modernize "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 21:30 +0200
[PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 21:30 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 22:20 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 22:50 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 23:00 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:20 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 23:30 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:40 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Nicolas Palix (LIG)" <Nicolas.Palix@imag.fr> - 2016-06-21 23:10 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 23:30 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:40 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:50 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-22 00:40 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-22 07:30 +0200
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-22 17:20 +0200
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-22 21:50 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-22 00:00 +0200
Re: [PATCH v3 3/8] coccicheck: enable parmap support Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:30 +0200
[PATCH v3 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 21:30 +0200
Re: [PATCH v3 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 22:40 +0200
Re: [PATCH v3 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci "Nicolas Palix (LIG)" <Nicolas.Palix@imag.fr> - 2016-06-21 23:50 +0200
[PATCH v3 4/8] coccicheck: replace --very-quiet with --quit when debugging "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 21:30 +0200
[PATCH v3 1/8] coccicheck: move spatch binary check up "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 21:30 +0200
Re: [PATCH v3 1/8] coccicheck: move spatch binary check up "Nicolas Palix (LIG)" <Nicolas.Palix@imag.fr> - 2016-06-21 22:40 +0200
[PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 21:30 +0200
Re: [PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation "Nicolas Palix (LIG)" <Nicolas.Palix@imag.fr> - 2016-06-21 23:20 +0200
Re: [PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:20 +0200
Re: [PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-22 02:10 +0200
Re: [PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation Julia Lawall <julia.lawall@lip6.fr> - 2016-06-22 08:10 +0200
Re: [PATCH v3 0/8] coccicheck: modernize Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 22:30 +0200
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 23:00 +0200
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:10 +0200
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 23:30 +0200
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-06-21 23:40 +0200
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize Julia Lawall <julia.lawall@lip6.fr> - 2016-06-21 23:40 +0200
Page 1 of 2 [1] 2 Next page →
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 21:30 +0200 |
| Subject | [PATCH v3 0/8] coccicheck: modernize |
| Message-ID | <rMzwZ-1xY-3@gated-at.bofh.it> |
This v3 series addresses the feedback from the last v2 series
on the coccicheck enhancements [0], namely:
o it drops the indexing heuristics in favor for a .cocciconfig use
o drops glimpse support as its simply not well maintained, recommends
idutils instead.
o adds a Linux .cocciconfig -- the assumption is you'd run spatch when
you're at the top level of the kernel. This has not only the side effect
of picking up .cocciconfig, but also that the coccicheck use of the
make variables passed on are assumed to be correct given the base
directory as the current directory.
o makes SPFLAGS more useful by ensuring it is always at the end of options
o rebases on top of Deepa Dinamani's patch
"coccicheck: Allow for overriding spatch flags", under the assumption this
is already merged. I don't see it on linux-next but I do think Michal
has merged already onto his tree. This is also rebased on to linux-next
next-20160621
These changes are also visible on kernel.org, on a branch based on linux-next
next-20160621 with Deepa's commit merged first.
[0] http://lkml.kernel.org/r/1466116292-21843-1-git-send-email-mcgrof@kernel.org
[1] https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160621-cocciconfig-v3
Luis R. Rodriguez (8):
coccicheck: move spatch binary check up
coccicheck: make SPFLAGS more useful
coccicheck: enable parmap support
coccicheck: replace --very-quiet with --quit when debugging
scripts: add Linux .cocciconfig for coccinelle
coccicheck: add support for requring a coccinelle version
coccicheck: refer to coccicheck bottest wiki for documentation
scripts/coccinelle: require coccinelle >= 1.0.4 on
device_node_continue.cocci
.cocciconfig | 3 +
.gitignore | 1 +
scripts/coccicheck | 98 ++++++++++++++++++++--
.../iterators/device_node_continue.cocci | 3 +
4 files changed, 96 insertions(+), 9 deletions(-)
create mode 100644 .cocciconfig
--
2.8.2
[toc] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 21:30 +0200 |
| Subject | [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMzx0-1xY-9@gated-at.bofh.it> |
| In reply to | #1428092 |
Coccinelle has had parmap support since 1.0.2, this means
it supports --jobs, enabling built-in multithreaded functionality,
instead of needing one to script it out. Just look for --jobs
in the help output to determine if this is supported.
Also enable the load balancing to be dynamic, so that if a
thread finishes early we keep feeding it.
Note: now that we have all things handled for us, redirect stderr to
stdout as well to capture any possible errors or warnings issued by
coccinelle.
If --jobs is not supported we fallback to the old mechanism.
This also now accepts DEBUG_FILE= to specify where you want
stderr to be redirected to, by default we redirect stderr to
/dev/null.
Also since while at it propagate back into the shell script any
coccinelle error code. When used in serialized mode where all cocci
files are run this also stops processing if an error has occured.
This lets us handle some errors in coccinelle cocci files and if
they bail out we should inspect the errors. This will be more
useful later to help annotate coccinelle version dependency
requirements. This will let you run only SmPL files that your
system supports.
As a small example, prior to this change, on an 8-core system:
Before:
$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report DEBUG_FILE=cocci.err
...
real 29m14.912s
user 103m1.796s
sys 0m4.464s
After:
real 16m22.435s
user 128m30.060s
sys 0m2.712s
v3:
o move USE_JOBS to avoid being overriden
v2:
o redirect coccinelle stderr to /dev/null by default and
only if DEBUG_FILE is used do we pass it to a file
o fix typo of paramap/parmap
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
scripts/coccicheck | 41 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/scripts/coccicheck b/scripts/coccicheck
index 5319fae910b4..a77f0f246405 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -12,8 +12,8 @@ if [ ! -x "$SPATCH" ]; then
exit 1
fi
-trap kill_running SIGTERM SIGINT
-declare -a SPATCH_PID
+USE_JOBS="no"
+$SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
# The verbosity may be set by the environmental parameter V=
# as for example with 'make V=1 coccicheck'
@@ -56,6 +56,14 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then
OPTIONS="--patch $srctree $OPTIONS"
fi
+# You can override by using SPFLAGS
+if [ "$USE_JOBS" = "no" ]; then
+ trap kill_running SIGTERM SIGINT
+ declare -a SPATCH_PID
+else
+ OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1"
+fi
+
if [ "$MODE" = "" ] ; then
if [ "$ONLINE" = "0" ] ; then
echo 'You have not explicitly specified the mode to use. Using default "report" mode.'
@@ -82,7 +90,26 @@ if [ "$ONLINE" = "0" ] ; then
echo ''
fi
-run_cmd() {
+run_cmd_parmap() {
+ if [ $VERBOSE -ne 0 ] ; then
+ echo "Running ($NPROC in parallel): $@"
+ fi
+ if [ "$DEBUG_FILE" != "/dev/null" -a "$DEBUG_FILE" != "" ]; then
+ if [ -f $DEBUG_FILE ]; then
+ echo "Debug file $DEBUG_FILE exists, bailing"
+ exit
+ fi
+ else
+ DEBUG_FILE="/dev/null"
+ fi
+ $@ 2>$DEBUG_FILE
+ if [[ $? -ne 0 ]]; then
+ echo "coccicheck failed"
+ exit $?
+ fi
+}
+
+run_cmd_old() {
local i
if [ $VERBOSE -ne 0 ] ; then
echo "Running ($NPROC in parallel): $@"
@@ -97,6 +124,14 @@ run_cmd() {
wait
}
+run_cmd() {
+ if [ "$USE_JOBS" = "yes" ]; then
+ run_cmd_parmap $@
+ else
+ run_cmd_old $@
+ fi
+}
+
kill_running() {
for i in $(seq 0 $(( NPROC - 1 )) ); do
if [ $VERBOSE -eq 2 ] ; then
--
2.8.2
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 22:20 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMAjn-264-9@gated-at.bofh.it> |
| In reply to | #1428093 |
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote:
> Coccinelle has had parmap support since 1.0.2, this means
> it supports --jobs, enabling built-in multithreaded functionality,
> instead of needing one to script it out. Just look for --jobs
> in the help output to determine if this is supported.
>
> Also enable the load balancing to be dynamic, so that if a
> thread finishes early we keep feeding it.
>
> Note: now that we have all things handled for us, redirect stderr to
> stdout as well to capture any possible errors or warnings issued by
> coccinelle.
>
> If --jobs is not supported we fallback to the old mechanism.
> This also now accepts DEBUG_FILE= to specify where you want
> stderr to be redirected to, by default we redirect stderr to
> /dev/null.
Why do you want to do something different for standard error in the parmap
and nonparmap case?
julia
> Also since while at it propagate back into the shell script any
> coccinelle error code. When used in serialized mode where all cocci
> files are run this also stops processing if an error has occured.
> This lets us handle some errors in coccinelle cocci files and if
> they bail out we should inspect the errors. This will be more
> useful later to help annotate coccinelle version dependency
> requirements. This will let you run only SmPL files that your
> system supports.
>
> As a small example, prior to this change, on an 8-core system:
>
> Before:
>
> $ export COCCI=scripts/coccinelle/free/kfree.cocci
> $ time make coccicheck MODE=report DEBUG_FILE=cocci.err
> ...
>
> real 29m14.912s
> user 103m1.796s
> sys 0m4.464s
>
> After:
>
> real 16m22.435s
> user 128m30.060s
> sys 0m2.712s
>
> v3:
>
> o move USE_JOBS to avoid being overriden
>
> v2:
>
> o redirect coccinelle stderr to /dev/null by default and
> only if DEBUG_FILE is used do we pass it to a file
> o fix typo of paramap/parmap
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> scripts/coccicheck | 41 ++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 5319fae910b4..a77f0f246405 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -12,8 +12,8 @@ if [ ! -x "$SPATCH" ]; then
> exit 1
> fi
>
> -trap kill_running SIGTERM SIGINT
> -declare -a SPATCH_PID
> +USE_JOBS="no"
> +$SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
>
> # The verbosity may be set by the environmental parameter V=
> # as for example with 'make V=1 coccicheck'
> @@ -56,6 +56,14 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then
> OPTIONS="--patch $srctree $OPTIONS"
> fi
>
> +# You can override by using SPFLAGS
> +if [ "$USE_JOBS" = "no" ]; then
> + trap kill_running SIGTERM SIGINT
> + declare -a SPATCH_PID
> +else
> + OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1"
> +fi
> +
> if [ "$MODE" = "" ] ; then
> if [ "$ONLINE" = "0" ] ; then
> echo 'You have not explicitly specified the mode to use. Using default "report" mode.'
> @@ -82,7 +90,26 @@ if [ "$ONLINE" = "0" ] ; then
> echo ''
> fi
>
> -run_cmd() {
> +run_cmd_parmap() {
> + if [ $VERBOSE -ne 0 ] ; then
> + echo "Running ($NPROC in parallel): $@"
> + fi
> + if [ "$DEBUG_FILE" != "/dev/null" -a "$DEBUG_FILE" != "" ]; then
> + if [ -f $DEBUG_FILE ]; then
> + echo "Debug file $DEBUG_FILE exists, bailing"
> + exit
> + fi
> + else
> + DEBUG_FILE="/dev/null"
> + fi
> + $@ 2>$DEBUG_FILE
> + if [[ $? -ne 0 ]]; then
> + echo "coccicheck failed"
> + exit $?
> + fi
> +}
> +
> +run_cmd_old() {
> local i
> if [ $VERBOSE -ne 0 ] ; then
> echo "Running ($NPROC in parallel): $@"
> @@ -97,6 +124,14 @@ run_cmd() {
> wait
> }
>
> +run_cmd() {
> + if [ "$USE_JOBS" = "yes" ]; then
> + run_cmd_parmap $@
> + else
> + run_cmd_old $@
> + fi
> +}
> +
> kill_running() {
> for i in $(seq 0 $(( NPROC - 1 )) ); do
> if [ $VERBOSE -eq 2 ] ; then
> --
> 2.8.2
>
>
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 22:50 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMAMq-2gO-23@gated-at.bofh.it> |
| In reply to | #1428132 |
On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > Coccinelle has had parmap support since 1.0.2, this means > > it supports --jobs, enabling built-in multithreaded functionality, > > instead of needing one to script it out. Just look for --jobs > > in the help output to determine if this is supported. > > > > Also enable the load balancing to be dynamic, so that if a > > thread finishes early we keep feeding it. > > > > Note: now that we have all things handled for us, redirect stderr to > > stdout as well to capture any possible errors or warnings issued by > > coccinelle. > > > > If --jobs is not supported we fallback to the old mechanism. > > This also now accepts DEBUG_FILE= to specify where you want > > stderr to be redirected to, by default we redirect stderr to > > /dev/null. > > Why do you want to do something different for standard error in the parmap > and nonparmap case? We should just deprecate non-parmap later. Luis
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 23:00 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMAW5-2lh-5@gated-at.bofh.it> |
| In reply to | #1428149 |
On Tue, Jun 21, 2016 at 10:43:04PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > Coccinelle has had parmap support since 1.0.2, this means > > > > it supports --jobs, enabling built-in multithreaded functionality, > > > > instead of needing one to script it out. Just look for --jobs > > > > in the help output to determine if this is supported. > > > > > > > > Also enable the load balancing to be dynamic, so that if a > > > > thread finishes early we keep feeding it. > > > > > > > > Note: now that we have all things handled for us, redirect stderr to > > > > stdout as well to capture any possible errors or warnings issued by > > > > coccinelle. > > > > > > > > If --jobs is not supported we fallback to the old mechanism. > > > > This also now accepts DEBUG_FILE= to specify where you want > > > > stderr to be redirected to, by default we redirect stderr to > > > > /dev/null. > > > > > > Why do you want to do something different for standard error in the parmap > > > and nonparmap case? > > > > We should just deprecate non-parmap later. > > that's not really getting at the point. I like the DEBUG_FILE= solution. > I don't like merging stderr and stdout. So you've put what to my mind is > the good solution only in the deprecated case (to my understanding of > the commit message). stderr is not being merged to stdout though. By default stderr goes to /dev/null and if you want it you specify a DEBUG_FILE. What will be deprecated has no clean solution for any of this and its unclear exactly what happens given separate processes are run in the background and we just wait. Luis
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 23:20 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMBfr-2Hb-5@gated-at.bofh.it> |
| In reply to | #1428158 |
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > On Tue, Jun 21, 2016 at 10:43:04PM +0200, Julia Lawall wrote: > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > Coccinelle has had parmap support since 1.0.2, this means > > > > > it supports --jobs, enabling built-in multithreaded functionality, > > > > > instead of needing one to script it out. Just look for --jobs > > > > > in the help output to determine if this is supported. > > > > > > > > > > Also enable the load balancing to be dynamic, so that if a > > > > > thread finishes early we keep feeding it. > > > > > > > > > > Note: now that we have all things handled for us, redirect stderr to > > > > > stdout as well to capture any possible errors or warnings issued by > > > > > coccinelle. > > > > > > > > > > If --jobs is not supported we fallback to the old mechanism. > > > > > This also now accepts DEBUG_FILE= to specify where you want > > > > > stderr to be redirected to, by default we redirect stderr to > > > > > /dev/null. > > > > > > > > Why do you want to do something different for standard error in the parmap > > > > and nonparmap case? > > > > > > We should just deprecate non-parmap later. > > > > that's not really getting at the point. I like the DEBUG_FILE= solution. > > I don't like merging stderr and stdout. So you've put what to my mind is > > the good solution only in the deprecated case (to my understanding of > > the commit message). > > stderr is not being merged to stdout though. By default stderr goes to /dev/null > and if you want it you specify a DEBUG_FILE. Above it says: Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle. If DEBUG_FILE is an option for the parmap case, it should be mentioned there too. julia > > What will be deprecated has no clean solution for any of this and its unclear > exactly what happens given separate processes are run in the background > and we just wait. > > Luis >
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 23:30 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMBp7-2Kp-9@gated-at.bofh.it> |
| In reply to | #1428169 |
On Tue, Jun 21, 2016 at 11:10:00PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > On Tue, Jun 21, 2016 at 10:43:04PM +0200, Julia Lawall wrote: > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > Coccinelle has had parmap support since 1.0.2, this means > > > > > > it supports --jobs, enabling built-in multithreaded functionality, > > > > > > instead of needing one to script it out. Just look for --jobs > > > > > > in the help output to determine if this is supported. > > > > > > > > > > > > Also enable the load balancing to be dynamic, so that if a > > > > > > thread finishes early we keep feeding it. > > > > > > > > > > > > Note: now that we have all things handled for us, redirect stderr to > > > > > > stdout as well to capture any possible errors or warnings issued by > > > > > > coccinelle. > > > > > > > > > > > > If --jobs is not supported we fallback to the old mechanism. > > > > > > This also now accepts DEBUG_FILE= to specify where you want > > > > > > stderr to be redirected to, by default we redirect stderr to > > > > > > /dev/null. > > > > > > > > > > Why do you want to do something different for standard error in the parmap > > > > > and nonparmap case? > > > > > > > > We should just deprecate non-parmap later. > > > > > > that's not really getting at the point. I like the DEBUG_FILE= solution. > > > I don't like merging stderr and stdout. So you've put what to my mind is > > > the good solution only in the deprecated case (to my understanding of > > > the commit message). > > > > stderr is not being merged to stdout though. By default stderr goes to /dev/null > > and if you want it you specify a DEBUG_FILE. > > Above it says: > > Note: now that we have all things handled for us, redirect stderr to > stdout as well to capture any possible errors or warnings issued by > coccinelle. Ah crap, sorry I left that in the commit log message by mistake. > If DEBUG_FILE is an option for the parmap case, it should be mentioned > there too. Indeed, its as described instead, stderr goes to /dev/null by default unless DEBUG_FILE is specified, if it is specified then stderr goes to DEBUG_FILE. Luis
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 23:40 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMByN-2NM-17@gated-at.bofh.it> |
| In reply to | #1428183 |
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > On Tue, Jun 21, 2016 at 11:10:00PM +0200, Julia Lawall wrote: > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > On Tue, Jun 21, 2016 at 10:43:04PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > > > Coccinelle has had parmap support since 1.0.2, this means > > > > > > > it supports --jobs, enabling built-in multithreaded functionality, > > > > > > > instead of needing one to script it out. Just look for --jobs > > > > > > > in the help output to determine if this is supported. > > > > > > > > > > > > > > Also enable the load balancing to be dynamic, so that if a > > > > > > > thread finishes early we keep feeding it. > > > > > > > > > > > > > > Note: now that we have all things handled for us, redirect stderr to > > > > > > > stdout as well to capture any possible errors or warnings issued by > > > > > > > coccinelle. > > > > > > > > > > > > > > If --jobs is not supported we fallback to the old mechanism. > > > > > > > This also now accepts DEBUG_FILE= to specify where you want > > > > > > > stderr to be redirected to, by default we redirect stderr to > > > > > > > /dev/null. > > > > > > > > > > > > Why do you want to do something different for standard error in the parmap > > > > > > and nonparmap case? > > > > > > > > > > We should just deprecate non-parmap later. > > > > > > > > that's not really getting at the point. I like the DEBUG_FILE= solution. > > > > I don't like merging stderr and stdout. So you've put what to my mind is > > > > the good solution only in the deprecated case (to my understanding of > > > > the commit message). > > > > > > stderr is not being merged to stdout though. By default stderr goes to /dev/null > > > and if you want it you specify a DEBUG_FILE. > > > > Above it says: > > > > Note: now that we have all things handled for us, redirect stderr to > > stdout as well to capture any possible errors or warnings issued by > > coccinelle. > > Ah crap, sorry I left that in the commit log message by mistake. > > > If DEBUG_FILE is an option for the parmap case, it should be mentioned > > there too. > > Indeed, its as described instead, stderr goes to /dev/null by default > unless DEBUG_FILE is specified, if it is specified then stderr goes > to DEBUG_FILE. OK, good. julia
[toc] | [prev] | [next] | [standalone]
| From | "Nicolas Palix (LIG)" <Nicolas.Palix@imag.fr> |
|---|---|
| Date | 2016-06-21 23:10 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMB5L-2DJ-5@gated-at.bofh.it> |
| In reply to | #1428149 |
[Multipart message — attachments visible in raw view] — view raw
Hi, Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > >> On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: >>> >>> >>> On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: >>> >>>> Coccinelle has had parmap support since 1.0.2, this means >>>> it supports --jobs, enabling built-in multithreaded functionality, >>>> instead of needing one to script it out. Just look for --jobs >>>> in the help output to determine if this is supported. >>>> >>>> Also enable the load balancing to be dynamic, so that if a >>>> thread finishes early we keep feeding it. >>>> >>>> Note: now that we have all things handled for us, redirect stderr to >>>> stdout as well to capture any possible errors or warnings issued by >>>> coccinelle. >>>> >>>> If --jobs is not supported we fallback to the old mechanism. >>>> This also now accepts DEBUG_FILE= to specify where you want >>>> stderr to be redirected to, by default we redirect stderr to >>>> /dev/null. >>> >>> Why do you want to do something different for standard error in the parmap >>> and nonparmap case? >> >> We should just deprecate non-parmap later. > > that's not really getting at the point. I like the DEBUG_FILE= solution. > I don't like merging stderr and stdout. So you've put what to my mind is > the good solution only in the deprecated case (to my understanding of > the commit message). I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap). > > julia > > -- Nicolas Palix http://lig-membres.imag.fr/palix/
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 23:30 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMBp8-2Kp-31@gated-at.bofh.it> |
| In reply to | #1428163 |
On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > Hi, > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > >>> > >>> > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > >>> > >>>>Coccinelle has had parmap support since 1.0.2, this means > >>>>it supports --jobs, enabling built-in multithreaded functionality, > >>>>instead of needing one to script it out. Just look for --jobs > >>>>in the help output to determine if this is supported. > >>>> > >>>>Also enable the load balancing to be dynamic, so that if a > >>>>thread finishes early we keep feeding it. > >>>> > >>>>Note: now that we have all things handled for us, redirect stderr to > >>>>stdout as well to capture any possible errors or warnings issued by > >>>>coccinelle. > >>>> > >>>>If --jobs is not supported we fallback to the old mechanism. > >>>>This also now accepts DEBUG_FILE= to specify where you want > >>>>stderr to be redirected to, by default we redirect stderr to > >>>>/dev/null. > >>> > >>>Why do you want to do something different for standard error in the parmap > >>>and nonparmap case? > >> > >>We should just deprecate non-parmap later. > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > >I don't like merging stderr and stdout. So you've put what to my mind is > >the good solution only in the deprecated case (to my understanding of > >the commit message). > > I agree. You're not just "enabling parmap support". You're > also changing how messages to stderr are handled. > Maybe add the DEBUG_FILE mechanism in a separate patch for both > modes (parmap and non-parmap). I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts? Luis
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 23:40 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMByN-2NM-19@gated-at.bofh.it> |
| In reply to | #1428190 |
[Multipart message — attachments visible in raw view] — view raw
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > > Hi, > > > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > >>> > > >>> > > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > >>> > > >>>>Coccinelle has had parmap support since 1.0.2, this means > > >>>>it supports --jobs, enabling built-in multithreaded functionality, > > >>>>instead of needing one to script it out. Just look for --jobs > > >>>>in the help output to determine if this is supported. > > >>>> > > >>>>Also enable the load balancing to be dynamic, so that if a > > >>>>thread finishes early we keep feeding it. > > >>>> > > >>>>Note: now that we have all things handled for us, redirect stderr to > > >>>>stdout as well to capture any possible errors or warnings issued by > > >>>>coccinelle. > > >>>> > > >>>>If --jobs is not supported we fallback to the old mechanism. > > >>>>This also now accepts DEBUG_FILE= to specify where you want > > >>>>stderr to be redirected to, by default we redirect stderr to > > >>>>/dev/null. > > >>> > > >>>Why do you want to do something different for standard error in the parmap > > >>>and nonparmap case? > > >> > > >>We should just deprecate non-parmap later. > > > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > > >I don't like merging stderr and stdout. So you've put what to my mind is > > >the good solution only in the deprecated case (to my understanding of > > >the commit message). > > > > I agree. You're not just "enabling parmap support". You're > > also changing how messages to stderr are handled. > > Maybe add the DEBUG_FILE mechanism in a separate patch for both > > modes (parmap and non-parmap). > > I'd prefer to just rip out non-parmap support and bump coccinelle > requiremetns to at least 1.0.3, thoughts? There are already too many changes in this patch series. Also, I don't know what the 0-day people would find convenient. julia
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 23:50 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMBIt-2Rm-1@gated-at.bofh.it> |
| In reply to | #1428201 |
[Multipart message — attachments visible in raw view] — view raw
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > On Tue, Jun 21, 2016 at 11:32:11PM +0200, Julia Lawall wrote: > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > > > > Hi, > > > > > > > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > > > > > > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > >>> > > > > >>> > > > > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > >>> > > > > >>>>Coccinelle has had parmap support since 1.0.2, this means > > > > >>>>it supports --jobs, enabling built-in multithreaded functionality, > > > > >>>>instead of needing one to script it out. Just look for --jobs > > > > >>>>in the help output to determine if this is supported. > > > > >>>> > > > > >>>>Also enable the load balancing to be dynamic, so that if a > > > > >>>>thread finishes early we keep feeding it. > > > > >>>> > > > > >>>>Note: now that we have all things handled for us, redirect stderr to > > > > >>>>stdout as well to capture any possible errors or warnings issued by > > > > >>>>coccinelle. > > > > >>>> > > > > >>>>If --jobs is not supported we fallback to the old mechanism. > > > > >>>>This also now accepts DEBUG_FILE= to specify where you want > > > > >>>>stderr to be redirected to, by default we redirect stderr to > > > > >>>>/dev/null. > > > > >>> > > > > >>>Why do you want to do something different for standard error in the parmap > > > > >>>and nonparmap case? > > > > >> > > > > >>We should just deprecate non-parmap later. > > > > > > > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > > > > >I don't like merging stderr and stdout. So you've put what to my mind is > > > > >the good solution only in the deprecated case (to my understanding of > > > > >the commit message). > > > > > > > > I agree. You're not just "enabling parmap support". You're > > > > also changing how messages to stderr are handled. > > > > Maybe add the DEBUG_FILE mechanism in a separate patch for both > > > > modes (parmap and non-parmap). > > > > > > I'd prefer to just rip out non-parmap support and bump coccinelle > > > requiremetns to at least 1.0.3, thoughts? > > > > There are already too many changes in this patch series. > > > > Also, I don't know what the 0-day people would find convenient. > > I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap > case due to the way parallelism is supported there, it uses wait(1) to > wait on the shell, and for spawning this nasty thing: > > eval "$@ --max $NPROC --index $i &" > > Specially since we are likely to be able to deprecate this sooner > rather than later I see little point in adding DEBUG_FILE into this > mess. Sorry, I didn't realize there was parallelism without parmap. My thought was that if someone is running Coccinelle on only one core, then why force them to use parmap. Coccinelle could of course be updated to not use parmap when the number of cores is 1. julia
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-22 00:40 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMCuS-3oV-13@gated-at.bofh.it> |
| In reply to | #1428206 |
On Tue, Jun 21, 2016 at 11:44:09PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > On Tue, Jun 21, 2016 at 11:32:11PM +0200, Julia Lawall wrote: > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > > > > > Hi, > > > > > > > > > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > > > > > > > > > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > >>> > > > > > >>> > > > > > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > >>> > > > > > >>>>Coccinelle has had parmap support since 1.0.2, this means > > > > > >>>>it supports --jobs, enabling built-in multithreaded functionality, > > > > > >>>>instead of needing one to script it out. Just look for --jobs > > > > > >>>>in the help output to determine if this is supported. > > > > > >>>> > > > > > >>>>Also enable the load balancing to be dynamic, so that if a > > > > > >>>>thread finishes early we keep feeding it. > > > > > >>>> > > > > > >>>>Note: now that we have all things handled for us, redirect stderr to > > > > > >>>>stdout as well to capture any possible errors or warnings issued by > > > > > >>>>coccinelle. > > > > > >>>> > > > > > >>>>If --jobs is not supported we fallback to the old mechanism. > > > > > >>>>This also now accepts DEBUG_FILE= to specify where you want > > > > > >>>>stderr to be redirected to, by default we redirect stderr to > > > > > >>>>/dev/null. > > > > > >>> > > > > > >>>Why do you want to do something different for standard error in the parmap > > > > > >>>and nonparmap case? > > > > > >> > > > > > >>We should just deprecate non-parmap later. > > > > > > > > > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > > > > > >I don't like merging stderr and stdout. So you've put what to my mind is > > > > > >the good solution only in the deprecated case (to my understanding of > > > > > >the commit message). > > > > > > > > > > I agree. You're not just "enabling parmap support". You're > > > > > also changing how messages to stderr are handled. > > > > > Maybe add the DEBUG_FILE mechanism in a separate patch for both > > > > > modes (parmap and non-parmap). > > > > > > > > I'd prefer to just rip out non-parmap support and bump coccinelle > > > > requiremetns to at least 1.0.3, thoughts? > > > > > > There are already too many changes in this patch series. > > > > > > Also, I don't know what the 0-day people would find convenient. > > > > I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap > > case due to the way parallelism is supported there, it uses wait(1) to > > wait on the shell, and for spawning this nasty thing: > > > > eval "$@ --max $NPROC --index $i &" > > > > Specially since we are likely to be able to deprecate this sooner > > rather than later I see little point in adding DEBUG_FILE into this > > mess. > > Sorry, I didn't realize there was parallelism without parmap. Yea :( so is the change OK as-is then, only I need to update the commit log? > My thought > was that if someone is running Coccinelle on only one core, then why force > them to use parmap. Oh but that's different feedback. Sure, but why should that be an issue ? It would seem that coccinelle would just do the right thing with -j 1 used. > Coccinelle could of course be updated to not use > parmap when the number of cores is 1. :) Single CPU systems are probably odd bests these days, either way I can update the script to avoid parmap if number of cpus is 1 since I'm respinning. Luis
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-22 07:30 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMITD-7ER-1@gated-at.bofh.it> |
| In reply to | #1428239 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, 22 Jun 2016, Luis R. Rodriguez wrote: > On Tue, Jun 21, 2016 at 11:44:09PM +0200, Julia Lawall wrote: > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > On Tue, Jun 21, 2016 at 11:32:11PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > > > > > > Hi, > > > > > > > > > > > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > > > > > > > > > > > > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > >>> > > > > > > >>> > > > > > > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > >>> > > > > > > >>>>Coccinelle has had parmap support since 1.0.2, this means > > > > > > >>>>it supports --jobs, enabling built-in multithreaded functionality, > > > > > > >>>>instead of needing one to script it out. Just look for --jobs > > > > > > >>>>in the help output to determine if this is supported. > > > > > > >>>> > > > > > > >>>>Also enable the load balancing to be dynamic, so that if a > > > > > > >>>>thread finishes early we keep feeding it. > > > > > > >>>> > > > > > > >>>>Note: now that we have all things handled for us, redirect stderr to > > > > > > >>>>stdout as well to capture any possible errors or warnings issued by > > > > > > >>>>coccinelle. > > > > > > >>>> > > > > > > >>>>If --jobs is not supported we fallback to the old mechanism. > > > > > > >>>>This also now accepts DEBUG_FILE= to specify where you want > > > > > > >>>>stderr to be redirected to, by default we redirect stderr to > > > > > > >>>>/dev/null. > > > > > > >>> > > > > > > >>>Why do you want to do something different for standard error in the parmap > > > > > > >>>and nonparmap case? > > > > > > >> > > > > > > >>We should just deprecate non-parmap later. > > > > > > > > > > > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > > > > > > >I don't like merging stderr and stdout. So you've put what to my mind is > > > > > > >the good solution only in the deprecated case (to my understanding of > > > > > > >the commit message). > > > > > > > > > > > > I agree. You're not just "enabling parmap support". You're > > > > > > also changing how messages to stderr are handled. > > > > > > Maybe add the DEBUG_FILE mechanism in a separate patch for both > > > > > > modes (parmap and non-parmap). > > > > > > > > > > I'd prefer to just rip out non-parmap support and bump coccinelle > > > > > requiremetns to at least 1.0.3, thoughts? > > > > > > > > There are already too many changes in this patch series. > > > > > > > > Also, I don't know what the 0-day people would find convenient. > > > > > > I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap > > > case due to the way parallelism is supported there, it uses wait(1) to > > > wait on the shell, and for spawning this nasty thing: > > > > > > eval "$@ --max $NPROC --index $i &" > > > > > > Specially since we are likely to be able to deprecate this sooner > > > rather than later I see little point in adding DEBUG_FILE into this > > > mess. > > > > Sorry, I didn't realize there was parallelism without parmap. > > Yea :( so is the change OK as-is then, only I need to update the commit log? > > > My thought > > was that if someone is running Coccinelle on only one core, then why force > > them to use parmap. > > Oh but that's different feedback. Sure, but why should that be an issue ? > It would seem that coccinelle would just do the right thing with -j 1 used. > > > Coccinelle could of course be updated to not use > > parmap when the number of cores is 1. > > :) Single CPU systems are probably odd bests these days, either way I can > update the script to avoid parmap if number of cpus is 1 since I'm respinning. Some semantic patches have to be run single core, eg due to the use of finalize. Perhaps there would be some reason to run them single core, if one had the same nmber of semantic patches as cores. This was more relevant before dynamic load balancing though. Single core is also better when using an option that takes a lot of include files and when using --include-headers-for-types. Then one has maximal sharing of include file information across the treatment of the different C files. In contrast, chunksize 1 is worst. In that case, there is no effective caching of parsed header files, because Coccinelle has no shared memory. Actually, it would be probably good to raise the default chunksize a bit for the latter reason. It would depend on which files get assigned to which chunks though how much benefit it might have. julia
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-22 17:20 +0200 |
| Subject | Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMS6B-57o-5@gated-at.bofh.it> |
| In reply to | #1428422 |
On Wed, Jun 22, 2016 at 07:25:11AM +0200, Julia Lawall wrote: > > > On Wed, 22 Jun 2016, Luis R. Rodriguez wrote: > > > On Tue, Jun 21, 2016 at 11:44:09PM +0200, Julia Lawall wrote: > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > On Tue, Jun 21, 2016 at 11:32:11PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > > > > > > > > > > > > > > > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > > > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > > >>> > > > > > > > >>> > > > > > > > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > >>> > > > > > > > >>>>Coccinelle has had parmap support since 1.0.2, this means > > > > > > > >>>>it supports --jobs, enabling built-in multithreaded functionality, > > > > > > > >>>>instead of needing one to script it out. Just look for --jobs > > > > > > > >>>>in the help output to determine if this is supported. > > > > > > > >>>> > > > > > > > >>>>Also enable the load balancing to be dynamic, so that if a > > > > > > > >>>>thread finishes early we keep feeding it. > > > > > > > >>>> > > > > > > > >>>>Note: now that we have all things handled for us, redirect stderr to > > > > > > > >>>>stdout as well to capture any possible errors or warnings issued by > > > > > > > >>>>coccinelle. > > > > > > > >>>> > > > > > > > >>>>If --jobs is not supported we fallback to the old mechanism. > > > > > > > >>>>This also now accepts DEBUG_FILE= to specify where you want > > > > > > > >>>>stderr to be redirected to, by default we redirect stderr to > > > > > > > >>>>/dev/null. > > > > > > > >>> > > > > > > > >>>Why do you want to do something different for standard error in the parmap > > > > > > > >>>and nonparmap case? > > > > > > > >> > > > > > > > >>We should just deprecate non-parmap later. > > > > > > > > > > > > > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > > > > > > > >I don't like merging stderr and stdout. So you've put what to my mind is > > > > > > > >the good solution only in the deprecated case (to my understanding of > > > > > > > >the commit message). > > > > > > > > > > > > > > I agree. You're not just "enabling parmap support". You're > > > > > > > also changing how messages to stderr are handled. > > > > > > > Maybe add the DEBUG_FILE mechanism in a separate patch for both > > > > > > > modes (parmap and non-parmap). > > > > > > > > > > > > I'd prefer to just rip out non-parmap support and bump coccinelle > > > > > > requiremetns to at least 1.0.3, thoughts? > > > > > > > > > > There are already too many changes in this patch series. > > > > > > > > > > Also, I don't know what the 0-day people would find convenient. > > > > > > > > I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap > > > > case due to the way parallelism is supported there, it uses wait(1) to > > > > wait on the shell, and for spawning this nasty thing: > > > > > > > > eval "$@ --max $NPROC --index $i &" > > > > > > > > Specially since we are likely to be able to deprecate this sooner > > > > rather than later I see little point in adding DEBUG_FILE into this > > > > mess. > > > > > > Sorry, I didn't realize there was parallelism without parmap. > > > > Yea :( so is the change OK as-is then, only I need to update the commit log? > > > > > My thought > > > was that if someone is running Coccinelle on only one core, then why force > > > them to use parmap. > > > > Oh but that's different feedback. Sure, but why should that be an issue ? > > It would seem that coccinelle would just do the right thing with -j 1 used. > > > > > Coccinelle could of course be updated to not use > > > parmap when the number of cores is 1. > > > > :) Single CPU systems are probably odd bests these days, either way I can > > update the script to avoid parmap if number of cpus is 1 since I'm respinning. > > Some semantic patches have to be run single core, eg due to the use of > finalize. Perhaps there would be some reason to run them single core, if > one had the same nmber of semantic patches as cores. This was more > relevant before dynamic load balancing though. Single core is also better > when using an option that takes a lot of include files and when using > --include-headers-for-types. Then one has maximal sharing of include file > information across the treatment of the different C files. In contrast, > chunksize 1 is worst. In that case, there is no effective caching of > parsed header files, because Coccinelle has no shared memory. I've disabled parmap for 1 CPU now. > Actually, it would be probably good to raise the default chunksize a bit > for the latter reason. It would depend on which files get assigned to > which chunks though how much benefit it might have. What value do you have in mind? Or should we leave this as a separate future change? Luis
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-22 21:50 +0200 |
| Subject | Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMWjT-7HS-5@gated-at.bofh.it> |
| In reply to | #1428886 |
> > Actually, it would be probably good to raise the default chunksize a bit > > for the latter reason. It would depend on which files get assigned to > > which chunks though how much benefit it might have. > > What value do you have in mind? Or should we leave this as a separate future > change? It's not urgent. Experiments would have to be done to find a good tradeoff. If the value is too large, there is the risk of a load imbalance at the end. julia
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-22 00:00 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMBIt-2Rm-3@gated-at.bofh.it> |
| In reply to | #1428201 |
On Tue, Jun 21, 2016 at 11:32:11PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > On Tue, Jun 21, 2016 at 11:00:53PM +0200, Nicolas Palix (LIG) wrote: > > > Hi, > > > > > > Le 21/06/16 à 22:43, Julia Lawall a écrit : > > > > > > > > > > > >On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > >>On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > >>> > > > >>> > > > >>>On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > >>> > > > >>>>Coccinelle has had parmap support since 1.0.2, this means > > > >>>>it supports --jobs, enabling built-in multithreaded functionality, > > > >>>>instead of needing one to script it out. Just look for --jobs > > > >>>>in the help output to determine if this is supported. > > > >>>> > > > >>>>Also enable the load balancing to be dynamic, so that if a > > > >>>>thread finishes early we keep feeding it. > > > >>>> > > > >>>>Note: now that we have all things handled for us, redirect stderr to > > > >>>>stdout as well to capture any possible errors or warnings issued by > > > >>>>coccinelle. > > > >>>> > > > >>>>If --jobs is not supported we fallback to the old mechanism. > > > >>>>This also now accepts DEBUG_FILE= to specify where you want > > > >>>>stderr to be redirected to, by default we redirect stderr to > > > >>>>/dev/null. > > > >>> > > > >>>Why do you want to do something different for standard error in the parmap > > > >>>and nonparmap case? > > > >> > > > >>We should just deprecate non-parmap later. > > > > > > > >that's not really getting at the point. I like the DEBUG_FILE= solution. > > > >I don't like merging stderr and stdout. So you've put what to my mind is > > > >the good solution only in the deprecated case (to my understanding of > > > >the commit message). > > > > > > I agree. You're not just "enabling parmap support". You're > > > also changing how messages to stderr are handled. > > > Maybe add the DEBUG_FILE mechanism in a separate patch for both > > > modes (parmap and non-parmap). > > > > I'd prefer to just rip out non-parmap support and bump coccinelle > > requiremetns to at least 1.0.3, thoughts? > > There are already too many changes in this patch series. > > Also, I don't know what the 0-day people would find convenient. I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap case due to the way parallelism is supported there, it uses wait(1) to wait on the shell, and for spawning this nasty thing: eval "$@ --max $NPROC --index $i &" Specially since we are likely to be able to deprecate this sooner rather than later I see little point in adding DEBUG_FILE into this mess. Luis
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 23:30 +0200 |
| Subject | Re: [PATCH v3 3/8] coccicheck: enable parmap support |
| Message-ID | <rMAW5-2lh-7@gated-at.bofh.it> |
| In reply to | #1428149 |
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > On Tue, Jun 21, 2016 at 10:17:38PM +0200, Julia Lawall wrote: > > > > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > > Coccinelle has had parmap support since 1.0.2, this means > > > it supports --jobs, enabling built-in multithreaded functionality, > > > instead of needing one to script it out. Just look for --jobs > > > in the help output to determine if this is supported. > > > > > > Also enable the load balancing to be dynamic, so that if a > > > thread finishes early we keep feeding it. > > > > > > Note: now that we have all things handled for us, redirect stderr to > > > stdout as well to capture any possible errors or warnings issued by > > > coccinelle. > > > > > > If --jobs is not supported we fallback to the old mechanism. > > > This also now accepts DEBUG_FILE= to specify where you want > > > stderr to be redirected to, by default we redirect stderr to > > > /dev/null. > > > > Why do you want to do something different for standard error in the parmap > > and nonparmap case? > > We should just deprecate non-parmap later. that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message). julia
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2016-06-21 21:30 +0200 |
| Subject | [PATCH v3 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci |
| Message-ID | <rMzx0-1xY-17@gated-at.bofh.it> |
| In reply to | #1428092 |
Make use of the new Requires: tag to be able to specify coccinelle binary version requirements. The cocci file device_node_continue.cocci requires at least coccinelle 1.0.4. Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> --- scripts/coccinelle/iterators/device_node_continue.cocci | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/coccinelle/iterators/device_node_continue.cocci b/scripts/coccinelle/iterators/device_node_continue.cocci index 38ab744a4037..a36c16db171b 100644 --- a/scripts/coccinelle/iterators/device_node_continue.cocci +++ b/scripts/coccinelle/iterators/device_node_continue.cocci @@ -5,8 +5,11 @@ // Copyright: (C) 2015 Julia Lawall, Inria. GPLv2. // URL: http://coccinelle.lip6.fr/ // Options: --no-includes --include-headers +// Requires: 1.0.4 // Keywords: for_each_child_of_node, etc. +// This uses a conjunction, which requires at least coccinelle >= 1.0.4 + virtual patch virtual context virtual org -- 2.8.2
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2016-06-21 22:40 +0200 |
| Subject | Re: [PATCH v3 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci |
| Message-ID | <rMACJ-2cL-1@gated-at.bofh.it> |
| In reply to | #1428094 |
On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > Make use of the new Requires: tag to be able to specify coccinelle binary > version requirements. The cocci file device_node_continue.cocci requires at > least coccinelle 1.0.4. > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> Acked-by: Julia Lawall <julia.lawall@lip6.fr> > --- > scripts/coccinelle/iterators/device_node_continue.cocci | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/scripts/coccinelle/iterators/device_node_continue.cocci b/scripts/coccinelle/iterators/device_node_continue.cocci > index 38ab744a4037..a36c16db171b 100644 > --- a/scripts/coccinelle/iterators/device_node_continue.cocci > +++ b/scripts/coccinelle/iterators/device_node_continue.cocci > @@ -5,8 +5,11 @@ > // Copyright: (C) 2015 Julia Lawall, Inria. GPLv2. > // URL: http://coccinelle.lip6.fr/ > // Options: --no-includes --include-headers > +// Requires: 1.0.4 > // Keywords: for_each_child_of_node, etc. > > +// This uses a conjunction, which requires at least coccinelle >= 1.0.4 > + > virtual patch > virtual context > virtual org > -- > 2.8.2 > >
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web