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

[Anton Eliasson]

On 02/11/2023 22.27, Julia Lawall wrote:
> ----- Mail original -----
> > De: "Anton Eliasson" <anton.eliasson@xxxxxxxx>
> > À: "Julia Lawall" <Julia.Lawall@xxxxxxxx>, "nicolas palix" <nicolas.palix@xxxxxxx>
> > Cc: "cocci" <cocci@xxxxxxxx>, "linux-kernel" <linux-kernel@xxxxxxxxxxxxxxx>, "Anton Eliasson" <anton.eliasson@xxxxxxxx>,
> > kernel@xxxxxxxx
> > 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 <anton.eliasson@xxxxxxxx>
> > ---
> > 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