[PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Julia Lawall]

An initial rule just matching a function call makes it possible to focus
on one all at a time, which improves performance in complex functions with
multiple locking calls.  Using a constraint expression lock2 != pre.lock1;
makes it possible to eliminate the double matching of the lock, lock
sequence.

Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>

---

diff --git a/scripts/coccinelle/locks/flags.cocci b/scripts/coccinelle/locks/flags.cocci
index debd70e..33f29a5 100644
--- a/scripts/coccinelle/locks/flags.cocci
+++ b/scripts/coccinelle/locks/flags.cocci
@@ -12,9 +12,9 @@ virtual context
 virtual org
 virtual report

-@r exists@
-expression lock1,lock2,flags;
-position p1,p2;
+@pre exists@
+expression lock1,flags;
+position p1;
 @@

 (
@@ -24,25 +24,12 @@ read_lock_irqsave@p1(lock1,flags)
 |
 write_lock_irqsave@p1(lock1,flags)
 )
-... when != flags
-(
-spin_lock_irqsave(lock1,flags)
-|
-read_lock_irqsave(lock1,flags)
-|
-write_lock_irqsave(lock1,flags)
-|
-spin_lock_irqsave@p2(lock2,flags)
-|
-read_lock_irqsave@p2(lock2,flags)
-|
-write_lock_irqsave@p2(lock2,flags)
-)

-@d exists@
-expression f <= r.flags;
-expression lock1,lock2,flags;
-position r.p1, r.p2;
+@r exists@
+expression lock2 != pre.lock1;
+expression f <= pre.flags;
+expression pre.lock1,pre.flags;
+position pre.p1,p2;
 @@

 (
@@ -63,16 +50,16 @@ position r.p1, r.p2;

 // ----------------------------------------------------------------------

-@script:python depends on d && org@
-p1 << r.p1;
+@script:python depends on org@
+p1 << pre.p1;
 p2 << r.p2;
 @@

 cocci.print_main("original lock",p1)
 cocci.print_secs("nested lock+irqsave that reuses flags",p2)

-@script:python depends on d && report@
-p1 << r.p1;
+@script:python depends on report@
+p1 << pre.p1;
 p2 << r.p2;
 @@

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Masahiro Yamada]

Hi Julia,


2017-11-13 1:08 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
> An initial rule just matching a function call makes it possible to focus
> on one all at a time, which improves performance in complex functions with
> multiple locking calls.  Using a constraint expression lock2 != pre.lock1;
> makes it possible to eliminate the double matching of the lock, lock
> sequence.
>
> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>


If I apply this patch, locks/flags.cocci does not work for me.


$ make coccicheck COCCI=scripts/coccinelle/locks/flags.cocci
MODE=report  DEBUG_FILE=log.txt

Please check for false positives in the output before submitting a patch.
When using "patch" mode, carefully review the patch before submitting it.

607 610
coccicheck failed
$ cat  log.txt
/usr/bin/spatch -D report --no-show-diff --very-quiet --cocci-file
scripts/coccinelle/locks/flags.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/kconfig.h --jobs 8
--chunksize 1
Fatal error: exception Failure("meta: parse error: \n = File
\"scripts/coccinelle/locks/flags.cocci\", line 29, column 20,  charpos
= 607\n    around = 'pre', whole content = expression lock2 !=
pre.lock1;\n")



Could you check it?


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Julia Lawall]

On Wed, 15 Nov 2017, Masahiro Yamada wrote:

> Hi Julia,
>
>
> 2017-11-13 1:08 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
> > An initial rule just matching a function call makes it possible to focus
> > on one all at a time, which improves performance in complex functions with
> > multiple locking calls.  Using a constraint expression lock2 != pre.lock1;
> > makes it possible to eliminate the double matching of the lock, lock
> > sequence.
> >
> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
>
>
> If I apply this patch, locks/flags.cocci does not work for me.
>
>
> $ make coccicheck COCCI=scripts/coccinelle/locks/flags.cocci
> MODE=report  DEBUG_FILE=log.txt
>
> Please check for false positives in the output before submitting a patch.
> When using "patch" mode, carefully review the patch before submitting it.
>
> 607 610
> coccicheck failed
> $ cat  log.txt
> /usr/bin/spatch -D report --no-show-diff --very-quiet --cocci-file
> scripts/coccinelle/locks/flags.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/kconfig.h --jobs 8
> --chunksize 1
> Fatal error: exception Failure("meta: parse error: \n = File
> \"scripts/coccinelle/locks/flags.cocci\", line 29, column 20,  charpos
> = 607\n    around = 'pre', whole content = expression lock2 !=
> pre.lock1;\n")
>
>
>
> Could you check it?

What version of Coccinelle do you have?

But the way, Thierry Martinez should get in touch with you shortly about
the badzero problem.

julia

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Masahiro Yamada]

