2023-10-03 19:50:38

by Anton Eliasson

[permalink] [raw]
Subject: [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

Exiting on error breaks the chain mode. Return the error instead in
order for the caller to propagate it or in the case of chain, try the
next mode.

Signed-off-by: Anton Eliasson <[email protected]>
---
scripts/coccicheck | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e52cb43fede6..95a312730e98 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -153,7 +153,7 @@ run_cmd_parmap() {
err=$?
if [[ $err -ne 0 ]]; then
echo "coccicheck failed"
- exit $err
+ return $err
fi
}


--
2.30.2


2023-11-02 21:28:11

by Julia Lawall

[permalink] [raw]
Subject: Re: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap



----- Mail original -----
> De: "Anton Eliasson" <[email protected]>
> ?: "Julia Lawall" <[email protected]>, "nicolas palix" <[email protected]>
> Cc: "cocci" <[email protected]>, "linux-kernel" <[email protected]>, "Anton Eliasson" <[email protected]>,
> [email protected]
> Envoy?: Mardi 3 Octobre 2023 16:25:14
> Objet: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

> Exiting on error breaks the chain mode. Return the error instead in
> order for the caller to propagate it or in the case of chain, try the
> next mode.
>
> Signed-off-by: Anton Eliasson <[email protected]>
> ---
> scripts/coccicheck | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index e52cb43fede6..95a312730e98 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -153,7 +153,7 @@ run_cmd_parmap() {
> err=$?
> if [[ $err -ne 0 ]]; then
> echo "coccicheck failed"
> - exit $err
> + return $err
> fi
> }
>

I tried disabling OCaml in my version of Coccinelle and then ran make coccicheck with this patch. But I didn't see any improvement. On the other hand, it keeps going if I just remove the exit line entirely. Is that what is wanted? One can still see the coccicheck failed message.

julia

> --
> 2.30.2

2023-11-17 16:37:45

by Anton Eliasson

[permalink] [raw]
Subject: Re: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

On 02/11/2023 22.27, Julia Lawall wrote:
>
> ----- Mail original -----
>> De: "Anton Eliasson" <[email protected]>
>> À: "Julia Lawall" <[email protected]>, "nicolas palix" <[email protected]>
>> Cc: "cocci" <[email protected]>, "linux-kernel" <[email protected]>, "Anton Eliasson" <[email protected]>,
>> [email protected]
>> Envoyé: Mardi 3 Octobre 2023 16:25:14
>> Objet: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap
>> Exiting on error breaks the chain mode. Return the error instead in
>> order for the caller to propagate it or in the case of chain, try the
>> next mode.
>>
>> Signed-off-by: Anton Eliasson <[email protected]>
>> ---
>> scripts/coccicheck | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/coccicheck b/scripts/coccicheck
>> index e52cb43fede6..95a312730e98 100755
>> --- a/scripts/coccicheck
>> +++ b/scripts/coccicheck
>> @@ -153,7 +153,7 @@ run_cmd_parmap() {
>> err=$?
>> if [[ $err -ne 0 ]]; then
>> echo "coccicheck failed"
>> - exit $err
>> + return $err
>> fi
>> }
>>
> I tried disabling OCaml in my version of Coccinelle and then ran make coccicheck with this patch. But I didn't see any improvement. On the other hand, it keeps going if I just remove the exit line entirely. Is that what is wanted? One can still see the coccicheck failed message.
>
> julia
>
Hi again!

It only makes a difference in chain mode, I think. Without my change
coccicheck stops after encountering the first semantic patch that
doesn't support the patch rule (see the console output below). With the
change coccicheck keeps going with the next semantic patch which is what
I expect. Do you see a different behavior?

However, I found now that with the change it gets harder to cancel
coccicheck. A single ctrl-c is no longer enough. I basically have to
hold down ctrl-c until all the script layers have returned and exited.
So that's maybe not ideal.

> $ git describe
> v6.6
> $ make coccicheck MODE=chain
> You have selected the "chain" mode.
> All available modes will be tried (in that order): patch, report,
> context, org
>
> Please check for false positives in the output before submitting a patch.
> When using "patch" mode, carefully review the patch before submitting it.
>
> /usr/bin/spatch -D patch --very-quiet --cocci-file
> ./scripts/coccinelle/api/alloc/alloc_cast.cocci --no-includes
> --include-headers --dir . -I ./arch/x86/include -I
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I
> ./include/generated/uapi --include ./include/linux/compiler-version.h
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> 14406 files match
> /usr/bin/spatch -D patch --very-quiet --cocci-file
> ./scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci --no-includes
> --include-headers --dir . -I ./arch/x86/include -I
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I
> ./include/generated/uapi --include ./include/linux/compiler-version.h
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> 44 files match
> /usr/bin/spatch -D patch --very-quiet --cocci-file
> ./scripts/coccinelle/api/alloc/zalloc-simple.cocci --no-includes
> --include-headers --dir . -I ./arch/x86/include -I
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I
> ./include/generated/uapi --include ./include/linux/compiler-version.h
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> 2146 files match
> diff -u -p a/fs/direct-io.c b/fs/direct-io.c
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -1128,15 +1128,9 @@ ssize_t __blockdev_direct_IO(struct kioc
>      if (iov_iter_rw(iter) == READ && !count)
>          return 0;
>
> -    dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
> +    dio = kmem_cache_zalloc(dio_cache, GFP_KERNEL);
>      if (!dio)
>          return -ENOMEM;
> -    /*
> -     * Believe it or not, zeroing out the page array caused a .5%
> -     * performance regression in a database benchmark.  So, we take
> -     * care to only zero out what's needed.
> -     */
> -    memset(dio, 0, offsetof(struct dio, pages));
>
>      dio->flags = flags;
>      if (dio->flags & DIO_LOCKING && iov_iter_rw(iter) == READ) {
> /usr/bin/spatch -D patch --very-quiet --cocci-file
> ./scripts/coccinelle/api/atomic_as_refcounter.cocci --include-headers
> --very-quiet --dir . -I ./arch/x86/include -I
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I
> ./include/generated/uapi --include ./include/linux/compiler-version.h
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> virtual rule patch not supported
> coccicheck failed
> make[1]: *** [/home/antone/git/linux/Makefile:2005: coccicheck] Error 255
> make: *** [Makefile:234: __sub-make] Error 2