Hi Julia,


2017-11-15 23:51 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
>
>
> On Wed, 15 Nov 2017, Masahiro Yamada wrote:
>
>> Hi Julia,
>>
>>
>> 2017-11-13 1:08 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
>> > An initial rule just matching a function call makes it possible to focus
>> > on one all at a time, which improves performance in complex functions with
>> > multiple locking calls.  Using a constraint expression lock2 != pre.lock1;
>> > makes it possible to eliminate the double matching of the lock, lock
>> > sequence.
>> >
>> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
>>
>>
>> If I apply this patch, locks/flags.cocci does not work for me.
>>
>>
>> $ make coccicheck COCCI=scripts/coccinelle/locks/flags.cocci
>> MODE=report  DEBUG_FILE=log.txt
>>
>> Please check for false positives in the output before submitting a patch.
>> When using "patch" mode, carefully review the patch before submitting it.
>>
>> 607 610
>> coccicheck failed
>> $ cat  log.txt
>> /usr/bin/spatch -D report --no-show-diff --very-quiet --cocci-file
>> scripts/coccinelle/locks/flags.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/kconfig.h --jobs 8
>> --chunksize 1
>> Fatal error: exception Failure("meta: parse error: \n = File
>> \"scripts/coccinelle/locks/flags.cocci\", line 29, column 20,  charpos
>> = 607\n    around = 'pre', whole content = expression lock2 !=
>> pre.lock1;\n")
>>
>>
>>
>> Could you check it?
>
> What version of Coccinelle do you have?


I tried 1.0.4 and 1.0.6,
but both failed.


(1.0.4 is distribution version (/usr/bin/spatch) on Ubuntu 16.04LTS



The output from 1.0.6 is a bit more readable,
but the content is the same.


meta: parse error:
  File "scripts/coccinelle/locks/flags.cocci", line 29, column 20, charpos = 607
  around = 'pre',
  whole content = expression lock2 != pre.lock1;



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Julia Lawall]

On Thu, 16 Nov 2017, Masahiro Yamada wrote:

> Hi Julia,
>
>
> 2017-11-15 23:51 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
> >
> >
> > On Wed, 15 Nov 2017, Masahiro Yamada wrote:
> >
> >> Hi Julia,
> >>
> >>
> >> 2017-11-13 1:08 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
> >> > An initial rule just matching a function call makes it possible to focus
> >> > on one all at a time, which improves performance in complex functions with
> >> > multiple locking calls.  Using a constraint expression lock2 != pre.lock1;
> >> > makes it possible to eliminate the double matching of the lock, lock
> >> > sequence.
> >> >
> >> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> >>
> >>
> >> If I apply this patch, locks/flags.cocci does not work for me.
> >>
> >>
> >> $ make coccicheck COCCI=scripts/coccinelle/locks/flags.cocci
> >> MODE=report  DEBUG_FILE=log.txt
> >>
> >> Please check for false positives in the output before submitting a patch.
> >> When using "patch" mode, carefully review the patch before submitting it.
> >>
> >> 607 610
> >> coccicheck failed
> >> $ cat  log.txt
> >> /usr/bin/spatch -D report --no-show-diff --very-quiet --cocci-file
> >> scripts/coccinelle/locks/flags.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/kconfig.h --jobs 8
> >> --chunksize 1
> >> Fatal error: exception Failure("meta: parse error: \n = File
> >> \"scripts/coccinelle/locks/flags.cocci\", line 29, column 20,  charpos
> >> = 607\n    around = 'pre', whole content = expression lock2 !=
> >> pre.lock1;\n")
> >>
> >>
> >>
> >> Could you check it?
> >
> > What version of Coccinelle do you have?
>
>
> I tried 1.0.4 and 1.0.6,
> but both failed.
>
>
> (1.0.4 is distribution version (/usr/bin/spatch) on Ubuntu 16.04LTS
>
>
>
> The output from 1.0.6 is a bit more readable,
> but the content is the same.
>
>
> meta: parse error:
>   File "scripts/coccinelle/locks/flags.cocci", line 29, column 20, charpos = 607
>   around = 'pre',
>   whole content = expression lock2 != pre.lock1;

OK, I will check on it later today. It may require the very soon to be
released 1.0.7.

julia

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Julia Lawall]

On Thu, 16 Nov 2017, Masahiro Yamada wrote:

> Hi Julia,
>
>
> 2017-11-15 23:51 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
> >
> >
> > On Wed, 15 Nov 2017, Masahiro Yamada wrote:
> >
> >> Hi Julia,
> >>
> >>
> >> 2017-11-13 1:08 GMT+09:00 Julia Lawall <julia.lawall@lip6.fr>:
> >> > An initial rule just matching a function call makes it possible to focus
> >> > on one all at a time, which improves performance in complex functions with
> >> > multiple locking calls.  Using a constraint expression lock2 != pre.lock1;
> >> > makes it possible to eliminate the double matching of the lock, lock
> >> > sequence.
> >> >
> >> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> >>
> >>
> >> If I apply this patch, locks/flags.cocci does not work for me.
> >>
> >>
> >> $ make coccicheck COCCI=scripts/coccinelle/locks/flags.cocci
> >> MODE=report  DEBUG_FILE=log.txt
> >>
> >> Please check for false positives in the output before submitting a patch.
> >> When using "patch" mode, carefully review the patch before submitting it.
> >>
> >> 607 610
> >> coccicheck failed
> >> $ cat  log.txt
> >> /usr/bin/spatch -D report --no-show-diff --very-quiet --cocci-file
> >> scripts/coccinelle/locks/flags.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/kconfig.h --jobs 8
> >> --chunksize 1
> >> Fatal error: exception Failure("meta: parse error: \n = File
> >> \"scripts/coccinelle/locks/flags.cocci\", line 29, column 20,  charpos
> >> = 607\n    around = 'pre', whole content = expression lock2 !=
> >> pre.lock1;\n")
> >>
> >>
> >>
> >> Could you check it?
> >
> > What version of Coccinelle do you have?
>
>
> I tried 1.0.4 and 1.0.6,
> but both failed.

For me, it also fails for 1.0.6.  It should be OK for 1.0.7.

julia

>
>
> (1.0.4 is distribution version (/usr/bin/spatch) on Ubuntu 16.04LTS
>
>
>
> The output from 1.0.6 is a bit more readable,
> but the content is the same.
>
>
> meta: parse error:
>   File "scripts/coccinelle/locks/flags.cocci", line 29, column 20, charpos = 607
>   around = 'pre',
>   whole content = expression lock2 != pre.lock1;
>
>
>
> --
> Best Regards
> Masahiro Yamada
>

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Joe Perches]

On Wed, 2017-11-15 at 22:55 +0100, Julia Lawall wrote:
> What version of Coccinelle do you have?
> > I tried 1.0.4 and 1.0.6,
> > but both failed.
> For me, it also fails for 1.0.6.  It should be OK for 1.0.7.

Then likely this patch should not be applied or
some form of local version checking should be done
in the script itself.

Scripts that use various tools are subject to
language versioning requirements.

For instance, there are many things that are
possible with later versions of perl, but are not
done for various scripts because many distributions
do not ship with the latest versions.

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Masahiro Yamada]

2017-11-16 7:02 GMT+09:00 Joe Perches <joe@perches.com>:
> On Wed, 2017-11-15 at 22:55 +0100, Julia Lawall wrote:
>> What version of Coccinelle do you have?
>> > I tried 1.0.4 and 1.0.6,
>> > but both failed.
>> For me, it also fails for 1.0.6.  It should be OK for 1.0.7.
>
> Then likely this patch should not be applied or
> some form of local version checking should be done
> in the script itself.
>
> Scripts that use various tools are subject to
> language versioning requirements.
>
> For instance, there are many things that are
> possible with later versions of perl, but are not
> done for various scripts because many distributions
> do not ship with the latest versions.
>


OK.

// Requires: 1.0.7

is necessary
and applicable after coccinelle 1.0.7 is released.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH] coccinelle: flags.cocci: reorganize patterns to improve efficiency

[Julia Lawall]

On Wed, 15 Nov 2017, Joe Perches wrote:

> On Wed, 2017-11-15 at 22:55 +0100, Julia Lawall wrote:
> > What version of Coccinelle do you have?
> > > I tried 1.0.4 and 1.0.6,
> > > but both failed.
> > For me, it also fails for 1.0.6.  It should be OK for 1.0.7.
>
> Then likely this patch should not be applied or
> some form of local version checking should be done
> in the script itself.
>
> Scripts that use various tools are subject to
> language versioning requirements.
>
> For instance, there are many things that are
> possible with later versions of perl, but are not
> done for various scripts because many distributions
> do not ship with the latest versions.

It will say coccicheck failed if it is not compatible with the version of
Coccinelle.  But it shouldn't break anything.  We should put that it
requires 1.0.7, but I don't know if that is actually checked, or just
provided as documentation for the reader.

julia