Re: [Cocci] [PATCH] lib/test_kmod: Fix an integer overflow test
On Sat, Feb 24, 2018 at 11:45:16AM +0300, Dan Carpenter wrote: > On Sat, Feb 24, 2018 at 02:59:41AM +0000, Luis R. Rodriguez wrote: > > On Mon, Jan 22, 2018 at 01:27:54PM +0300, Dan Carpenter wrote: > > > The main problem is that the parentheses are in the wrong place and the > > > unlikely() call returns either 0 or 1 so it's never less than zero. > > > > Doh, thanks, yes. Seems worth considering a grammar rule for it. > > > > > The other problem is that signed integer overflows like "INT_MAX + 1" are > > > undefined behavior. > > > > Likewise. > > > > This seems like another possible generic typo issue. But I would not > > resolve it > > the way you did, in this particular case below num_test_devs represents the > > number of already registered devs, before we increment. So the way to > > resolve > > this would be: > > > > if (num_test_devs + 1 == INT_MAX) > > > > I'll get this upstream, thanks! > > There is no issue if num_test_devs is INT_MAX. But capping it at > INT_MAX - 1 is also fine. If num_test_devs is INT_MAX, then doing num_test_devs + 1 overflows and as you noted that is undefined? Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] lib/test_kmod: Fix an integer overflow test
On Mon, Jan 22, 2018 at 01:27:54PM +0300, Dan Carpenter wrote: > The main problem is that the parentheses are in the wrong place and the > unlikely() call returns either 0 or 1 so it's never less than zero. Doh, thanks, yes. Seems worth considering a grammar rule for it. > The other problem is that signed integer overflows like "INT_MAX + 1" are > undefined behavior. Likewise. This seems like another possible generic typo issue. But I would not resolve it the way you did, in this particular case below num_test_devs represents the number of already registered devs, before we increment. So the way to resolve this would be: if (num_test_devs + 1 == INT_MAX) I'll get this upstream, thanks! Luis > Fixes: d9c6a72d6fa2 ("kmod: add test driver to stress test the module loader") > Signed-off-by: Dan Carpenter > > diff --git a/lib/test_kmod.c b/lib/test_kmod.c > index e372b97eee13..30fd6d9e5361 100644 > --- a/lib/test_kmod.c > +++ b/lib/test_kmod.c > @@ -1141,7 +1141,7 @@ static struct kmod_test_device > *register_test_dev_kmod(void) > mutex_lock(®_dev_mutex); > > /* int should suffice for number of devices, test for wrap */ > - if (unlikely(num_test_devs + 1) < 0) { > + if (num_test_devs == INT_MAX) { > pr_err("reached limit of number of test devices\n"); > goto out; > } > -- Luis Rodriguez, SUSE LINUX GmbH Maxfeldstrasse 5; D-90409 Nuernberg ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Having rules span multiple files
On Wed, Nov 29, 2017 at 10:28:43PM +0100, Julia Lawall wrote: > Anohter option is to create a file with lists of file names that should be > considered together. So you could have: > > a/foo.c > a/xyz.c > > b/bar.c > b/mno.c > b/zzz.c > > Then use --file-groups with the name of this file. Ah, now we just need something which given a directory or config option infers the number of associated files to that config option. :) Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Having rules span multiple files
On Wed, Nov 29, 2017 at 10:21:00PM +0100, Michael Stefaniuc wrote: > On 11/29/2017 09:58 PM, Luis R. Rodriguez wrote: > > I've recently implemented support for a functionality which would allow us > > to remove freezing calls on kthreads on filesystems *iff* the filesystem > > implements support for filesystem freezing, this can be determined if > > the filesystem implements the fs_freeze superblock callback. So the below > > rule looks for the callback first, then looks to see if it has a kthread > > implemented, and finally on the kthread fn, it removes any freeze calls. > > This works quite nice on a filesystem where there is a match for all > > the above in one file, for example fs/ext4/super.c, however it doesn't > > work when say the fs_freeze is one file, and the kthread on another. > > > > How could I get the rules to span multiple files? > You mean treat multiple .c files as a single compilation unit? > Just pass all those files together on the command line: > spatch foo.c bar.c > or > spatch fs/ext4/*.c Neat that works, thanks! Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Having rules span multiple files
I've recently implemented support for a functionality which would allow us to remove freezing calls on kthreads on filesystems *iff* the filesystem implements support for filesystem freezing, this can be determined if the filesystem implements the fs_freeze superblock callback. So the below rule looks for the callback first, then looks to see if it has a kthread implemented, and finally on the kthread fn, it removes any freeze calls. This works quite nice on a filesystem where there is a match for all the above in one file, for example fs/ext4/super.c, however it doesn't work when say the fs_freeze is one file, and the kthread on another. How could I get the rules to span multiple files? @ has_freeze_fs @ identifier super_ops; expression freeze_op; @@ struct super_operations super_ops = { .freeze_fs = freeze_op, }; @ has_kthread depends on has_freeze_fs @ identifier kthread_fn; expression task; @@ task = kthread_run(kthread_fn, ...); @ remove_set_freezable depends on has_kthread @ identifier has_kthread.kthread_fn; expression time; @@ kthread_fn(...) { <+... ( - set_freezable(); | - try_to_freeze(); | - freezable_schedule(); + schedule(); | - freezable_schedule_timeout(time); + schedule_timeout(time); ) ...+> } ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3] coccinelle: fix parallel build with CHECK=scripts/coccicheck
On Tue, Nov 14, 2017 at 06:04:49PM +0900, Masahiro Yamada wrote: > diff --git a/scripts/coccicheck b/scripts/coccicheck > index 040a8b1..7da82a1 100755 > --- a/scripts/coccicheck > +++ b/scripts/coccicheck > @@ -70,6 +64,13 @@ if [ "$C" = "1" -o "$C" = "2" ]; then > # Take only the last argument, which is the C file to test > shift $(( $# - 1 )) > OPTIONS="$COCCIINCLUDE $1" > + > +# If -j option is given to Make, scripts/coccicheck runs in parallel. > +# If coccinelle also runs in parallel, it fails because multiple > processes > +# try to get access to the same subdirectory that stores stdout/stderr. > +# No need to parallelize coccinelle in this case - this mode takes only > +# one file input. > +NPROC=1 Shouldn't this also warn to the user, and recommend to use a proper form to parallelize coccinelle? Otherwise a user might get the impression they are parallelizing coccinelle where they really did not. Luis > else > ONLINE=0 > if [ "$KBUILD_EXTMOD" = "" ] ; then ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Hunt for array users of roc_dointvec()
On Thu, Feb 02, 2017 at 12:06:28AM +0100, Julia Lawall wrote: > > > Do you require that there is exactly one element in the arrays? > > > > Well my eventual hunt is for all array users. So I'm starting off by > > first looking for all simple setups with non-array cases, and then > > giving context for those that are not singular. So eventually I will > > write a rule that will show the entries which do not have > > > > ( > > maxelen = sizeof(int), > > | > > maxlen = sizeof(unsigned int)., > > ) > > > > I figure this list should be small enough to inspect with one's eyeballs > > then. > > These arrays seem to need something to mark the end. So you need to put > {}, in each of your rules. Here is an example match: > > arch/x86/entry/vdso/vdso32-setup.c > > static struct ctl_table abi_table2[] = { > { > .procname = "vsyscall32", > .data = &vdso32_enabled, > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec > }, > {} > }; That fixed it, thanks! Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Hunt for array users of roc_dointvec()
On Wed, Feb 1, 2017 at 4:34 PM, Julia Lawall wrote: > On Wed, 1 Feb 2017, Luis R. Rodriguez wrote: >> Ah you are right, but it then goes ahead and uses it directly: >> >> ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos); >> >> So I would need to capture this use in another rule. I can do >> that later once I iron out the first part, which is to first >> hunt for non maxlen = sizeof(unsigned int). > > Do you require that there is exactly one element in the arrays? Well my eventual hunt is for all array users. So I'm starting off by first looking for all simple setups with non-array cases, and then giving context for those that are not singular. So eventually I will write a rule that will show the entries which do not have ( maxelen = sizeof(int), | maxlen = sizeof(unsigned int)., ) I figure this list should be small enough to inspect with one's eyeballs then. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Hunt for array users of roc_dointvec()
On Wed, Feb 01, 2017 at 11:14:56PM +0100, Julia Lawall wrote: > > > On Wed, 1 Feb 2017, Luis R. Rodriguez wrote: > > > On Wed, Feb 01, 2017 at 10:59:29PM +0100, Julia Lawall wrote: > > > > > > > > > On Wed, 1 Feb 2017, Luis R. Rodriguez wrote: > > > > > > > I'm trying to hunt for users of the proc_dointvec() handler in > > > > setups where more than one value is expected to be used. The way > > > > this old API works is it lets you specifiy in a structure the > > > > variables and the size of the variables. In the singular case the > > > > size will be maxlen = sizeof(int). If an array is used a user > > > > may use maxlen = 2*sizeof(int), or other things. > > > > > > > > Since I am looking for unsigned int users of proc_dointvec() to later > > > > consider converting them to proc_douintvec() I can just focus for now > > > > when the is not maxlen = (unsigned int) or maxlen = sizeof(int), and > > > > then > > > > manually review the cases. Unfrotunately I cannot seem to construct > > > > grammar to query context for this. > > > > > > > > Let's consider just trying to get context for the case where the size is > > > > not maxlen = sizeof(unsigned int). I tried this bug get no results: > > > > > > Do you have an example of a C file on which you expect to get a match? > > > > Sure, > > > > ipv4_local_port_range() on the file net/ipv4/sysctl_net_ipv4.c has an > > example in C code on a functoin where it uses: > > > > .maxlen = sizeof(range), > > This example doesn't have fields procname and proc_handler. Ah you are right, but it then goes ahead and uses it directly: ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos); So I would need to capture this use in another rule. I can do that later once I iron out the first part, which is to first hunt for non maxlen = sizeof(unsigned int). > I'm trying to figure out what is the goal of your first rule: > > @ find_proc @ > identifier ctls; > expression name; > @@ > > struct ctl_table ctls[] = { > { > .procname = name, > .proc_handler = proc_dointvec, > }, > }; I am looking for all entries which use proc_handler = proc_dointvec. The subsequent rule then rlies on the name to again check for if the maxlen with unsigned int is present. The third rule is to look for the same but where no maxelen with the unsigned int is used. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Hunt for array users of roc_dointvec()
On Wed, Feb 01, 2017 at 10:59:29PM +0100, Julia Lawall wrote: > > > On Wed, 1 Feb 2017, Luis R. Rodriguez wrote: > > > I'm trying to hunt for users of the proc_dointvec() handler in > > setups where more than one value is expected to be used. The way > > this old API works is it lets you specifiy in a structure the > > variables and the size of the variables. In the singular case the > > size will be maxlen = sizeof(int). If an array is used a user > > may use maxlen = 2*sizeof(int), or other things. > > > > Since I am looking for unsigned int users of proc_dointvec() to later > > consider converting them to proc_douintvec() I can just focus for now > > when the is not maxlen = (unsigned int) or maxlen = sizeof(int), and then > > manually review the cases. Unfrotunately I cannot seem to construct > > grammar to query context for this. > > > > Let's consider just trying to get context for the case where the size is > > not maxlen = sizeof(unsigned int). I tried this bug get no results: > > Do you have an example of a C file on which you expect to get a match? Sure, ipv4_local_port_range() on the file net/ipv4/sysctl_net_ipv4.c has an example in C code on a functoin where it uses: .maxlen = sizeof(range), However the typical use seems to be static definitions as part of the file, as in kernel/sysctl.c on the line after: static struct ctl_table kern_table[] = { An example would be on the traceoff_on_warning. I know that's not an array but its an example to then manually inspect. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Hunt for array users of roc_dointvec()
I'm trying to hunt for users of the proc_dointvec() handler in setups where more than one value is expected to be used. The way this old API works is it lets you specifiy in a structure the variables and the size of the variables. In the singular case the size will be maxlen = sizeof(int). If an array is used a user may use maxlen = 2*sizeof(int), or other things. Since I am looking for unsigned int users of proc_dointvec() to later consider converting them to proc_douintvec() I can just focus for now when the is not maxlen = (unsigned int) or maxlen = sizeof(int), and then manually review the cases. Unfrotunately I cannot seem to construct grammar to query context for this. Let's consider just trying to get context for the case where the size is not maxlen = sizeof(unsigned int). I tried this bug get no results: virtual context @ find_proc @ identifier ctls; expression name; @@ struct ctl_table ctls[] = { { .procname = name, .proc_handler = proc_dointvec, }, }; @ singular_ints @ identifier find_proc.ctls; expression find_proc.name; @@ struct ctl_table ctls[] = { { .procname = name, .maxlen = sizeof(unsigned int), }, }; @ array_ints depends on context && !singular_ints @ identifier find_proc.ctls; expression find_proc.name, E; @@ struct ctl_table ctls[] = { { * .procname = name, * .maxlen = E, }, }; ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
On Thu, Aug 25, 2016 at 10:10:52PM +0200, Daniel Vetter wrote: > Cutting down since a lot of this is probably better discussed at > ks/lpc. Aside, if you want to check out Chris Wilson's work on our new > depency handling, it's called kfence. > > https://lkml.org/lkml/2016/7/17/37 Thanks more reading.. :) > On Thu, Aug 25, 2016 at 9:41 PM, Luis R. Rodriguez wrote: > >> > So .. I agree, let's avoid the hacks. Patches welcomed. > >> > >> Hm, this is a definite change of tack - back when I discussed this > >> with Greg about 2 ks ago it sounded like "don't do this". The only > >> thing we need is some way to wait for rootfs before we do the > >> request_firmware. Everything else we handle already in the kernel. > > > > OK so lets just get this userspace event done, and we're set. > > Ah great. As long as that special wait-for-rootfs-pls firmware request > is still allowed, i915&gfx folks will be happy. We will call it from > probe though ;-) but all from our own workers. We should strive for this to be transparent to drivers, ie, this safeguard of looking for firmware should be something handled by the kernel as otherwise we're just forcing a race condition given the review in the last thread. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
Summoning Felix for the embedded aspect on initramfs below. Jörg might be interested in the async facilities you speak of as well. On Thu, Aug 25, 2016 at 01:05:44PM +0200, Daniel Vetter wrote: > On Wed, Aug 24, 2016 at 10:39 PM, Luis R. Rodriguez wrote: > > On Wed, Aug 24, 2016 at 08:55:55AM +0200, Daniel Vetter wrote: > >> On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez > >> wrote: > >> > Thou shalt not make firmware calls early on init or probe. > > > > <-- snip --> > > > >> > There are 4 offenders at this time: > >> > > >> > mcgrof@ergon ~/linux-next (git::20160609)$ export > >> > COCCI=scripts/coccinelle/api/request_firmware.cocci > >> > mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report > >> > > >> > drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on > >> > its init routine on line 321. > >> > drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call > >> > on its probe routine on line 136. > >> > drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on > >> > its probe routine on line 796. > >> > drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call > >> > on its probe routine on line 1246. > >> > >> Plus all gpu drivers which need firmware. And yes we must load them at > >> probe > > > > Do you have an upstream driver in mind that does this ? Is it on device > > drier module probe or a DRM subsystem specific probe call of some sort ? > > i915 is the one I care about for obvious reasons ;-) It's all from the > pci device probe function, but nested really deeply. The above SmPL grammar should capture well nested calls of calls within probe, so curious why it didn't pick up i915. Let's see. i915_pci_probe() --> i915_driver_load() --> i915_load_modeset_init() --> (drivers/gpu/drm/i915/i915_drv.c) a) intel_csr_ucode_init() (drivers/gpu/drm/i915/intel_csr.c) ... b) intel_guc_init() (drivers/gpu/drm/i915/intel_guc_loader.c) The two firmwares come from: a) intel_csr_ucode_init() --> schedule_work(&dev_priv->csr.work); csr_load_work_fn() --> request_firmware() b) intel_guc_init() --> guc_fw_fetch() request_firmware() --- a) is not dealt with given the grammar does not include scheduled work, however using work to offload loading firmware seems reasonable here. b) This should have been picked up, but its not. Upon closer inspection the grammar currently expects the module init and probe to be on the same file. Loosening this: diff --git a/scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci b/scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci index cf180c59e042..e19e6d3dfc0f 100644 --- a/scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci +++ b/scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci @@ -49,7 +49,7 @@ identifier init; module_init(init); -@ has_probe depends on defines_module_init@ +@ has_probe @ identifier drv_calls, drv_probe; type bus_driver; identifier probe_op =~ "(probe)"; @@ -59,7 +59,7 @@ bus_driver drv_calls = { .probe_op = drv_probe, }; -@hascall depends on !after_start && defines_module_init@ +@hascall depends on !after_start @ position p; @@ I get a lot more complaints but still -- i915 b) case is not yet covered: ./drivers/bluetooth/ath3k.c: ERROR: driver call request firmware call on its probe routine on line 546. ./drivers/bluetooth/bcm203x.c: ERROR: driver call request firmware call on its probe routine on line 193. ./drivers/bluetooth/bcm203x.c: ERROR: driver call request firmware call on its probe routine on line 218. ./drivers/bluetooth/bfusb.c: ERROR: driver call request firmware call on its probe routine on line 655. ./drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its init routine on line 321. ./drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on its probe routine on line 136. ./drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on its probe routine on line 1246. ./sound/soc/codecs/wm2000.c: ERROR: driver call request firmware call on its probe routine on line 893. ./sound/soc/sh/siu_dai.c: ERROR: driver call request firmware call on its probe routine on line 747. ./drivers/net/wireless/intersil/orinoco/orinoco_usb.c: ERROR: driver call request firmware call on its probe routine on line 1661. ./sound/soc/intel/common/sst-acpi.c: ERROR: driver call request firmware call on its probe routine on line 161. ./drivers/input/touchscreen/goodix.c: ERROR: driver call request firmware call on its prob
Re: [Cocci] [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
On Wed, Aug 24, 2016 at 08:55:55AM +0200, Daniel Vetter wrote: > On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez wrote: > > Thou shalt not make firmware calls early on init or probe. <-- snip --> > > There are 4 offenders at this time: > > > > mcgrof@ergon ~/linux-next (git::20160609)$ export > > COCCI=scripts/coccinelle/api/request_firmware.cocci > > mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report > > > > drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its > > init routine on line 321. > > drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on > > its probe routine on line 136. > > drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its > > probe routine on line 796. > > drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on > > its probe routine on line 1246. > > Plus all gpu drivers which need firmware. And yes we must load them at > probe Do you have an upstream driver in mind that does this ? Is it on device drier module probe or a DRM subsystem specific probe call of some sort ? > because people are generally pissed when they boot their machine > and the screen goes black. On top of that a lot of people want their > gpu drivers to be built-in, but can't ship the firmware blobs in the > kernel image because gpl. Yep, there's a bit a contradiction there ... Can they use initramfs for this ? Also just curious -- as with other subsystems, is it possible to load a generic driver first, say vesa, and then a more enhanced one later ? I am not saying this is ideal or am I suggesting this, I'd just like to know the feasibility of this. > I think what would work is loading the different subsystems of the > driver in parallel (we already do that largely) Init level stuff is actually pretty synchronous, and in fact both init and probe are called serially. There are a few subsystems which have been doing things a bit differently, but these are exceptions. When you say we already do this largely, can you describe a bit more precisely what you mean ? >, and then if one > firmware blob isn't there yet simply stall that async worker until it > shows up. Is this an existing framework or do you mean if we add something generic to do this async loading of subsystems ? > But the answers I've gotten thus far from request_firmware() > folks (well at least Greg) is don't do that ... Well in this patch set I'm adding myself as a MAINTAINER and I've been extending the firmware API recently to help with a few new features I want, I've been wanting to hear more feedback from other's needs and I had actually not gotten much -- except only recently with the usermode helper and reasons why some folks thought they could not use direct firmware loading from the fs. I'm keen to hear or more use cases and needs specially if they have to do with improving boot time and asynchronous boot. > Is that problem still somewhere on the radar? Not mine. > Atm there's various > wait_for_rootfs hacks out there floating in vendor/product trees. This one I've heard about recently, and I suggested two possible solutions, with a preference to a simply notification of when rootfs is available from userspace. > "Avoid at all costs" sounds like upstream prefers to not care about > android/cros in those case (yes I know most arm socs don't need > firmware, which would make it a problem fo just be a subset of all > devices). In my days of dealing with Android I learned most folks did not frankly care too much about upstream-first model. That means things were reactive. That's a business mind set and that's fine. However for upstream we want what is best and to discuss. I haven't seen proposals so, so long as we just hear of "hacks" that some folks do in vendor/product trees, what can we do ? In so far as async probe is concerned -- that is now upstream. http://www.do-not-panic.com/2015/12/linux-asynchronous-probe.html In so far as modules are concerned -- this should work without issue now, and if there is an issue its very likely a bug in the subsystem. As I noted in the post, built-in support requires more love. A simple option for you to test this is to test the two debug patches at the end there and boot. Alternatively inits can just peg the async request for all modules. Should be an easy change, just hadn't had a change to do it yet. Maybe its time. I'm also trying to make more async functionality possible early in boot with dependencies annotated somehow, and have a bit of work to help with this (refer to recent linker tables patches) already which may even be possible to used to facelift our old kernel init le
Re: [Cocci] [PATCH v4 0/9] coccicheck: modernize
On Wed, Jun 29, 2016 at 03:14:50PM -0700, Luis R. Rodriguez wrote: > There were quite a bit of comments from the v3 series [0], since there > was quite a bit of review needed for some other changes I've had discussions > with Nicolas and Julia privately over these changes and collected their Acks > privately to avoid unnecessary e-mail waste. *Poke* Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 1/9] coccicheck: move spatch binary check up
This has no functional changes. This is being done to enable us to later use spatch binary for some flag checking for certain features early on. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- scripts/coccicheck | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index f6627863fdc3..f137b04dfdd3 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -7,6 +7,11 @@ SPATCH="`which ${SPATCH:=spatch}`" +if [ ! -x "$SPATCH" ]; then +echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' +exit 1 +fi + trap kill_running SIGTERM SIGINT declare -a SPATCH_PID @@ -51,11 +56,6 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then OPTIONS="--patch $srctree $OPTIONS" fi -if [ ! -x "$SPATCH" ]; then -echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' -exit 1 -fi - if [ "$MODE" = "" ] ; then if [ "$ONLINE" = "0" ] ; then echo 'You have not explicitly specified the mode to use. Using default "report" mode.' -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 9/9] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
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 Acked-by: Julia Lawall Acked-by: Nicolas Palix --- 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.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 3/9] coccicheck: enable parmap support
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 and use it only if your number of processors detected is > 1. If parmap is enabled also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it. stderr is currently sent to /dev/null, addressing a way to capture that will be addressed next. If --jobs is not supported we fallback to the old mechanism. We expect to deprecate the old mechanism as soon as we can get confirmation all users are ready. 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. Extend Documentation/coccinelle.txt as well. 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 ... real29m14.912s user103m1.796s sys 0m4.464s After: real16m22.435s user128m30.060s sys 0m2.712s v4: o expand Documentation/coccinelle.txt to reflect parmap support info o update commit log to reflect what we actually do now with stderr o split out DEBUG_FILE use into another patch o detect number of CPUs and if its 1 then skip parmap support, note that if you still support parmap, but have 1 CPU you will also go through the new branches, so the old complex multithreaded process is skipped as well. 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 Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 15 +++ scripts/coccicheck | 35 --- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index bb9632c20cfb..64daaf18874b 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -94,11 +94,26 @@ To enable verbose messages set the V= variable, for example: make coccicheck MODE=report V=1 + Coccinelle parallelization + + By default, coccicheck tries to run as parallel as possible. To change the parallelism, set the J= variable. For example, to run across 4 CPUs: make coccicheck MODE=report J=4 +As of Coccinelle 1.0.2 Coccinelle uses Ocaml parmap for parallelization, +if support for this is detected you will benefit from parmap parallelization. + +When parmap is enabled coccicheck will enable dynamic load balancing by using +'--chunksize 1' argument, this ensures we keep feeding threads with work +one by one, so that we avoid the situation where most work gets done by only +a few threads. With dynamic load balancing, if a thread finishes early we keep +feeding it more work. + +When parmap is enabled, if an error occurs in Coccinelle, this error +value is propagated back, the return value of the 'make coccicheck' +captures this return value. Using Coccinelle with a single semantic patch ~~~ diff --git a/scripts/coccicheck b/scripts/coccicheck index 5319fae910b4..4b65a0fd50a1 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,16 @@ 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 +elif [ "$NPROC" != "1" ]; then + # Using 0 should work as well, refer to _SC_NPROCESSORS_ONLN use on + # https://github.com/rdicosmo/parmap/blob/master/setcore_stubs.c + 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 +92,18 @@ if [ "$ONLINE" = "0&qu
[Cocci] [PATCH v4 5/9] coccicheck: replace --very-quiet with --quiet when debugging
When debugging (using --profile or --show-trying) you want to avoid supressing output, use --quiet instead. While at it, extend documentation for SPFLAGS use. For instance one can use: $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ make coccicheck DEBUG_FILE="poo.err" MODE=report SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c Expand Documentation/coccinelle.txt as well. v4: expand Documentation/coccinelle.txt v3: rebased, resolve conflicts, expand Documentation/coccinelle.txt v2: use egrep instead of the *"=--option"* check, this doesn't work for disjunctions. Signed-off-by: Luis R. Rodriguez Acked-by: Julia Lawall --- Documentation/coccinelle.txt | 12 scripts/coccicheck | 21 + 2 files changed, 33 insertions(+) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 2516c5ef1691..66e822f8caee 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -175,6 +175,18 @@ instance: make coccicheck COCCI=scripts/coccinelle/free/kfree.cocci MODE=report DEBUG_FILE=cocci.err cat cocci.err +You can use SPFLAGS to add debugging flags, for instance you may want to +add both --profile --show-trying to SPFLAGS when debugging. For instance +you may want to use: + +rm -f err.log +export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci +make coccicheck DEBUG_FILE="err.log" MODE=report SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c + +err.log will now have the profiling information, while stdout will +provide some progress information as Coccinelle moves forward with +work. + DEBUG_FILE support is only supported when using coccinelle >= 1.2. Additional flags diff --git a/scripts/coccicheck b/scripts/coccicheck index 3f0bb3f0fddc..081ba5bff79c 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -32,6 +32,27 @@ fi FLAGS="--very-quiet" +# You can use SPFLAGS to append extra arguments to coccicheck or override any +# heuristics done in this file as Coccinelle accepts the last options when +# options conflict. +# +# A good example for use of SPFLAGS is if you want to debug your cocci script, +# you can for instance use the following: +# +# $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci +# $ make coccicheck MODE=report DEBUG_FILE="all.err" SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c +# +# "--show-trying" should show you what rule is being processed as it goes to +# stdout, you do not need a debug file for that. The profile output will be +# be sent to stdout, if you provide a DEBUG_FILE the profiling data can be +# inspected there. +# +# --profile will not output if --very-quiet is used, so avoid it. +echo $SPFLAGS | egrep -e "--profile|--show-trying" 2>&1 > /dev/null +if [ $? -eq 0 ]; then + FLAGS="--quiet" +fi + # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" COCCIINCLUDE=${LINUXINCLUDE//-I/-I } -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 8/9] coccicheck: refer to Documentation/coccinelle.txt and wiki
Refer to the Documentation/coccinelle.txt and supplemental documentation on the wiki: https://bottest.wiki.kernel.org/coccicheck This page shall always refer to the linux-next iteration of scripts/coccicheck. v4: only refer to the wiki as supplemental documentation, and also update Documentation/coccinelle.txt. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 9 + scripts/coccicheck | 5 +++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 1c26908ebc16..01fb1dae3163 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -38,6 +38,15 @@ as a regular user, and install it with sudo make install + Supplemental documentation + + +For supplemental documentation refer to the wiki: + +https://bottest.wiki.kernel.org/coccicheck + +The wiki documentation always refers to the linux-next version of the script. + Using Coccinelle on the Linux kernel ~~ diff --git a/scripts/coccicheck b/scripts/coccicheck index f9293ab04a8b..c92c1528a54d 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -1,9 +1,10 @@ #!/bin/bash - +# Linux kernel coccicheck +# +# Read Documentation/coccinelle.txt # # This script requires at least spatch # version 1.0.0-rc11. -# DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 9/9] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
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. Acked-by: Julia Lawall Acked-by: Nicolas Palix Signed-off-by: Luis R. Rodriguez --- 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.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 5/9] coccicheck: replace --very-quiet with --quit when debugging
When debugging (using --profile or --show-trying) you want to avoid supressing output, use --quiet instead. While at it, extend documentation for SPFLAGS use. For instance one can use: $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ make coccicheck DEBUG_FILE="poo.err" MODE=report SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c Expand Documentation/coccinelle.txt as well. v4: expand Documentation/coccinelle.txt v3: rebased, resolve conflicts, expand Documentation/coccinelle.txt v2: use egrep instead of the *"=--option"* check, this doesn't work for disjunctions. Signed-off-by: Luis R. Rodriguez --- Documentation/coccinelle.txt | 12 scripts/coccicheck | 23 +++ 2 files changed, 35 insertions(+) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 2516c5ef1691..4f5b2287a2b1 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -175,6 +175,18 @@ instance: make coccicheck COCCI=scripts/coccinelle/free/kfree.cocci MODE=report DEBUG_FILE=cocci.err cat cocci.err +You can use SPFLAGS to add debugging flags, for instance you may want to +add both --profile --show-trying to SPFLAGS when debugging. For instance +you may want to use: + +rm -f err.log +export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci +make coccicheck DEBUG_FILE="err.log" MODE=report SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c + +err.log will now have the profiling information, while stdout will +provide with some progress information as Coccinelle move forward with +work. + DEBUG_FILE support is only supported when using coccinelle >= 1.2. Additional flags diff --git a/scripts/coccicheck b/scripts/coccicheck index 3f0bb3f0fddc..1454a44d90a2 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -32,6 +32,29 @@ fi FLAGS="--very-quiet" +# You can use SPFLAGS to append extra arguments to coccicheck or override any +# heuristics done in this file as Coccinelle accepts the last options when +# options conflict. +# +# A good example for use of SPFLAGS is if you want to debug your cocci script, +# you can for instance use the following: +# +# $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci +# $ make coccicheck MODE=report DEBUG_FILE="all.err" SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c +# +# "--show-trying" should show you what rule is being processed as it goes to +# stdout, you do not need a debug file for that. The profile output will be +# be sent to stdout, if you provide a DEBUG_FILE the profiling data can be +# inspected there. +# +# --profile will not output if --very-quiet is used, so avoid it. +echo $SPFLAGS | egrep -e "--profile|--show-trying" 2>&1 > /dev/null +if [ $? -eq 0 ]; then + FLAGS="--quiet" +else + FLAGS="--very-quiet" +fi + # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" COCCIINCLUDE=${LINUXINCLUDE//-I/-I } -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 6/9] scripts: add Linux .cocciconfig for coccinelle
Coccinelle supports reading .cocciconfig, the order of precedence for variables for .cocciconfig is as follows: o Your current user's home directory is processed first o Your directory from which spatch is called is processed next o The directory provided with the --dir option is processed last, if used Since coccicheck runs through make, it naturally runs from the kernel proper dir, as such the second rule above would be implied for picking up a .cocciconfig when using 'make coccicheck'. 'make coccicheck' also supports using M= targets.If you do not supply any M= target, it is assumed you want to target the entire kernel. The kernel coccicheck script has: if [ "$KBUILD_EXTMOD" = "" ] ; then OPTIONS="--dir $srctree $COCCIINCLUDE" else OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE" fi KBUILD_EXTMOD is set when an explicit target with M= is used. For both cases the spatch --dir argument is used, as such third rule applies when whether M= is used or not, and when M= is used the target directory can have its own .cocciconfig file. When M= is not passed as an argument to coccicheck the target directory is the same as the directory from where spatch was called. If not using the kernel's coccicheck target, keep the above precedence order logic of .cocciconfig reading. If using the kernel's coccicheck target, override any of the kernel's .coccicheck's settings using SPFLAGS. We help Coccinelle when used against Linux with a set of sensible defaults options for Linux with our own Linux .cocciconfig. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now. The options picked up by coccinelle when reading a .cocciconfig do not appear as arguments to spatch processes running on your system, to confirm what options will be used by Coccinelle run: spatch --print-options-only You can override with your own preferred index option by using SPFLAGS. Coccinelle supports both glimpse and idutils. Glimpse had historically provided the best performance, however recent benchmarks reveal idutils is performing just as well. Due to some recent fixes however you however will need at least coccinelle >= 1.0.6 if using idutils. Coccinelle carries a script scripts/idutils_index.sh which creates the idutils database with as follows: mkid -i C --output .id-utils.index If using just "--use-idutils" coccinelle expects your idutils database to be on the top level of the kernel as a file named ".id-utils.index". If you do not use this you can symlink your database file to it, or you can specify the database file following the "--use-idutils" argument. Examples: make SPFLAGS=--use-idutils coccicheck This assumes you have $srctree/.id-utils.index, where $srctree is the top level of the kernel. make SPFLAGS="--use-idutils /full-path/to/ID" coccicheck Here you specify the full path of the idutils ID database. Using .cocciconfig is possible, however given the order of precedence followed by Coccinelle, and since the kernel now carries its own .cocciconfig, you will need to use SPFLAGS to use idutils if desired. v4: o Recommend upgrade for using idutils with coccinelle due to some recent fixes. o Refer to using --print-options-only for testing what options are picked up by .cocciconfig reading. o Expand commit log considerably explaining *why* .cocconfig from two precedence rules are used when using coccicheck, and how to properly override these if needed. o Expand Documentation/coccinelle.txt v3: Expand commit log a bit more Signed-off-by: Luis R. Rodriguez Acked-by: Julia Lawall --- .cocciconfig | 3 ++ .gitignore | 1 + Documentation/coccinelle.txt | 70 3 files changed, 74 insertions(+) create mode 100644 .cocciconfig diff --git a/.cocciconfig b/.cocciconfig new file mode 100644 index ..43967c6b2015 --- /dev/null +++ b/.cocciconfig @@ -0,0 +1,3 @@ +[spatch] + options = --timeout 200 + options = --use-gitgrep diff --git a/.gitignore b/.gitignore index 2be25f771bd8..c2ed4ecb0acd 100644 --- a/.gitignore +++ b/.gitignore @@ -67,6 +67,7 @@ Module.symvers # !.gitignore !.mailmap +!.cocciconfig # # Generated include files diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 66e822f8caee..b50ac7e126e8 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -189,6 +189,60 @@ work. DEBUG_FILE support is only supported when using coccinelle >= 1.2. + .cocciconfig support +~~ + +Coccinelle supports reading .cocciconfig for default Coccinelle options that +should be used every time spatch is spawned, the order of precedence for +variables for .cocciconfig is as follows: + + o Your c
[Cocci] [RFC v4 8/9] coccicheck: refer to Documentation/coccinelle.txt and wiki
Refer to the Documentation/coccinelle.txt and supplemental documentation on the wiki: https://bottest.wiki.kernel.org/coccicheck This page shall always refer to the linux-next iteration of scripts/coccicheck. v4: only refer to the wiki as supplemental documentation, and also update Documentation/coccinelle.txt. Signed-off-by: Luis R. Rodriguez --- Documentation/coccinelle.txt | 9 + scripts/coccicheck | 5 +++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index b90aa0bc8d6c..e35c0fdf1fef 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -38,6 +38,15 @@ as a regular user, and install it with sudo make install + Supplemental documentation + + +For supplemental documentation refer to the wiki: + +https://bottest.wiki.kernel.org/coccicheck + +The wiki documentation always refers to the linux-next version of the script. + Using Coccinelle on the Linux kernel ~~ diff --git a/scripts/coccicheck b/scripts/coccicheck index 0552e6152692..6676b528cfad 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -1,9 +1,10 @@ #!/bin/bash - +# Linux kernel coccicheck +# +# Read Documentation/coccinelle.txt # # This script requires at least spatch # version 1.0.0-rc11. -# DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 2/9] coccicheck: make SPFLAGS more useful
SPFLAGS is set early, it means that any heuristics done on coccicheck cannot be overridden currently. Move SPFLAGS after OPTIONS and set this at the end. This lets you override any heuristics as coccinelle treats conflicts by only listening to the last option that makes sense. v3: this patch was added in the v3 series v4: Update Documentation/coccinelle.txt explaining how SPFLAGS works as well. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 3 ++- scripts/coccicheck | 5 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 7f773d51fdd9..bb9632c20cfb 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -146,7 +146,8 @@ MODE variable explained above. ~~ Additional flags can be passed to spatch through the SPFLAGS -variable. +variable. This works as Coccinelle respects the last flags +given to it when options are in conflict. make SPFLAGS=--use-glimpse coccicheck make SPFLAGS=--use-idutils coccicheck diff --git a/scripts/coccicheck b/scripts/coccicheck index f137b04dfdd3..5319fae910b4 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -30,7 +30,7 @@ else NPROC="$J" fi -FLAGS="--very-quiet $SPFLAGS" +FLAGS="--very-quiet" # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" @@ -106,6 +106,9 @@ kill_running() { done } +# You can override heuristics with SPFLAGS, these must always go last +OPTIONS="$OPTIONS $SPFLAGS" + coccinelle () { COCCI="$1" -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 0/9] coccicheck: modernize
There were quite a bit of comments from the v3 series [0], since there was quite a bit of review needed for some other changes I've had discussions with Nicolas and Julia privately over these changes and collected their Acks privately to avoid unnecessary e-mail waste. I've annotated the changes in the commit log with any changes done in the v4 series. If most importance is the use of .cocciconfig, update of the documentation and recommending an upgrade of coccinelle for users wishing to use idutils. A future 1.0.6 release of coccinellew will support it best. This series is rebased on top of linux-next tag next-20160629, I've put up a branch of these changes up on my linux-next tree, the branch is 20160629-coccicheck-v4 [1]. [0] https://lkml.kernel.org/r/1466536893-23355-1-git-send-email-mcg...@kernel.org [1] https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160629-coccicheck-v4 Luis R. Rodriguez (9): coccicheck: move spatch binary check up coccicheck: make SPFLAGS more useful coccicheck: enable parmap support coccicheck: add support for DEBUG_FILE coccicheck: replace --very-quiet with --quiet when debugging scripts: add Linux .cocciconfig for coccinelle coccicheck: add support for requring a coccinelle version coccicheck: refer to Documentation/coccinelle.txt and wiki scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci .cocciconfig | 3 + .gitignore | 1 + Documentation/coccinelle.txt | 148 - scripts/coccicheck | 94 +++-- .../iterators/device_node_continue.cocci | 3 + 5 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 .cocciconfig -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 4/9] coccicheck: add support for DEBUG_FILE
Enable to capture stderr via a DEBUG_FILE variable passed to coccicheck. You can now do: $ rm -f cocci.err $ export COCCI=scripts/coccinelle/free/kfree.cocci $ make coccicheck MODE=report DEBUG_FILE=cocci.err ... $ cat cocci.err This will be come more useful once we add support to use more things which would go into stderr, such as profiling. That will be done separately in another commit. Expand Documentation/coccinelle.txt with details. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 20 scripts/coccicheck | 10 +- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 64daaf18874b..2516c5ef1691 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -157,6 +157,26 @@ semantic patch as shown in the previous section. The "report" mode is the default. You can select another one with the MODE variable explained above. + Debugging Coccinelle SmPL patches +~~ + +Using coccicheck is best as it provides in the spatch command line +include options matching the options used when we compile the kernel. +You can learn what these options are by using V=1, you could then +manually run Coccinelle with debug options added. + +Alternatively you can debug running Coccinelle against SmPL patches +by asking for stderr to be redirected to stderr, by default stderr +is redirected to /dev/null, if you'd like to capture stderr you +can specify the DEBUG_FILE="file.txt" option to coccicheck. For +instance: + +rm -f cocci.err +make coccicheck COCCI=scripts/coccinelle/free/kfree.cocci MODE=report DEBUG_FILE=cocci.err +cat cocci.err + +DEBUG_FILE support is only supported when using coccinelle >= 1.2. + Additional flags ~~ diff --git a/scripts/coccicheck b/scripts/coccicheck index 4b65a0fd50a1..3f0bb3f0fddc 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -96,7 +96,15 @@ run_cmd_parmap() { if [ $VERBOSE -ne 0 ] ; then echo "Running ($NPROC in parallel): $@" fi - $@ 2>/dev/null + 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 $? -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 6/9] scripts: add Linux .cocciconfig for coccinelle
Coccinelle supports reading .cocciconfig, the order of precedence for variables for .coccoconfig is as follows: o Your current user's home directory is processed first o Your directory from which spatch is called is processed next o The directory provided with the --dir option is processed last, if used Since coccicheck runs through make, it naturally runs from the kernel proper dir, as such the second rule above would be implied for picking up a .cocciconfig when using 'make coccicheck'. 'make coccicheck' also supports using M= targets, and you can also avoid supplying any target, in the later case its assumed you want to target the entire kernel. coccicheck has: if [ "$KBUILD_EXTMOD" = "" ] ; then OPTIONS="--dir $srctree $COCCIINCLUDE" else OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE" fi For both cases --dir is used, as such following the third rule the target also can have its own .cocciconfig file, since make is used for it, its the same file when no M=path is passed. If using spatch manually just keep the above order logic in mind when making changes, if you need to override any settings use SPFLAGS. We help Coccinelle when used against Linux with a set of sensible defaults options for Linux with our own Linux .cocciconfig. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now. To confirm what options will be used by Coccinelle run: spatch --print-options-only The options picked up by .cocciconfig can ony be verified with the above, by default Coccinelle does not paste these the spawned command line run for an spatch instance. Coccinelle also has support for idutils and this helps, if you use it, however its use is obviously optional. Upstream coccinelle has a script scripts/idutils_index.sh in its repository, it assumes you do: mkid -i C --output .id-utils.index Coccinelle searches for the index in the directory on which it is working ... If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows: First build the index, for example: mkid -s Pick the cocci file you wnat to work with: export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci Then run coccicheck: $ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID" Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order: o Your current user's home directory is processed first o Your directory from which spatch is called is processed next o The directory provided with the --dir option is processed last, if used v4: o Refer to using --print-options-only for testing what options are picked up by .cocciconfig reading. o Expand commit log considerably explaining *why* .cocconfig from two precedence rules are used when using coccicheck, and how to properly override these if needed. o Expand Documentation/coccinelle.txt v3: Expand commit log a bit more Signed-off-by: Luis R. Rodriguez --- .cocciconfig | 3 +++ .gitignore | 1 + Documentation/coccinelle.txt | 47 3 files changed, 51 insertions(+) create mode 100644 .cocciconfig diff --git a/.cocciconfig b/.cocciconfig new file mode 100644 index ..43967c6b2015 --- /dev/null +++ b/.cocciconfig @@ -0,0 +1,3 @@ +[spatch] + options = --timeout 200 + options = --use-gitgrep diff --git a/.gitignore b/.gitignore index 2be25f771bd8..c2ed4ecb0acd 100644 --- a/.gitignore +++ b/.gitignore @@ -67,6 +67,7 @@ Module.symvers # !.gitignore !.mailmap +!.cocciconfig # # Generated include files diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 4f5b2287a2b1..a425b1f1df99 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -189,6 +189,53 @@ work. DEBUG_FILE support is only supported when using coccinelle >= 1.2. + .cocciconfig support +~~ + +Coccinelle supports reading .cocciconfig for default Coccinelle options that +should be used every time spatch is spawned, the order of precedence for +variables for .coccoconfig is as follows: + + o Your current user's home directory is processed first + o Your directory from which spatch is called is processed next + o The directory provided with the --dir option is processed last, if used + +Since coccicheck runs through make, it naturally runs from the kernel +proper dir, as such the second rule above would be implied for picking up a +.cocciconfig when using 'make coccicheck'. + +'make coccicheck' also supports using M= targets, and you can also avoid +supplying any target, in the later case its assumed you want to target +the entire kernel. coccicheck has: + +if [ "$KBUILD_EXTMOD" = "
Re: [Cocci] [RFC v4 0/9] coccicheck: private review for modernization
On Wed, Jun 29, 2016 at 03:10:17PM -0700, Luis R. Rodriguez wrote: > Here's a v4 RFC, sending it privately to avoid bikeshedding in public. > These are rebased on top of linux-next tag next-20160623. Whoops.. so much for that. Ignore please. Will send the fixed set next... Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v4 1/9] coccicheck: move spatch binary check up
This has no functional changes. This is being done to enable us to later use spatch binary for some flag checking for certain features early on. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- scripts/coccicheck | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index f6627863fdc3..f137b04dfdd3 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -7,6 +7,11 @@ SPATCH="`which ${SPATCH:=spatch}`" +if [ ! -x "$SPATCH" ]; then +echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' +exit 1 +fi + trap kill_running SIGTERM SIGINT declare -a SPATCH_PID @@ -51,11 +56,6 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then OPTIONS="--patch $srctree $OPTIONS" fi -if [ ! -x "$SPATCH" ]; then -echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' -exit 1 -fi - if [ "$MODE" = "" ] ; then if [ "$ONLINE" = "0" ] ; then echo 'You have not explicitly specified the mode to use. Using default "report" mode.' -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 2/9] coccicheck: make SPFLAGS more useful
SPFLAGS is set early, it means that any heuristics done on coccicheck cannot be overridden currently. Move SPFLAGS after OPTIONS and set this at the end. This lets you override any heuristics as coccinelle treats conflicts by only listening to the last option that makes sense. v3: this patch was added in the v3 series v4: Update Documentation/coccinelle.txt explaining how SPFLAGS works as well. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 3 ++- scripts/coccicheck | 5 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 7f773d51fdd9..bb9632c20cfb 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -146,7 +146,8 @@ MODE variable explained above. ~~ Additional flags can be passed to spatch through the SPFLAGS -variable. +variable. This works as Coccinelle respects the last flags +given to it when options are in conflict. make SPFLAGS=--use-glimpse coccicheck make SPFLAGS=--use-idutils coccicheck diff --git a/scripts/coccicheck b/scripts/coccicheck index f137b04dfdd3..5319fae910b4 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -30,7 +30,7 @@ else NPROC="$J" fi -FLAGS="--very-quiet $SPFLAGS" +FLAGS="--very-quiet" # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" @@ -106,6 +106,9 @@ kill_running() { done } +# You can override heuristics with SPFLAGS, these must always go last +OPTIONS="$OPTIONS $SPFLAGS" + coccinelle () { COCCI="$1" -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 0/9] coccicheck: private review for modernization
Here's a v4 RFC, sending it privately to avoid bikeshedding in public. These are rebased on top of linux-next tag next-20160623. Luis R. Rodriguez (9): coccicheck: move spatch binary check up coccicheck: make SPFLAGS more useful coccicheck: enable parmap support coccicheck: add support for DEBUG_FILE 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 Documentation/coccinelle.txt and wiki scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci .cocciconfig | 3 + .gitignore | 1 + Documentation/coccinelle.txt | 125 - scripts/coccicheck | 96 ++-- .../iterators/device_node_continue.cocci | 3 + 5 files changed, 218 insertions(+), 10 deletions(-) create mode 100644 .cocciconfig -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 4/9] coccicheck: add support for DEBUG_FILE
Enable to capture stderr via a DEBUG_FILE variable passed to coccicheck. You can now do: $ rm -f cocci.err $ export COCCI=scripts/coccinelle/free/kfree.cocci $ make coccicheck MODE=report DEBUG_FILE=cocci.err ... $ cat cocci.err This will be come more useful once we add support to use more things which would go into stderr, such as profiling. That will be done separately in another commit. Expand Documentation/coccinelle.txt with details. Signed-off-by: Luis R. Rodriguez --- Documentation/coccinelle.txt | 20 scripts/coccicheck | 10 +- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index 64daaf18874b..2516c5ef1691 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -157,6 +157,26 @@ semantic patch as shown in the previous section. The "report" mode is the default. You can select another one with the MODE variable explained above. + Debugging Coccinelle SmPL patches +~~ + +Using coccicheck is best as it provides in the spatch command line +include options matching the options used when we compile the kernel. +You can learn what these options are by using V=1, you could then +manually run Coccinelle with debug options added. + +Alternatively you can debug running Coccinelle against SmPL patches +by asking for stderr to be redirected to stderr, by default stderr +is redirected to /dev/null, if you'd like to capture stderr you +can specify the DEBUG_FILE="file.txt" option to coccicheck. For +instance: + +rm -f cocci.err +make coccicheck COCCI=scripts/coccinelle/free/kfree.cocci MODE=report DEBUG_FILE=cocci.err +cat cocci.err + +DEBUG_FILE support is only supported when using coccinelle >= 1.2. + Additional flags ~~ diff --git a/scripts/coccicheck b/scripts/coccicheck index 4b65a0fd50a1..3f0bb3f0fddc 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -96,7 +96,15 @@ run_cmd_parmap() { if [ $VERBOSE -ne 0 ] ; then echo "Running ($NPROC in parallel): $@" fi - $@ 2>/dev/null + 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 $? -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 3/9] coccicheck: enable parmap support
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 and use it only if your number of processors detected is > 1. If parmap is enabled also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it. stderr is currently sent to /dev/null, addressing a way to capture that will be addressed next. If --jobs is not supported we fallback to the old mechanism. We expect to deprecate the old mechanism as soon as we can get confirmation all users are ready. 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. Extend Documentation/coccinelle.txt as well. 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 ... real29m14.912s user103m1.796s sys 0m4.464s After: real16m22.435s user128m30.060s sys 0m2.712s v4: o expand Documentation/coccinelle.txt to reflect parmap support info o update commit log to reflect what we actually do now with stderr o split out DEBUG_FILE use into another patch o detect number of CPUs and if its 1 then skip parmap support, note that if you still support parmap, but have 1 CPU you will also go through the new branches, so the old complex multithreaded process is skipped as well. 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 --- Documentation/coccinelle.txt | 15 +++ scripts/coccicheck | 35 --- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index bb9632c20cfb..64daaf18874b 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -94,11 +94,26 @@ To enable verbose messages set the V= variable, for example: make coccicheck MODE=report V=1 + Coccinelle parallelization + + By default, coccicheck tries to run as parallel as possible. To change the parallelism, set the J= variable. For example, to run across 4 CPUs: make coccicheck MODE=report J=4 +As of Coccinelle 1.0.2 Coccinelle uses Ocaml parmap for parallelization, +if support for this is detected you will benefit from parmap parallelization. + +When parmap is enabled coccicheck will enable dynamic load balancing by using +'--chunksize 1' argument, this ensures we keep feeding threads with work +one by one, so that we avoid the situation where most work gets done by only +a few threads. With dynamic load balancing, if a thread finishes early we keep +feeding it more work. + +When parmap is enabled, if an error occurs in Coccinelle, this error +value is propagated back, the return value of the 'make coccicheck' +captures this return value. Using Coccinelle with a single semantic patch ~~~ diff --git a/scripts/coccicheck b/scripts/coccicheck index 5319fae910b4..4b65a0fd50a1 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,16 @@ 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 +elif [ "$NPROC" != "1" ]; then + # Using 0 should work as well, refer to _SC_NPROCESSORS_ONLN use on + # https://github.com/rdicosmo/parmap/blob/master/setcore_stubs.c + 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 +92,18 @@ if [ "$ONLINE" = "0" ] ; then e
[Cocci] [PATCH v4 7/9] coccicheck: add support for requring a coccinelle version
Enable Coccinelle SmPL patches to require a specific version of Coccinelle. In the event that the version does not match we just inform the user, if the user asked to go through all SmPL patches we just inform them of the need for a new version of coccinelle for the SmPL patch and continue on with the rest. This uses the simple kernel scripts/ld-version.sh to create a weight on the version provided by spatch. The -dirty attribute is ignored if supplied, the benefit of scripts/ld-version.sh is it has a long history and well tested. While at it, document the // Options stuff as well. v4: Document // Options and // Requires as well on Documentation/coccinelle.txt. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 19 +++ scripts/coccicheck | 14 ++ 2 files changed, 33 insertions(+) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index b50ac7e126e8..1c26908ebc16 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -277,6 +277,25 @@ thus active by default. However, by indexing the code with one of these tools, and according to the cocci file used, spatch could proceed the entire code base more quickly. + SmPL patch specific options +~ + +SmPL patches can have their own requirements for options passed +to Coccinelle. SmPL patch specific options can be provided by +providing them at the top of the SmPL patch, for instance: + +// Options: --no-includes --include-headers + + SmPL patch Coccinelle requirements + + +As Coccinelle features get added some more advanced SmPL patches +may require newer versions of Coccinelle. If an SmPL patch requires +at least a version of Coccinelle, this can be specified as follows, +as an example if requiring at least Coccinelle >= 1.0.5: + +// Requires: 1.0.5 + Proposing new semantic patches diff --git a/scripts/coccicheck b/scripts/coccicheck index 081ba5bff79c..f9293ab04a8b 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -5,6 +5,7 @@ # version 1.0.0-rc11. # +DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" if [ ! -x "$SPATCH" ]; then @@ -12,6 +13,9 @@ if [ ! -x "$SPATCH" ]; then exit 1 fi +SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}') +SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh) + USE_JOBS="no" $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes" @@ -171,6 +175,16 @@ coccinelle () { COCCI="$1" OPT=`grep "Option" $COCCI | cut -d':' -f2` +REQ=`grep "Requires" $COCCI | cut -d':' -f2 | sed "s| ||"` +REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh) +if [ "$REQ_NUM" != "0" ] ; then + if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then + echo "Skipping coccinele SmPL patch: $COCCI" + echo "You have coccinelle: $SPATCH_VERSION" + echo "This SmPL patch requires: $REQ" + return + fi +fi # The option '--parse-cocci' can be used to syntactically check the SmPL files. # -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [RFC v4 7/9] coccicheck: add support for requring a coccinelle version
Enable Coccinelle SmPL patches to require a specific version of Coccinelle. In the event that the version does not match we just inform the user, if the user asked to go through all SmPL patches we just inform them of the need for a new version of coccinelle for the SmPL patch and continue on with the rest. This uses the simple kernel scripts/ld-version.sh to create a weight on the version provided by spatch. The -dirty attribute is ignored if supplied, the benefit of scripts/ld-version.sh is it has a long history and well tested. While at it, document the // Options stuff as well. v4: Document // Options and // Requires as well on Documentation/coccinelle.txt. Signed-off-by: Luis R. Rodriguez Acked-by: Nicolas Palix --- Documentation/coccinelle.txt | 19 +++ scripts/coccicheck | 14 ++ 2 files changed, 33 insertions(+) diff --git a/Documentation/coccinelle.txt b/Documentation/coccinelle.txt index a425b1f1df99..b90aa0bc8d6c 100644 --- a/Documentation/coccinelle.txt +++ b/Documentation/coccinelle.txt @@ -254,6 +254,25 @@ thus active by default. However, by indexing the code with one of these tools, and according to the cocci file used, spatch could proceed the entire code base more quickly. + SmPL patch specific options +~ + +SmPL patches can have their own requirements for options passed +to Coccinelle. SmPL patch specific options can be provided by +providing them at the top of the SmPL patch, for instance: + +// Options: --no-includes --include-headers + + SmPL patch Coccinelle requirements + + +As Coccinelle features get added some more advanced SmPL patches +may require newer versions of Coccinelle. If an SmPL patch requires +at least a version of Coccinelle, this can be specified as follows, +as an example if requiring at least Coccinelle >= 1.0.5: + +// Requires: 1.0.5 + Proposing new semantic patches diff --git a/scripts/coccicheck b/scripts/coccicheck index 1454a44d90a2..0552e6152692 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -5,6 +5,7 @@ # version 1.0.0-rc11. # +DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" if [ ! -x "$SPATCH" ]; then @@ -12,6 +13,9 @@ if [ ! -x "$SPATCH" ]; then exit 1 fi +SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}') +SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh) + USE_JOBS="no" $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes" @@ -173,6 +177,16 @@ coccinelle () { COCCI="$1" OPT=`grep "Option" $COCCI | cut -d':' -f2` +REQ=`grep "Requires" $COCCI | cut -d':' -f2 | sed "s| ||"` +REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh) +if [ "$REQ_NUM" != "0" ] ; then + if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then + echo "Skipping coccinele SmPL patch: $COCCI" + echo "You have coccinelle: $SPATCH_VERSION" + echo "This SmPL patch requires: $REQ" + return + fi +fi # The option '--parse-cocci' can be used to syntactically check the SmPL files. # -- 2.8.4 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3] bundles/pycaml/: use .NOTPARALLEL on bundles
On Sat, Jun 25, 2016 at 02:54:46PM +0200, SF Markus Elfring wrote: > > bundles/menhirLib/Makefile | 1 + > > bundles/parmap/Makefile| 1 + > > bundles/pcre/Makefile | 1 + > > bundles/pycaml/Makefile| 1 + > > How do you think about to [...] I'm dropping this patch in favor of some other stuff being which will fix this in a better way. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 sup
Re: [Cocci] [PATCH v3 5/8] scripts: add Linux .cocciconfig for coccinelle
On Tue, Jun 21, 2016 at 11:12:54PM +0200, Julia Lawall wrote: > On Tue, 21 Jun 2016, Nicolas Palix (LIG) wrote: > > > Le 21/06/16 à 21:21, Luis R. Rodriguez a écrit : > > > Help Coccinelle when used against Linux with a set of sensible defaults > > > options for Linux. This hints to coccinelle git can be used for 'git grep' > > > queries over coccigrep. A timeout of 200 seconds should suffice for now. > > > > > > If you use idutils you can override for 'make coccicheck' by using the > > > SPFLAGS option as follows: > > > > > > First build the index, for example: > > > mkid -s > > > > > > Pick the cocci file you wnat to work with: > > > export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci > > > > > > Then run coccicheck: > > > $ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID" > > > > > > Coccinelle supports reading .cocciconfig from different directories, > > > the later one overriding the previous reads in the following order: > > > > > > o Your current user's home directory is processed first > > > o Your directory from which spatch is called is processed next > > > o The directory provided with the --dir option is processed last, if used > > > > > > Signed-off-by: Luis R. Rodriguez > > Acked-by: Nicolas Palix > > Hmm, I can see at least some advantages to encouraging people to do it the > Coccinelle way, with the Coccinelle script rather than mkid directly. > Then, if we need some other features specific to Coccinelle, we can just > add them. I can simply document that if users are used to using their own target output file, and if they wanted it to be used by coccinelle simply symlinking .id-utils.index to it would enable coccinelle to pick it up by default. If that is done, would the new .cocciconfig not override though? Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 5/8] scripts: add Linux .cocciconfig for coccinelle
On Tue, Jun 21, 2016 at 10:29:53PM +0200, Julia Lawall wrote: [...] > mkid -i C --output .id-utils.index > > [...] Coccinelle searches for the index in the directory > on which it is working Can you clarify if this is $PWD from which we spawn spatch or the --dir, or the current directory that spatch is working on at the moment. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation
On Tue, Jun 21, 2016 at 11:18:41PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Nicolas Palix (LIG) wrote: > > > Hi, > > > > Le 21/06/16 à 21:21, Luis R. Rodriguez a écrit : > > > Sprinkling *tons* of documentation on the script is not a good > > > idea, instead refer to a wiki for further coccicheck documentation: > > > > > > https://bottest.wiki.kernel.org/coccicheck > > > > > > This page shall always refer to the linux-next iteration of > > > scripts/coccicheck. > > > > Can you say a word about Documentation/coccinelle.txt too ? > > And update it according to the script changes. > > I think this would be a good idea. I'd much prefer to just keep docs in a wiki, and we nuke this outdated doc or refer in the Documentation/coccinelle.txt to the wiki. This lets us document things much faster and lets us add more information in a much nicer format. But that's just me... The existing file Documentation/coccinelle.txt even provides documentation on how to build coccinelle --we're way passed that point. Note, I havne't updated the docs on the wiki to refer to any of this, and would not unless these changes get merged at least to linux-next. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize
On Tue, Jun 21, 2016 at 11:30:03PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > On Tue, Jun 21, 2016 at 11:02:49PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > > That is sanitized as follows: > > > > > > > > # spatch only allows include directories with the syntax "-I include" > > > > > > > > # while gcc also allows "-Iinclude" and "-include include" > > > > > > > > COCCIINCLUDE=${LINUXINCLUDE//-I/-I } > > > > > > > > COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} > > > > > > I don't get the second case. Is it to replace -include by --include? > > > Coccinelle actually supports both, although it doesn't advertise that. > > > > Oh neat, yeah. So a follow up patch later can be to remove that second line? > > If so as of what version of coccinelle? > > Forever. Single - has always been supported. Double - was added at some > point. OK so indeed the second line above is indeed not needed for sure. After this series settles we can nuke that line. > > > Also, in LINUXINCLUDE, what is the meaning of -include? For Coccinelle, > > > it is not the same as -I. It is for files that should be included that > > > are not in the set of includes seen by whatever is the specified include > > > strategy (--all-includes, etc). The argument is a specific file name, > > > not > > > a directory. It is a way of eg not bothering with --recursive-includes > > > when there is one or a few key header files that each file will need. > > > > Its used to force to include a single file, it is a file. > > OK, close enough then. Great thanks. > > > > So the point is to annotate that the .cocconfig is picked up first due > > > > to the fact make is used and its issued from the top level makefile > > > > and starts from the top level. The fact that --dir is used is important > > > > but secondary to its introduction as well. > > > > > > OK, the original text seemed to me to imply that running from the kernel > > > directory was essential to getting the kernels .cocciconfig, > > > > And what I meant to imply was that since coccicheck uses the kernel > > makefiles it would kick off from kernel proper. > > > > > so I wanted to point out that this is not the case. > > > > I should have elaborated with all these details, its perhaps best to be > > explicit about this so I can respin with a clearer commit log. > > Thanks. People may come across this message, and it could be good for it > to be as helpful as possible. Indeed. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize
On Tue, Jun 21, 2016 at 11:02:49PM +0200, Julia Lawall wrote: > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > That is sanitized as follows: > > > > # spatch only allows include directories with the syntax "-I include" > > > > # while gcc also allows "-Iinclude" and "-include include" > > > > COCCIINCLUDE=${LINUXINCLUDE//-I/-I } > > > > COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} > > I don't get the second case. Is it to replace -include by --include? > Coccinelle actually supports both, although it doesn't advertise that. Oh neat, yeah. So a follow up patch later can be to remove that second line? If so as of what version of coccinelle? > Also, in LINUXINCLUDE, what is the meaning of -include? For Coccinelle, > it is not the same as -I. It is for files that should be included that > are not in the set of includes seen by whatever is the specified include > strategy (--all-includes, etc). The argument is a specific file name, not > a directory. It is a way of eg not bothering with --recursive-includes > when there is one or a few key header files that each file will need. Its used to force to include a single file, it is a file. > > So the point is to annotate that the .cocconfig is picked up first due > > to the fact make is used and its issued from the top level makefile > > and starts from the top level. The fact that --dir is used is important > > but secondary to its introduction as well. > > OK, the original text seemed to me to imply that running from the kernel > directory was essential to getting the kernels .cocciconfig, And what I meant to imply was that since coccicheck uses the kernel makefiles it would kick off from kernel proper. > so I wanted to point out that this is not the case. I should have elaborated with all these details, its perhaps best to be explicit about this so I can respin with a clearer commit log. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 0/8] coccicheck: modernize
On Tue, Jun 21, 2016 at 10:13:31PM +0200, Julia Lawall wrote: > > > On Tue, 21 Jun 2016, Luis R. Rodriguez wrote: > > > 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. > > I don't understand this point. Coccinelle picks up the .cocciconfig, if > any, of the directory on which you want to work, not of the current one. The order of precedence for variables for .coccoconfig is as follows: o Your current user's home directory is processed first o Your directory from which spatch is called is processed next o The directory provided with the --dir option is processed last, if used Since coccicheck runs through make, it naturally runs from the kernel proper dir, as such the second rule above would be implied for picking up a .cocciconfig. That's part of the point I'm making. Up next let us consider when M= is used or when it is not used, if used it populates KBUILD_EXTMOD. if [ "$KBUILD_EXTMOD" = "" ] ; then OPTIONS="--dir $srctree $COCCIINCLUDE" else OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE" fi Either way --dir is used, so the third rule applies and so your .cocciconfig from there is also read if one is found. My other point was that $COCCIINCLUDE has some useful tidbits of includes for coccinelle, and that also assumes one is on the top level dir of the kernel. That is sanitized as follows: # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" COCCIINCLUDE=${LINUXINCLUDE//-I/-I } COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} So the point is to annotate that the .cocconfig is picked up first due to the fact make is used and its issued from the top level makefile and starts from the top level. The fact that --dir is used is important but secondary to its introduction as well. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] bundles/pycaml/: force pycaml preparation to be done serially
On Tue, Jun 21, 2016 at 07:54:42PM +0200, SF Markus Elfring wrote: > I suggest therefore to consider additional possibilities. Patches welcomed. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 1/8] coccicheck: move spatch binary check up
This has no functional changes. This is being done to enable us to later use spatch binary for some flag checking for certain features early on. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index f6627863fdc3..f137b04dfdd3 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -7,6 +7,11 @@ SPATCH="`which ${SPATCH:=spatch}`" +if [ ! -x "$SPATCH" ]; then +echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' +exit 1 +fi + trap kill_running SIGTERM SIGINT declare -a SPATCH_PID @@ -51,11 +56,6 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then OPTIONS="--patch $srctree $OPTIONS" fi -if [ ! -x "$SPATCH" ]; then -echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' -exit 1 -fi - if [ "$MODE" = "" ] ; then if [ "$ONLINE" = "0" ] ; then echo 'You have not explicitly specified the mode to use. Using default "report" mode.' -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 3/8] coccicheck: enable parmap support
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 ... real29m14.912s user103m1.796s sys 0m4.464s After: real16m22.435s user128m30.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 --- 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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 5/8] scripts: add Linux .cocciconfig for coccinelle
Help Coccinelle when used against Linux with a set of sensible defaults options for Linux. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now. If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows: First build the index, for example: mkid -s Pick the cocci file you wnat to work with: export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci Then run coccicheck: $ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID" Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order: o Your current user's home directory is processed first o Your directory from which spatch is called is processed next o The directory provided with the --dir option is processed last, if used Signed-off-by: Luis R. Rodriguez --- .cocciconfig | 3 +++ .gitignore | 1 + 2 files changed, 4 insertions(+) create mode 100644 .cocciconfig diff --git a/.cocciconfig b/.cocciconfig new file mode 100644 index ..43967c6b2015 --- /dev/null +++ b/.cocciconfig @@ -0,0 +1,3 @@ +[spatch] + options = --timeout 200 + options = --use-gitgrep diff --git a/.gitignore b/.gitignore index 2be25f771bd8..c2ed4ecb0acd 100644 --- a/.gitignore +++ b/.gitignore @@ -67,6 +67,7 @@ Module.symvers # !.gitignore !.mailmap +!.cocciconfig # # Generated include files -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 6/8] coccicheck: add support for requring a coccinelle version
Enable Coccinelle SmPL patches to require a specific version of Coccinelle. In the event that the version does not match we just inform the user, if the user asked to go through all SmPL patches we just inform them of the need for a new version of coccinelle for the SmPL patch and continue on with the rest. This uses the simple kernel scripts/ld-version.sh to create a weight on the version provided by spatch. The -dirty attribute is ignored if supplied, the benefit of scripts/ld-version.sh is it has a long history and well tested. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 14 ++ 1 file changed, 14 insertions(+) diff --git a/scripts/coccicheck b/scripts/coccicheck index 998d764636e0..01b6716ea931 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -5,6 +5,7 @@ # version 1.0.0-rc11. # +DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" if [ ! -x "$SPATCH" ]; then @@ -12,6 +13,9 @@ if [ ! -x "$SPATCH" ]; then exit 1 fi +SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}') +SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh) + USE_JOBS="no" $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes" @@ -171,6 +175,16 @@ coccinelle () { COCCI="$1" OPT=`grep "Option" $COCCI | cut -d':' -f2` +REQ=`grep "Requires" $COCCI | cut -d':' -f2 | sed "s| ||"` +REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh) +if [ "$REQ_NUM" != "0" ] ; then + if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then + echo "Skipping coccinele SmPL patch: $COCCI" + echo "You have coccinelle: $SPATCH_VERSION" + echo "This SmPL patch requires: $REQ" + return + fi +fi # The option '--parse-cocci' can be used to syntactically check the SmPL files. # -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 0/8] coccicheck: modernize
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-mcg...@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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 4/8] coccicheck: replace --very-quiet with --quit when debugging
When debugging (using --profile or --show-trying) you want to avoid supressing output, use --quiet instead. While at it, extend documentation for SPFLAGS use. For instance one can use: $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ make coccicheck DEBUG_FILE="poo.err" MODE=report SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c v3: rebased, resolve conflicts v2: use egrep instead of the *"=--option"* check, this doesn't work for disjunctions. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 23 +++ 1 file changed, 23 insertions(+) diff --git a/scripts/coccicheck b/scripts/coccicheck index a77f0f246405..998d764636e0 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -32,6 +32,29 @@ fi FLAGS="--very-quiet" +# You can use SPFLAGS to append extra arguments to coccicheck or override any +# heuristics done in this file as Coccinelle accepts the last options when +# options conflict. +# +# A good example for use of SPFLAGS is if you want to debug your cocci script, +# you can for instance use the following: +# +# $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci +# $ make coccicheck MODE=report DEBUG_FILE="all.err" SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c +# +# "--show-trying" should show you what rule is being processed as it goes to +# stdout, you do not need a debug file for that. The profile output will be +# be sent to stdout, if you provide a DEBUG_FILE the profiling data can be +# inspected there. +# +# --profile will not output if --very-quiet is used, so avoid it. +echo $SPFLAGS | egrep -e "--profile|--show-trying" 2>&1 > /dev/null +if [ $? -eq 0 ]; then + FLAGS="--quiet" +else + FLAGS="--very-quiet" +fi + # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" COCCIINCLUDE=${LINUXINCLUDE//-I/-I } -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
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 --- 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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 7/8] coccicheck: refer to coccicheck bottest wiki for documentation
Sprinkling *tons* of documentation on the script is not a good idea, instead refer to a wiki for further coccicheck documentation: https://bottest.wiki.kernel.org/coccicheck This page shall always refer to the linux-next iteration of scripts/coccicheck. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index 01b6716ea931..adbcbbd1aad6 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -1,9 +1,14 @@ #!/bin/bash - +# Linux kernel coccicheck +# +# For more detailed documentation refer to: +# +# https://bottest.wiki.kernel.org/coccicheck +# +# This documentation always refers to the linux-next version of the script. # # This script requires at least spatch # version 1.0.0-rc11. -# DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3 2/8] coccicheck: make SPFLAGS more useful
SPFLAGS is set early, it means that any heuristics done on coccicheck cannot be overridden currently. Move SPFLAGS after OPTIONS and set this at the end. This lets you override any heuristics as coccinelle treats conflicts by only listening to the last option that makes sense. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index f137b04dfdd3..5319fae910b4 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -30,7 +30,7 @@ else NPROC="$J" fi -FLAGS="--very-quiet $SPFLAGS" +FLAGS="--very-quiet" # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" @@ -106,6 +106,9 @@ kill_running() { done } +# You can override heuristics with SPFLAGS, these must always go last +OPTIONS="$OPTIONS $SPFLAGS" + coccinelle () { COCCI="$1" -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] bundles/pycaml/: force pycaml preparation to be done serially
On Tue, Jun 21, 2016 at 9:22 AM, SF Markus Elfring wrote: >> …, the only consistent fix seems to be to use >> .NOTPARALLEL on all bundles. > > How do you think about a bit more fine-tuning for this setting? > > Would you really like to switch parallelisation off for the affected make > scripts completely? Building should work, period. Optimizations to building can be done, but since parallel building is brokena and something fixes it, that should be consdered first as it at least fixes general parallel building. The disabling of parallel building here is just for the prepare target. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] bundles/pycaml/: force pycaml preparation to be done serially
On Sat, Jun 18, 2016 at 09:12:16AM +0200, SF Markus Elfring wrote: > > +.ONESHELL: This ended causing some loop on some system, the only consistent fix seems to be to use .NOTPARALLEL on all bundles. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v3] bundles/pycaml/: use .NOTPARALLEL on bundles
We need to disable parallel builds on the preparation of bundles. This fixes parallel compilation when using 32 threads on a 32-core system and on an 8 core system: The error: make[6]: Entering directory '/home/mcgrof/coccinelle/bundles/pycaml' /usr/bin/ocamlopt.opt -I chemoelectric-pycaml-8614105 -cclib -lpycaml_stubs -ccopt -I/usr/include/python2.7 -ccopt -I/usr/include/x86_64-linux-gnu/python2.7 -cclib -lpython2.7 -c chemoelectric-pycaml-8614105/pycaml.ml -o chemoelectric-pycaml-8614105/pycaml.cmx make[6]: *** No rule to make target 'chemoelectric-pycaml-8614105/pycaml.a', needed by 'all-opt-build-targets'. Stop. make[6]: *** Waiting for unfinished jobs make[6]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' ../Makefile.bundles:33: recipe for target 'all.opt' failed make[5]: *** [all.opt] Error 2 make[5]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' Makefile:186: recipe for target '/home/mcgrof/coccinelle/bundles/pycaml/.opt' failed make[4]: *** [/home/mcgrof/coccinelle/bundles/pycaml/.opt] Error 2 make[4]: Leaving directory '/home/mcgrof/coccinelle' Makefile:180: recipe for target 'subdirs.opt' failed make[3]: *** [subdirs.opt] Error 1 make[3]: Leaving directory '/home/mcgrof/coccinelle' Makefile:163: recipe for target 'opt-compil' failed make[2]: *** [opt-compil] Error 2 make[2]: Leaving directory '/home/mcgrof/coccinelle' Makefile:143: recipe for target 'all-release' failed make[1]: *** [all-release] Error 2 make[1]: Leaving directory '/home/mcgrof/coccinelle' Makefile:121: recipe for target 'all' failed make: *** [all] Error 2 Signed-off-by: Luis R. Rodriguez --- bundles/menhirLib/Makefile | 1 + bundles/parmap/Makefile| 1 + bundles/pcre/Makefile | 1 + bundles/pycaml/Makefile| 1 + 4 files changed, 4 insertions(+) diff --git a/bundles/menhirLib/Makefile b/bundles/menhirLib/Makefile index 23ac0d6..e1bc4d2 100644 --- a/bundles/menhirLib/Makefile +++ b/bundles/menhirLib/Makefile @@ -33,6 +33,7 @@ include ../Makefile.bundles depend: .depend @$(MAKE) all-build +.NOTPARALLEL: .prepare .prepare: $(MARKER) echo "let libdir = \"$(shell pwd\)\"" > $(SRC_DIR)/installation.ml echo "let ocamlfind = false" >> $(SRC_DIR)/installation.ml diff --git a/bundles/parmap/Makefile b/bundles/parmap/Makefile index 67c65ec..675a71d 100644 --- a/bundles/parmap/Makefile +++ b/bundles/parmap/Makefile @@ -17,6 +17,7 @@ OCAMLCCFLAGS+=-ccopt -D_GNU_SOURCE include ../Makefile.bundles +.NOTPARALLEL: .prepare .prepare: $(MARKER) cd $(SRC_DIR) && ./configure touch $@ diff --git a/bundles/pcre/Makefile b/bundles/pcre/Makefile index 97aa12e..4a07d2f 100644 --- a/bundles/pcre/Makefile +++ b/bundles/pcre/Makefile @@ -24,6 +24,7 @@ EXTRALNKFLAGS=$(PCRE_LIBS:%=-ldopt %) include ../Makefile.bundles +.NOTPARALLEL: .prepare .prepare: $(MARKER) if [ "$$OCAMLVERSION" "<" 4.02.0 ]; then \ cp $(SRC_DIR)/pcre_compat312.ml $(SRC_DIR)/pcre_compat.ml; \ diff --git a/bundles/pycaml/Makefile b/bundles/pycaml/Makefile index 5045c27..0ee7757 100644 --- a/bundles/pycaml/Makefile +++ b/bundles/pycaml/Makefile @@ -17,6 +17,7 @@ export PYMAJOR=$(shell echo ${PYTHON_VERSION} | sed -e 's/\..*//') include ../Makefile.bundles +.NOTPARALLEL: .prepare .prepare: $(MARKER) rm -f "$(SRC_DIR)/pycaml.mli" $(CC) -E -w -D PYMAJOR$(PYMAJOR) -xc pycaml.pp.ml >$(SRC_DIR)/pycaml.ml -- 2.7.0 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 1/3] docs/demos: add a few ++ documentation and demos
This perhaps is not the best demo for use of ++ but it should suffice. This adds some basic documentation for it and a demo. Signed-off-by: Luis R. Rodriguez --- demos/plusplus1.c| 4 ++ demos/plusplus1.cocci| 17 demos/plusplus1.res | 5 +++ demos/plusplus2.c| 7 demos/plusplus2.cocci| 26 demos/plusplus2.res | 10 + docs/manual/cocci_syntax.tex | 95 7 files changed, 164 insertions(+) create mode 100644 demos/plusplus1.c create mode 100644 demos/plusplus1.cocci create mode 100644 demos/plusplus1.res create mode 100644 demos/plusplus2.c create mode 100644 demos/plusplus2.cocci create mode 100644 demos/plusplus2.res diff --git a/demos/plusplus1.c b/demos/plusplus1.c new file mode 100644 index ..2a28c8037cac --- /dev/null +++ b/demos/plusplus1.c @@ -0,0 +1,4 @@ +struct x { + int z; + char b; +}; diff --git a/demos/plusplus1.cocci b/demos/plusplus1.cocci new file mode 100644 index ..86e5950b2a93 --- /dev/null +++ b/demos/plusplus1.cocci @@ -0,0 +1,17 @@ +// This adds a float for one int variable found. +// Doing this however is limited to one int in the data structure, if +// more one int variable in the structure we would not know where to +// place the new one safely order-wise. See plusplus2.cocci for an +// example of dealing with this issue. + +@simpleplus@ +identifier x,v; +fresh identifier xx = v ## "_float"; +@@ + +struct x { ++ float xx; + ... + int v; + ... +} diff --git a/demos/plusplus1.res b/demos/plusplus1.res new file mode 100644 index ..ba5c5341639e --- /dev/null +++ b/demos/plusplus1.res @@ -0,0 +1,5 @@ +struct x { + float z_float; + int z; + char b; +}; diff --git a/demos/plusplus2.c b/demos/plusplus2.c new file mode 100644 index ..a03d26c6da14 --- /dev/null +++ b/demos/plusplus2.c @@ -0,0 +1,7 @@ +struct x { + int z; + int a; + char b; + int c; + int *d; +}; diff --git a/demos/plusplus2.cocci b/demos/plusplus2.cocci new file mode 100644 index ..bd61ac8f8771 --- /dev/null +++ b/demos/plusplus2.cocci @@ -0,0 +1,26 @@ +// This can support having a structure with more than two int variables, +// and adding a respective float for each. It can do this as order does +// not matter. +// +// This uses ++ to support the fact that the rule may be working +// with multiple variables that we need to modify and that order +// does not matter. +// +// If you don't use "++" you'll get "already tagged token" error since +// Coccinelle is concerned that the user has no way of specifying the order +// in which they should appear. By using "++" you are telling Coccinelle +// +// "I know that a lot of things can collect here, and I'm OK +//with that. I'm also OK with things getting added out of order. + +@plusplus@ +identifier x,v; +fresh identifier xx = v ## "_float"; +@@ + +struct x { +++ float xx; + ... + int v; + ... +} diff --git a/demos/plusplus2.res b/demos/plusplus2.res new file mode 100644 index ..4920fb7d7df9 --- /dev/null +++ b/demos/plusplus2.res @@ -0,0 +1,10 @@ +struct x { + float a_float; + float c_float; + float z_float; + int z; + int a; + char b; + int c; + int *d; +}; diff --git a/docs/manual/cocci_syntax.tex b/docs/manual/cocci_syntax.tex index e8d74a7d015f..49602ef1d732 100644 --- a/docs/manual/cocci_syntax.tex +++ b/docs/manual/cocci_syntax.tex @@ -851,6 +851,11 @@ rule should apply if rule XXX was never matched at all. \section{Transformation} +Coccinelle allows for transformations to enable modifying C code using +very precise grammar. + +\subsection{Basic transformations} + The transformation specification essentially has the form of C code, except that lines to remove are annotated with \verb+-+ in the first column, and lines to add are annotated with \verb-+-. A transformation specification @@ -1100,6 +1105,96 @@ write Some kinds of terms can only appear in + code. These include comments, ifdefs, and attributes (\texttt{\_\_attribute\_\_((...))}). +\subsection{Advanced transformations} + +You may run into the situation where grammar you specificy for +transformations might associate itself with a few consecutive tokens +in code, in such cases Coccinelle cannot gaurantee order when making +additions. If you are sure that order does not matter you can use the +optional double addition token \texttt{++} to annotate that Coccinelle +may add things out of order. This may be for intance rather safe in +certain situations when extending a data structure with more members, +based on existing members of the data structure. The following rule helps +to extend a data structure with a respective float for one present int. +This
[Cocci] [PATCH v2 2/3] docs/manual/cocci_syntax.tex: extend with python iteration
Signed-off-by: Luis R. Rodriguez --- docs/manual/cocci_syntax.tex | 51 ++-- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/docs/manual/cocci_syntax.tex b/docs/manual/cocci_syntax.tex index 49602ef1d732..5adfc95b0ce1 100644 --- a/docs/manual/cocci_syntax.tex +++ b/docs/manual/cocci_syntax.tex @@ -1698,9 +1698,12 @@ of every token. \section{Iteration} It is possible to iterate Coccinelle, giving the subsequent iterations a -different set of virtual rules or virtual identifier bindings. And example -is found in {\tt demos/iteration.cocci}. The example shown there is as -follows: +different set of virtual rules or virtual identifier bindings. Coccinelle +currently supports iteration with both ocaml and python scripting. An +example with ocaml is fond in {\tt demos/iteration.cocci}, a python +exmaple is found in {\tt demos/python\_iteration.cocci}. + +The ocaml scripting iteration example starts as follows. \begin{quote} \begin{verbatim} @@ -1724,6 +1727,29 @@ with Not_found -> \end{verbatim} \end{quote} +The respective python scripting iteration example starts as follows: + +\begin{quote} +\begin{verbatim} +virtual after_start + +@initialize:python@ +@@ + +seen = set() + +def add_if_not_present (source, f, file): +if (f, file) not in seen: +seen.add((f, file)) +it = Iteration() +if file != None: +it.set_files([file]) +it.add_virtual_rule(after_start) +it.add_virtual_identifier(err_ptr_function, f) +it.register() +\end{verbatim} +\end{quote} + The virtual rule {\tt after\_start} is used to distinguish between the first iteration (in which it is not considered to have matched) and all others. This is done by not mentioning {\tt after\_start} in the command @@ -1734,11 +1760,12 @@ The main code for performing the iteration is found in the function {\tt {\tt register}. {\tt New iteration} creates a structure representing the new iteration. {\tt set\_files} sets the list of files to be considered on the new iteration. If this function is not called, the new iteration -treats the same files as the current iteration. {\tt add\_virtual\_rule A} -has the same effect as putting {\tt -D a} on the command line. Note that -the first letter of the rule name is capitalized, although this is not done -elsewhere. {\tt add\_virtual\_identifier X v} has the same effect as -putting {\tt -D x=v} on the command line. Note again the case change. +treats the same files as the current iteration. {\tt add\_virtual\_rule a} +has the same effect as putting {\tt -D a} on the command line. If +using ocaml scripting instead of python scripting the first letter of the rule +name is capitalized, although this is not done elsewhere. +{\tt add\_virtual\_identifier x v} has the same effect as putting {\tt -D x=v} +on the command line. Again, when using ocaml scripting there is a case change. {\tt extend\_virtual\_identifiers()} (not shown) preserves all virtual identifiers of the current iteration that are not overridden by calls to {\tt add\_virtual\_identifier}. Finally, the call to {\tt register} queues @@ -1749,9 +1776,11 @@ Modification is not allowed when using iteration. Thus, it is required to use the {\tt --no-show-diff}, unless the semantic patch contains {\tt *}s (a semantic match rather than a semantic patch). -The remainder of the code above uses a hash table to ensure that the same -information is not enqueued more than once. Coccinelle itself provides no -support for this. +When using python scripting a tuple is used throughout the rest of the +code to ensure that the same information is not enqueued more than once. +When using ocaml scripting a hash table is used for the same purposes. +Coccinelle itself provides no support for this and as such addressing +this with scripting is necessary. %%% Local Variables: %%% mode: LaTeX -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 0/3] coccinelle: expand docs
This v2 modifies the ++ demo and documentation to something a bit more safe for its use, and while at it decided to extend the iteration documentation and document support for .cocciconfig. Luis R. Rodriguez (3): docs/demos: add a few ++ documentation and demos docs/manual/cocci_syntax.tex: extend with python iteration docs: document .cocciconfig demos/plusplus1.c| 4 + demos/plusplus1.cocci| 17 + demos/plusplus1.res | 5 ++ demos/plusplus2.c| 7 ++ demos/plusplus2.cocci| 26 +++ demos/plusplus2.res | 10 +++ docs/manual/cocci_syntax.tex | 169 --- docs/spatch.1.in | 32 +++- 8 files changed, 258 insertions(+), 12 deletions(-) create mode 100644 demos/plusplus1.c create mode 100644 demos/plusplus1.cocci create mode 100644 demos/plusplus1.res create mode 100644 demos/plusplus2.c create mode 100644 demos/plusplus2.cocci create mode 100644 demos/plusplus2.res -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 3/3] docs: document .cocciconfig
Add documentation and example to manual and man page. Signed-off-by: Luis R. Rodriguez --- docs/manual/cocci_syntax.tex | 23 +++ docs/spatch.1.in | 32 +++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/docs/manual/cocci_syntax.tex b/docs/manual/cocci_syntax.tex index 5adfc95b0ce1..dc7aa17712af 100644 --- a/docs/manual/cocci_syntax.tex +++ b/docs/manual/cocci_syntax.tex @@ -1782,6 +1782,29 @@ When using ocaml scripting a hash table is used for the same purposes. Coccinelle itself provides no support for this and as such addressing this with scripting is necessary. +\section{.cocciconfig support} + +Coccinelle supports enabling custom options to be preferred when running +spatch. This is supported through the search of .cocciconfig files in each of +the following directories, later lines extend and may override earlier ones: + +\begin{itemize} + \item Your current user's home directory is processed first + \item Your directory from which spatch is called is processed next + \item The directory provided with the --dir option is processed last, if used +\end{itemize} + +Newlines, even with \, are not tolerated in attribute values. An example +follows: + +\begin{quote} +\begin{verbatim} +[spatch] + options = --jobs 4 + options = --show-trying +\end{verbatim} +\end{quote} + %%% Local Variables: %%% mode: LaTeX %%% TeX-master: "main_grammar" diff --git a/docs/spatch.1.in b/docs/spatch.1.in index dae38952e88c..121048b1ea0b 100644 --- a/docs/spatch.1.in +++ b/docs/spatch.1.in @@ -55,7 +55,8 @@ and at the \fBscripts/coccinelle\fP directory of the Linux Kernel source code. .SH OPTIONS -Here is a summary of the most commonly used options: +Here is a summary of the most commonly used options (also +see the "Configuration Mechanism" section below): .TP .B \-\-sp\-file \fI\fP @@ -566,6 +567,31 @@ It's for the other (internal) uses of the spatch program. \fB\-\-compare\-c\fR +.SH CONFIGURATION MECHANISM + +Coccinelle uses a simple text format to store customizations into +\.cocciconfig files, that can be per project, user, or target. The order +in which the configuration file is processed is as follows, the +later lines always extend and may override earlier ones: + + o Your current user's home directory is processed first + o Your directory from which spatch is called is processed next + o The directory provided with the --dir option is processed last, if used + +Such a configuration file may look like this: + +.if n \{\ +.RS 4 +.\} +.nf +[spatch] + options = --jobs 4 + options = --show-trying +.fi +.if n \{\ +.RE +.\} + .SH FILES .I @SHAREDIR@/standard.iso .RS @@ -574,6 +600,10 @@ This file contains the default set of isomorphisms. .I @SHAREDIR@/standard.h .RS This file contains the default set of macro hints. +.RE +.I .cocciconfig +.RS +This file contains the custom set of spatch options. .SH ENVIRONMENT .IP COCCINELLE_HOME -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 4/8] scripts: add glimpse.sh for indexing the kernel
On Sat, Jun 18, 2016 at 07:51:55AM +0200, Julia Lawall wrote: > > > On Sat, 18 Jun 2016, Luis R. Rodriguez wrote: > > > On Fri, Jun 17, 2016 at 05:35:26PM +0200, Julia Lawall wrote: > > > On Fri, 17 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > On Fri, Jun 17, 2016 at 11:44:26AM +0200, Julia Lawall wrote: > > > > > I'm not sure that this is worth it. It adds a dependency on a tool > > > > > that > > > > > seems not to be well maintained. In terms of Coccinelle, I'm not sure > > > > > that it gives a big benefit. > > > > > > > > > > Attached is a graph showing the file selection time for Coccinelle > > > > > for a > > > > > selection of fairly complex semantic patches. Coccigrep is just a > > > > > line-by-line regexp search implemented in ocaml, gitgrep uses git > > > > > grep. > > > > > In most cases, glimpse is clearly faster. > > > > > > > > > > On the other hand, it seems that glimpse often selects more files. > > > > > Sometimes a few more, eg 16 vs 14, and sometimes quite a lot more, eg > > > > > 538 > > > > > vs 236. I suspect that this is because glimpse considers _ to be a > > > > > space, > > > > > and thus it can have many false positives. There are, however, a few > > > > > cases where glimpse also selects fewer files. > > > > > > > > > > The file processing time (ie parsing the file, searching for, matches > > > > > of > > > > > the semantic patch in the file, and performing the transformation) is > > > > > normally much higher than the file selection time. > > > > > > > > > > So it seems that git grep is currently a better option for the kernel. > > > > > > > > Great, thanks, consider this patch dropped, do we want the heuristics > > > > for the cache index in place though or should I drop that as well ? > > > > > > I assume you mean this patch: > > > > > > [PATCH v2 3/8] coccicheck: add indexing enhancement options > > > > > > I think it should be dropped. It adds complexity and git grep works > > > pretty well. > > > > Hmm but coccicheck does not make use of --git-grep even. > > > > > If people want to use something else, they can use SPARGS, > > > or a .cocciconfig file, eg: > > > > > > [spatch] > > > options = --use-glimpse > > > > Neat will these be used last and thus override anything? > > Good point. If it is in the home directory, it is overrided by > everything. So make coccicheck shouldn't have an option related to this > issue. Great. > > If so, what > > about just adding an upstream .cocciconfig with --use-gitgrep -- only > > issue then is what if a user wants to use idutils ? How do we let them > > override? > > If we have an upstream .cocciconfig with --use-gitgrep, then the user can > specify an SPARGS with --use-idutils and override. I take it you meant SPFLAGS. I just read the order rules, I'll past them for completeness: -- from coccinelle/read_options.ml: .cocciconfig files can be placed in the user's home directory, the directory from which spatch is called, and the directory provided with the --dir option. The .cocciconfig file in the user's home directory is processed first, the .cocciconfig file in the directory from which spatch is called is processed next, and the .cocciconfig file in the directory provided with the --dir option is processed last. In each case, the read options extend/override the previously read ones. In all cases, the user can extend/override the options found in the .cocciconfig files on the command line. --- So order is: 0. $HOME/.cocciconfig 1. $PWD/.cocciconfig 2. --dir .cocciconfig So indeed an upstream .cocciconfig would seem to work well. Drivers can also have their own .cocciconfig if they would need it, but I cannot see this being needed at this time though, but good to know and keep in mind. In the future this fact might be a bit more useful if we added support for instance of a rule namespace, then for instance if we know a tweak is only needed for one driver we might for instance have something like: [spatch rule=scripts/coccinelle/api/d_find_alias.cocci] options = --opt1 --opt2 ... But for now I think more than good with an upstream linux/.cocciconfig then and SPFLAGs. I will have to do just one small adjustment to SPFLAGS on coccicheck to ensure it does go at the end. I'll address that in the re-spin of this series. > If we are making an upstream .cocciconfig, I would put a timeout in it > too. In my experience, 120 (seconds) is fine. Maybe 200 to give a little > more margin. Again, this can be overridden on the command line. OK will use 200. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 4/8] scripts: add glimpse.sh for indexing the kernel
On Fri, Jun 17, 2016 at 05:35:26PM +0200, Julia Lawall wrote: > On Fri, 17 Jun 2016, Luis R. Rodriguez wrote: > > > On Fri, Jun 17, 2016 at 11:44:26AM +0200, Julia Lawall wrote: > > > I'm not sure that this is worth it. It adds a dependency on a tool that > > > seems not to be well maintained. In terms of Coccinelle, I'm not sure > > > that it gives a big benefit. > > > > > > Attached is a graph showing the file selection time for Coccinelle for a > > > selection of fairly complex semantic patches. Coccigrep is just a > > > line-by-line regexp search implemented in ocaml, gitgrep uses git grep. > > > In most cases, glimpse is clearly faster. > > > > > > On the other hand, it seems that glimpse often selects more files. > > > Sometimes a few more, eg 16 vs 14, and sometimes quite a lot more, eg 538 > > > vs 236. I suspect that this is because glimpse considers _ to be a space, > > > and thus it can have many false positives. There are, however, a few > > > cases where glimpse also selects fewer files. > > > > > > The file processing time (ie parsing the file, searching for, matches of > > > the semantic patch in the file, and performing the transformation) is > > > normally much higher than the file selection time. > > > > > > So it seems that git grep is currently a better option for the kernel. > > > > Great, thanks, consider this patch dropped, do we want the heuristics > > for the cache index in place though or should I drop that as well ? > > I assume you mean this patch: > > [PATCH v2 3/8] coccicheck: add indexing enhancement options > > I think it should be dropped. It adds complexity and git grep works > pretty well. Hmm but coccicheck does not make use of --git-grep even. > If people want to use something else, they can use SPARGS, > or a .cocciconfig file, eg: > > [spatch] > options = --use-glimpse Neat will these be used last and thus override anything? If so, what about just adding an upstream .cocciconfig with --use-gitgrep -- only issue then is what if a user wants to use idutils ? How do we let them override? Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] bundles/pycaml/: force pycaml preparation to be done serially
On Fri, Jun 17, 2016 at 08:47:24PM +0200, Luis R. Rodriguez wrote: > On Fri, Jun 17, 2016 at 08:10:14PM +0200, Julia Lawall wrote: > > > > > > On Fri, 17 Jun 2016, Luis R. Rodriguez wrote: > > > > > This fixes parallel compilation when using 32 threads on a 32-core > > > system: > > > > > > The error: > > > > > > make[6]: Entering directory '/home/mcgrof/coccinelle/bundles/pycaml' > > > /usr/bin/ocamlopt.opt -I chemoelectric-pycaml-8614105 -cclib > > > -lpycaml_stubs -ccopt -I/usr/include/python2.7 -ccopt > > > -I/usr/include/x86_64-linux-gnu/python2.7 -cclib -lpython2.7 -c > > > chemoelectric-pycaml-8614105/pycaml.ml -o > > > chemoelectric-pycaml-8614105/pycaml.cmx > > > make[6]: *** No rule to make target > > > 'chemoelectric-pycaml-8614105/pycaml.a', needed by > > > 'all-opt-build-targets'. Stop. > > > make[6]: *** Waiting for unfinished jobs > > > make[6]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' > > > ../Makefile.bundles:33: recipe for target 'all.opt' failed > > > make[5]: *** [all.opt] Error 2 > > > make[5]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' > > > Makefile:186: recipe for target > > > '/home/mcgrof/coccinelle/bundles/pycaml/.opt' failed > > > make[4]: *** [/home/mcgrof/coccinelle/bundles/pycaml/.opt] Error 2 > > > make[4]: Leaving directory '/home/mcgrof/coccinelle' > > > Makefile:180: recipe for target 'subdirs.opt' failed > > > make[3]: *** [subdirs.opt] Error 1 > > > make[3]: Leaving directory '/home/mcgrof/coccinelle' > > > Makefile:163: recipe for target 'opt-compil' failed > > > make[2]: *** [opt-compil] Error 2 > > > make[2]: Leaving directory '/home/mcgrof/coccinelle' > > > Makefile:143: recipe for target 'all-release' failed > > > make[1]: *** [all-release] Error 2 > > > make[1]: Leaving directory '/home/mcgrof/coccinelle' > > > Makefile:121: recipe for target 'all' failed > > > make: *** [all] Error 2 > > > > > > Signed-off-by: Luis R. Rodriguez > > > --- > > > bundles/pycaml/Makefile | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/bundles/pycaml/Makefile b/bundles/pycaml/Makefile > > > index 5045c27..4a619f4 100644 > > > --- a/bundles/pycaml/Makefile > > > +++ b/bundles/pycaml/Makefile > > > @@ -17,6 +17,8 @@ export PYMAJOR=$(shell echo ${PYTHON_VERSION} | sed -e > > > 's/\..*//') > > > > > > include ../Makefile.bundles > > > > > > +.NOTPARALLEL: .prepare > > > + > > > .prepare: $(MARKER) > > > rm -f "$(SRC_DIR)/pycaml.mli" > > > $(CC) -E -w -D PYMAJOR$(PYMAJOR) -xc pycaml.pp.ml >$(SRC_DIR)/pycaml.ml > > > > Thanks for the patch. I'm not sure that it will address all possible > > issues, > > Agreed. > > > but if it is an improvement, then ok. > > It makes parallel building work with a minimal change. The bigger > hammer is to not trust any bundles and add +.NOTPARALLEL: all to > the top level bundles/Makefile, that was my first approach, however > it didn't seem like a good idea either as then you'd slow the bundle > build generally. I actually thing we also need .ONESHELL on all bundle Makefiles as well. This alone does not suffice to make it build correctly, however it seems this is correct. So the following patch seems more appropriate. Let me know. >From 850cc1b16132d4984d1f4c61eea36472571db44b Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Fri, 17 Jun 2016 17:08:40 + Subject: [PATCH] bundles/pycaml/: use .ONESHELL and .NOTPARALLEL on bundles Bundles need their preparation to run in one shell [0], for pycaml we also need to disable parallel builds on the preparation. The .ONESHELL can be placed anywhere in the file. [0] https://www.gnu.org/software/make/manual/make.html#Execution This fixes parallel compilation when using 32 threads on a 32-core system: The error: make[6]: Entering directory '/home/mcgrof/coccinelle/bundles/pycaml' /usr/bin/ocamlopt.opt -I chemoelectric-pycaml-8614105 -cclib -lpycaml_stubs -ccopt -I/usr/include/python2.7 -ccopt -I/usr/include/x86_64-linux-gnu/python2.7 -cclib -lpython2.7 -c chemoelectric-pycaml-8614105/pycaml.ml -o chemoelectric-pycaml-8614105/pycaml.cmx make[6]: *** No rule to make target 'chemoelectric-pycaml-8614105/pycaml.a', needed by 'all-opt-build-targets
Re: [Cocci] [PATCH] bundles/pycaml/: force pycaml preparation to be done serially
On Fri, Jun 17, 2016 at 08:10:14PM +0200, Julia Lawall wrote: > > > On Fri, 17 Jun 2016, Luis R. Rodriguez wrote: > > > This fixes parallel compilation when using 32 threads on a 32-core > > system: > > > > The error: > > > > make[6]: Entering directory '/home/mcgrof/coccinelle/bundles/pycaml' > > /usr/bin/ocamlopt.opt -I chemoelectric-pycaml-8614105 -cclib > > -lpycaml_stubs -ccopt -I/usr/include/python2.7 -ccopt > > -I/usr/include/x86_64-linux-gnu/python2.7 -cclib -lpython2.7 -c > > chemoelectric-pycaml-8614105/pycaml.ml -o > > chemoelectric-pycaml-8614105/pycaml.cmx > > make[6]: *** No rule to make target > > 'chemoelectric-pycaml-8614105/pycaml.a', needed by > > 'all-opt-build-targets'. Stop. > > make[6]: *** Waiting for unfinished jobs > > make[6]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' > > ../Makefile.bundles:33: recipe for target 'all.opt' failed > > make[5]: *** [all.opt] Error 2 > > make[5]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' > > Makefile:186: recipe for target > > '/home/mcgrof/coccinelle/bundles/pycaml/.opt' failed > > make[4]: *** [/home/mcgrof/coccinelle/bundles/pycaml/.opt] Error 2 > > make[4]: Leaving directory '/home/mcgrof/coccinelle' > > Makefile:180: recipe for target 'subdirs.opt' failed > > make[3]: *** [subdirs.opt] Error 1 > > make[3]: Leaving directory '/home/mcgrof/coccinelle' > > Makefile:163: recipe for target 'opt-compil' failed > > make[2]: *** [opt-compil] Error 2 > > make[2]: Leaving directory '/home/mcgrof/coccinelle' > > Makefile:143: recipe for target 'all-release' failed > > make[1]: *** [all-release] Error 2 > > make[1]: Leaving directory '/home/mcgrof/coccinelle' > > Makefile:121: recipe for target 'all' failed > > make: *** [all] Error 2 > > > > Signed-off-by: Luis R. Rodriguez > > --- > > bundles/pycaml/Makefile | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/bundles/pycaml/Makefile b/bundles/pycaml/Makefile > > index 5045c27..4a619f4 100644 > > --- a/bundles/pycaml/Makefile > > +++ b/bundles/pycaml/Makefile > > @@ -17,6 +17,8 @@ export PYMAJOR=$(shell echo ${PYTHON_VERSION} | sed -e > > 's/\..*//') > > > > include ../Makefile.bundles > > > > +.NOTPARALLEL: .prepare > > + > > .prepare: $(MARKER) > > rm -f "$(SRC_DIR)/pycaml.mli" > > $(CC) -E -w -D PYMAJOR$(PYMAJOR) -xc pycaml.pp.ml >$(SRC_DIR)/pycaml.ml > > Thanks for the patch. I'm not sure that it will address all possible > issues, Agreed. > but if it is an improvement, then ok. It makes parallel building work with a minimal change. The bigger hammer is to not trust any bundles and add +.NOTPARALLEL: all to the top level bundles/Makefile, that was my first approach, however it didn't seem like a good idea either as then you'd slow the bundle build generally. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH] bundles/pycaml/: force pycaml preparation to be done serially
This fixes parallel compilation when using 32 threads on a 32-core system: The error: make[6]: Entering directory '/home/mcgrof/coccinelle/bundles/pycaml' /usr/bin/ocamlopt.opt -I chemoelectric-pycaml-8614105 -cclib -lpycaml_stubs -ccopt -I/usr/include/python2.7 -ccopt -I/usr/include/x86_64-linux-gnu/python2.7 -cclib -lpython2.7 -c chemoelectric-pycaml-8614105/pycaml.ml -o chemoelectric-pycaml-8614105/pycaml.cmx make[6]: *** No rule to make target 'chemoelectric-pycaml-8614105/pycaml.a', needed by 'all-opt-build-targets'. Stop. make[6]: *** Waiting for unfinished jobs make[6]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' ../Makefile.bundles:33: recipe for target 'all.opt' failed make[5]: *** [all.opt] Error 2 make[5]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' Makefile:186: recipe for target '/home/mcgrof/coccinelle/bundles/pycaml/.opt' failed make[4]: *** [/home/mcgrof/coccinelle/bundles/pycaml/.opt] Error 2 make[4]: Leaving directory '/home/mcgrof/coccinelle' Makefile:180: recipe for target 'subdirs.opt' failed make[3]: *** [subdirs.opt] Error 1 make[3]: Leaving directory '/home/mcgrof/coccinelle' Makefile:163: recipe for target 'opt-compil' failed make[2]: *** [opt-compil] Error 2 make[2]: Leaving directory '/home/mcgrof/coccinelle' Makefile:143: recipe for target 'all-release' failed make[1]: *** [all-release] Error 2 make[1]: Leaving directory '/home/mcgrof/coccinelle' Makefile:121: recipe for target 'all' failed make: *** [all] Error 2 Signed-off-by: Luis R. Rodriguez --- bundles/pycaml/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bundles/pycaml/Makefile b/bundles/pycaml/Makefile index 5045c27..4a619f4 100644 --- a/bundles/pycaml/Makefile +++ b/bundles/pycaml/Makefile @@ -17,6 +17,8 @@ export PYMAJOR=$(shell echo ${PYTHON_VERSION} | sed -e 's/\..*//') include ../Makefile.bundles +.NOTPARALLEL: .prepare + .prepare: $(MARKER) rm -f "$(SRC_DIR)/pycaml.mli" $(CC) -E -w -D PYMAJOR$(PYMAJOR) -xc pycaml.pp.ml >$(SRC_DIR)/pycaml.ml -- 2.8.1 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Parallel building on high number of threads failing
On Fri, Jun 17, 2016 at 05:28:58PM +0200, Julia Lawall wrote: > On Fri, 17 Jun 2016, Luis R. Rodriguez wrote: > > > Building coccinelle on a 4 core system using 4 threads works well, on > > a 32-core system however it fails. Building serially works well on the > > system. After autogen, configure, and then make -j 32 I get: > > Perhaps this can be looked into, but I don't think there is a strong goa > of making this work. I have enough interest to fix it myself so I've done that. I'll send the patch. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] scripts: add glimpse.sh for indexing the kernel
On Fri, Jun 17, 2016 at 06:05:38PM +0200, SF Markus Elfring wrote: > > To me what we can best do is see what we can take out of agrep, > > and port it to proper GNU grep, then see if git can also enable > > complex queries as Julia notes. > > How do you think about to improve such tools together with computation > tree logic? ;-) Patches are welcomed, I'm sure. Please send patches to git or GNU grep on their respective upstream list. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 4/8] scripts: add glimpse.sh for indexing the kernel
On Fri, Jun 17, 2016 at 11:44:26AM +0200, Julia Lawall wrote: > I'm not sure that this is worth it. It adds a dependency on a tool that > seems not to be well maintained. In terms of Coccinelle, I'm not sure > that it gives a big benefit. > > Attached is a graph showing the file selection time for Coccinelle for a > selection of fairly complex semantic patches. Coccigrep is just a > line-by-line regexp search implemented in ocaml, gitgrep uses git grep. > In most cases, glimpse is clearly faster. > > On the other hand, it seems that glimpse often selects more files. > Sometimes a few more, eg 16 vs 14, and sometimes quite a lot more, eg 538 > vs 236. I suspect that this is because glimpse considers _ to be a space, > and thus it can have many false positives. There are, however, a few > cases where glimpse also selects fewer files. > > The file processing time (ie parsing the file, searching for, matches of > the semantic patch in the file, and performing the transformation) is > normally much higher than the file selection time. > > So it seems that git grep is currently a better option for the kernel. Great, thanks, consider this patch dropped, do we want the heuristics for the cache index in place though or should I drop that as well ? Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2 3/8] coccicheck: add indexing enhancement options
On Fri, Jun 17, 2016 at 11:47:25AM +0200, Julia Lawall wrote: > > > On Thu, 16 Jun 2016, Luis R. Rodriguez wrote: > > > Coccinelle has support to make use of its own enhanced "grep" > > mechanisms instead of using regular grep for searching code, > > it calls this 'coccigrep'. In lack of any indexing optimization > > information it uses --use-coccigrep by default. > > > > This patch enable indexing optimizations heuristics so that coccigrep > > can automatically detect what indexing options are available and use > > them accordinly without any user input. > > > > Since git has its own index, support for using 'git grep' has been > > added to Coccinelle, that should on average perform better than > > using the internal coccigrep. Coccinelle has had idutils support > > as well for a while now, you however need to refer to the index > > file. We support detecting two idutils index files by default, > > ID and .id-utils.index, assuming you ran either of: > > > > # What you might have done: > > mkid -s > > # as in coccinelle scripts/idutils_index.sh > > mkid -i C --output .id-utils.index * > > > > Lastly, Coccinelle has had support for glimpseindex for a long while, > > however the glimpseindex tool, the agrep library were previously closed > > source, its all now open sourced, and provides the best performance, so > > support that if we can detect you have a glimpse index. > > > > You can always override the index as follows: > > > > $ make coccicheck V=1 MODE=report COCCI_INDEX="--use-idutils ID" > > Why not just have a generic COCCI_ARGS argument? Actually SPFLAGS exists already so we can just document to override using that. Will fix. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Parallel building on high number of threads failing
Building coccinelle on a 4 core system using 4 threads works well, on a 32-core system however it fails. Building serially works well on the system. After autogen, configure, and then make -j 32 I get: make[6]: Entering directory '/home/mcgrof/coccinelle/bundles/pycaml' /usr/bin/ocamlopt.opt -I chemoelectric-pycaml-8614105 -cclib -lpycaml_stubs -ccopt -I/usr/include/python2.7 -ccopt -I/usr/include/x86_64-linux-gnu/python2.7 -cclib -lpython2.7 -c chemoelectric-pycaml-8614105/pycaml.ml -o chemoelectric-pycaml-8614105/pycaml.cmx make[6]: *** No rule to make target 'chemoelectric-pycaml-8614105/pycaml.a', needed by 'all-opt-build-targets'. Stop. make[6]: *** Waiting for unfinished jobs make[6]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' ../Makefile.bundles:33: recipe for target 'all.opt' failed make[5]: *** [all.opt] Error 2 make[5]: Leaving directory '/home/mcgrof/coccinelle/bundles/pycaml' Makefile:186: recipe for target '/home/mcgrof/coccinelle/bundles/pycaml/.opt' failed make[4]: *** [/home/mcgrof/coccinelle/bundles/pycaml/.opt] Error 2 make[4]: Leaving directory '/home/mcgrof/coccinelle' Makefile:180: recipe for target 'subdirs.opt' failed make[3]: *** [subdirs.opt] Error 1 make[3]: Leaving directory '/home/mcgrof/coccinelle' Makefile:163: recipe for target 'opt-compil' failed make[2]: *** [opt-compil] Error 2 make[2]: Leaving directory '/home/mcgrof/coccinelle' Makefile:143: recipe for target 'all-release' failed make[1]: *** [all-release] Error 2 make[1]: Leaving directory '/home/mcgrof/coccinelle' Makefile:121: recipe for target 'all' failed make: *** [all] Error 2 So it seems there's a race against chemoelectric-pycaml-8614105/pycaml.a Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 5/8] coccicheck: replace --very-quiet with --quit when debugging
When debugging (using --profile or --show-trying) you want to avoid supressing output, use --quiet instead. While at it, extend documentation for SPFLAGS use. For instance one can use: $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ make coccicheck DEBUG_FILE="poo.err" MODE=report SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c v2: use egrep instead of the *"=--option"* check, this doesn't work for disjunctions. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index 30f5a531ad34..f3a7d9fe9bc9 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -159,7 +159,25 @@ else NPROC="$J" fi -FLAGS="$SPFLAGS --very-quiet" +# You can use SPFLAGS to append extra arguments to coccicheck. +# A good example is if you want to debug your cocci script, you can +# for instance use the following: +# +# $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci +# $ time make coccicheck MODE=report DEBUG_FILE="all.err" SPFLAGS="--profile --show-trying" M=./drivers/mfd/arizona-irq.c +# +# "--show-trying" should show you what rule is being processed as it goes to +# stdout, you do not need a debug file for that. The profile output will be +# be sent to stdout, if you provide a DEBUG_FILE the profiling data can be +# inspected there. +# +# --profile will not output if --very-quiet is used, so avoid it. +echo $SPFLAGS | egrep -e "--profile|--show-trying" 2>&1 > /dev/null +if [ $? -eq 0 ]; then + FLAGS="--quiet $SPFLAGS" +else + FLAGS="--very-quiet $SPFLAGS" +fi # spatch only allows include directories with the syntax "-I include" # while gcc also allows "-Iinclude" and "-include include" -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 6/8] coccicheck: add support for requring a coccinelle version
Enable Coccinelle SmPL patches to require a specific version of Coccinelle. In the event that the version does not match we just inform the user, if the user asked to go through all SmPL patches we just inform them of the need for a new version of coccinelle for the SmPL patch and continue on with the rest. This uses the simple kernel scripts/ld-version.sh to create a weight on the version provided by spatch. The -dirty attribute is ignored if supplied, the benefit of scripts/ld-version.sh is it has a long history and well tested. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 13 + 1 file changed, 13 insertions(+) diff --git a/scripts/coccicheck b/scripts/coccicheck index f3a7d9fe9bc9..268ea375273b 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -13,6 +13,9 @@ if [ ! -x "$SPATCH" ]; then exit 1 fi +SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}') +SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh) + USE_JOBS="no" $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes" @@ -312,6 +315,16 @@ coccinelle () { COCCI="$1" OPT=`grep "Option" $COCCI | cut -d':' -f2` +REQ=`grep "Requires" $COCCI | cut -d':' -f2 | sed "s| ||"` +REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh) +if [ "$REQ_NUM" != "0" ] ; then + if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then + echo "Skipping coccinele SmPL patch: $COCCI" + echo "You have coccinelle: $SPATCH_VERSION" + echo "This SmPL patch requires: $REQ" + return + fi +fi # The option '--parse-cocci' can be used to syntactically check the SmPL files. # -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 1/8] coccicheck: move spatch binary check up
This has no functional changes. This is being done to enable us to later use spatch binary for some flag checking for certain features early on. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index dd85a455b2ba..aa5e78fba270 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -7,6 +7,11 @@ SPATCH="`which ${SPATCH:=spatch}`" +if [ ! -x "$SPATCH" ]; then +echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' +exit 1 +fi + trap kill_running SIGTERM SIGINT declare -a SPATCH_PID @@ -51,11 +56,6 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then OPTIONS="--patch $srctree $OPTIONS" fi -if [ ! -x "$SPATCH" ]; then -echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/' -exit 1 -fi - if [ "$MODE" = "" ] ; then if [ "$ONLINE" = "0" ] ; then echo 'You have not explicitly specified the mode to use. Using default "report" mode.' -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 8/8] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
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 --- 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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 2/8] coccicheck: enable parmap support
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. Signed-off-by: Luis R. Rodriguez 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 ... real29m14.912s user103m1.796s sys 0m4.464s After: real16m22.435s user128m30.060s sys 0m2.712s 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 --- scripts/coccicheck | 40 +--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index aa5e78fba270..7acef3efc258 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' @@ -82,7 +82,33 @@ if [ "$ONLINE" = "0" ] ; then echo '' fi -run_cmd() { +if [ "$USE_JOBS" = "no" ]; then + trap kill_running SIGTERM SIGINT + declare -a SPATCH_PID +else + OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1" +fi + +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 +123,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 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
Thou shalt not make firmware calls early on init or probe. systemd already ripped support out for the usermode helper a while ago, there are still users that require the usermode helper, however systemd's use of the usermode helper exacerbated a long lasting issue of the fact that we have many drivers that load firmware on init or probe. Independent of the usermode helper loading firmware on init or probe is a bad idea for a few reasons. When the firmware is read directly from the filesystem by the kernel, if the driver is built-in to the kernel the firmware may not yet be available, for such uses one could use initramfs and stuff the firmware inside, or one also use CONFIG_EXTRA_FIRMWARE; however not all distributions use this, as such generally one cannot count on this. There is another corner cases to consider, since we are accessing the firmware directly folks cannot expect new found firmware on a filesystem after we switch off from an initramfs with pivot_root(). Supporting such situations is possible today but fixing it for good is really complex due to the large amount of variablity in the boot up process. Instead just document the expectations properly and add a grammar rule to enable folks to check / validate / police if drivers are using the request firmware API early on init or probe. The SmPL rule used to check for the probe routine is loose and is currently defined through a regexp, that can easily be extended to any other known bus probe routine names. It also uses the new Python iteration support which allows us to backtrack from a request_firmware*() call back to a possible probe or init, iteratively. Without iteration we would only be able to get reports for callers who directly use the request_firmware*() API on the initial probe or init routine. There are 4 offenders at this time: mcgrof@ergon ~/linux-next (git::20160609)$ export COCCI=scripts/coccinelle/api/request_firmware.cocci mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its init routine on line 321. drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on its probe routine on line 136. drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its probe routine on line 796. drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on its probe routine on line 1246. I checked and verified all these are valid reports. This list also matches the same list as in 20150805, so we have fortunately not gotten worse. Let's keep it that way and also fix these drivers. v2: changes from v1 [0]: o This now supports iteration, this extends our coverage on the report o Update documentation and commit log to accept the fate of not being able to remove the usermode helper. [0] https://lkml.kernel.org/r/1440811107-861-1-git-send-email-mcg...@do-not-panic.com Cc: Alessandro Rubini Cc: Kevin Cernekee Cc: Daniel Vetter Cc: Mimi Zohar Cc: David Woodhouse Cc: Kees Cook Cc: Dmitry Torokhov Cc: Ming Lei Cc: Jonathan Corbet Cc: Julia Lawall Cc: Gilles Muller Cc: Nicolas Palix Cc: Thierry Martinez Cc: Michal Marek Cc: cocci@systeme.lip6.fr Cc: Alessandro Rubini Cc: Kevin Cernekee Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: linux-ser...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux-ser...@vger.kernel.org Cc: linuxppc-...@lists.ozlabs.org Signed-off-by: Luis R. Rodriguez --- Documentation/firmware_class/README| 20 drivers/base/Kconfig | 2 +- .../request_firmware-avoid-init-probe-init.cocci | 130 + 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci diff --git a/Documentation/firmware_class/README b/Documentation/firmware_class/README index cafdca8b3b15..056d1cb9d365 100644 --- a/Documentation/firmware_class/README +++ b/Documentation/firmware_class/README @@ -93,6 +93,26 @@ user contexts to request firmware asynchronously, but can't be called in atomic contexts. +Requirements: += + +You should avoid at all costs requesting firmware on both init and probe paths +of your device driver. Reason for this is the complexity needed to ensure a +firmware will be available for a driver early in boot through different +build configurations. Consider built-in drivers needing firmware early, or +consider a driver assuming it will only get firmware after pivot_root(). + +Drivers that really need firmware early should use stuff the firmware in +initramfs or consider using CONFIG_EXTRA_FIRMWARE. Using initramfs is much +more portable to more distributions as not all distributions wish to enable +CONFIG_EXTRA_FIRMWARE. Should a driver require the firmware being built-in +it should depend on CONFIG_EXTRA_FIRMWARE. There is no current annotation for +requiring a firmware on initramfs.
[Cocci] [PATCH v2 0/8] coccicheck: modernize
This series gives coccicheck some love by modernizing it to catch up to some features implemented in Coccinelle, of most importance is parmap support and some indexing enhancements. This should help speed things up considerably, for me this now runs twice as fast in some situations. This also takes advantage of the open sourcing of Glimpse, thanks to the university of Arizona for doing and to Udi Manber for helping push that out. Since Coccinelle is ever evolving this also now allows developer to require specific Coccinelle versions on top of the SmPL patches. This is part of a larger series, all of which you can find here: https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160616-sysdata-v2 Since further development is done on top of this we may need to coordinate with another maintainer for how these go in. The coccicheck changes will be important to help speed up some of the new Coccinelle API checks and transformations implemented in later series on the request firmware API. Luis R. Rodriguez (8): coccicheck: move spatch binary check up coccicheck: enable parmap support coccicheck: add indexing enhancement options scripts: add glimpse.sh for indexing the kernel coccicheck: replace --very-quiet with --quit when debugging 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 .gitignore | 3 + scripts/coccicheck | 242 - .../iterators/device_node_continue.cocci | 3 + scripts/glimpse.sh | 11 + 4 files changed, 248 insertions(+), 11 deletions(-) create mode 100755 scripts/glimpse.sh -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 3/8] coccicheck: add indexing enhancement options
Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code, it calls this 'coccigrep'. In lack of any indexing optimization information it uses --use-coccigrep by default. This patch enable indexing optimizations heuristics so that coccigrep can automatically detect what indexing options are available and use them accordinly without any user input. Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal coccigrep. Coccinelle has had idutils support as well for a while now, you however need to refer to the index file. We support detecting two idutils index files by default, ID and .id-utils.index, assuming you ran either of: # What you might have done: mkid -s # as in coccinelle scripts/idutils_index.sh mkid -i C --output .id-utils.index * Lastly, Coccinelle has had support for glimpseindex for a long while, however the glimpseindex tool, the agrep library were previously closed source, its all now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index. You can always override the index as follows: $ make coccicheck V=1 MODE=report COCCI_INDEX="--use-idutils ID" These tests have been run on an 8 core system: Before: $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ time make coccicheck MODE=report coccigrep (default and without this patch): real0m16.369s user0m58.712s sys 0m5.064s After: $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ time make coccicheck MODE=report With glimpse: real0m6.549s user0m49.136s sys 0m3.076s With idutils: real0m6.749s user0m51.936s sys 0m3.876s With gitgrep: real0m6.805s user0m51.572s sys 0m4.432s v2 changes: o simplify DIR assignment to 1 line o detected a bug when KBUILD_EXTMOD is used other than the parent directory for both glimpse and idutils, so we avoid both when M=path/driver/ is used. This is being looked into upstream on Coccinelle. o move indexing heuristics to a file o document logic used for indexing o add idutils support, supports two indexing files o remove coccigrep heuristics as its the default anyway o add COCCI_INDEX to enable overriding heuristics, you can use this as follows, for example: o replace references to stderr file with DEBUG_FILE use instructions $ export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci $ make coccicheck V=1 MODE=report COCCI_INDEX="--use-coccigrep" $ make coccicheck V=1 MODE=report COCCI_INDEX="--use-idutils ID" $ make coccicheck V=1 MODE=report COCCI_INDEX="--use-glimpse" $ make coccicheck V=1 MODE=report COCCI_INDEX="--use-gitgrep" Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 150 + 1 file changed, 150 insertions(+) diff --git a/scripts/coccicheck b/scripts/coccicheck index 7acef3efc258..30f5a531ad34 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -5,6 +5,7 @@ # version 1.0.0-rc11. # +DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" if [ ! -x "$SPATCH" ]; then @@ -15,6 +16,134 @@ fi USE_JOBS="no" $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes" +function can_use_glimpse() +{ + $SPATCH --help | grep "\-\-use\-glimpse" > /dev/null + if [ $? -ne 0 ]; then + echo "no" + return + fi + if [ ! -f $DIR/.glimpse_index ]; then + echo "no" + return + fi + + # As of coccinelle 1.0.5 --use-glimpse cannot be used with M= other + # than the base directory. We expect a future release will let us + # specify the full path of the glimpse index but even if that's + # supported, glimpse use seems to require an index per every + # directory parsed, so you'd have to generate a glimpse index + # per directory. If M=path is used (where epath is not the top level) + # we'll have to fallback to alternatives then. + if [ "$KBUILD_EXTMOD" != "./" -a "$KBUILD_EXTMOD" != "" ]; then + echo "no" + return + fi + echo yes +} + +function can_use_idutils() +{ + $SPATCH --help | grep "\-\-use\-idutils" > /dev/null + if [ $? -ne 0 ]; then + echo "no" + return + fi + # As of coccinelle 1.0.5 --use-idutils will bust if one uses + # idutils with an index out of the main tree. + if [ "$KBUILD_EXTMOD" != "./" -a "$KBUILD_EXTMOD" != "" ]; then + echo "no" +
[Cocci] [PATCH v2 7/8] coccicheck: refer to coccicheck bottest wiki for documentation
Sprinkling *tons* of documentation on the script is not a good idea, instead refer to a wiki for further coccicheck documentation: https://bottest.wiki.kernel.org/coccicheck This page shall always refer to the linux-next iteration of scripts/coccicheck. Signed-off-by: Luis R. Rodriguez --- scripts/coccicheck | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/coccicheck b/scripts/coccicheck index 268ea375273b..46aeaad2847e 100755 --- a/scripts/coccicheck +++ b/scripts/coccicheck @@ -1,9 +1,14 @@ #!/bin/bash - +# Linux kernel coccicheck +# +# For more detailed documentation refer to: +# +# https://bottest.wiki.kernel.org/coccicheck +# +# This documentation always refers to the linux-next version of the script. # # This script requires at least spatch # version 1.0.0-rc11. -# DIR="$(dirname $(readlink -f $0))/.." SPATCH="`which ${SPATCH:=spatch}`" -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2 4/8] scripts: add glimpse.sh for indexing the kernel
Glimpse is a tool you can use to index the kernel. The tool was open sourced on 2014-09-26 [0] under the ISC license however the original release still [1] does not compile. A fix for this and a release that does compile is provided in a temporary tree [2]. v2 changesl: o simplify DIR in one line and add verbose release information to commit log about the open sourcing of glimpse and its current status. o add index files to .gitignore [0] http://webglimpse.net/ [1] https://github.com/gvelez17/glimpse/ [2] https://github.com/mcgrof/glimpse.git Signed-off-by: Luis R. Rodriguez --- .gitignore | 3 +++ scripts/glimpse.sh | 11 +++ 2 files changed, 14 insertions(+) create mode 100755 scripts/glimpse.sh diff --git a/.gitignore b/.gitignore index 2be25f771bd8..e6b40a6f3a7a 100644 --- a/.gitignore +++ b/.gitignore @@ -113,3 +113,6 @@ all.config # Kdevelop4 *.kdev4 + +# Glimpse +.glimpse_* diff --git a/scripts/glimpse.sh b/scripts/glimpse.sh new file mode 100755 index ..2e866228a88f --- /dev/null +++ b/scripts/glimpse.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +DIR="$(dirname $(readlink -f $0))/.." + +GLIMPSEINDEX="`which ${GLIMPSEINDEX:=glimpseindex}`" +if [ ! -x "$GLIMPSEINDEX" ]; then + echo 'glimpseindex can be obtained at https://github.com/mcgrof/glimpse.git' + exit 1 +fi + +find $DIR/* -name "*.[ch]" | $GLIMPSEINDEX -o -H . -F -- 2.8.2 ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [RFC] firmware: annotate thou shalt not request fw on init or probe
On Thu, Sep 03, 2015 at 07:56:33AM +0200, Julia Lawall wrote: > > > On Thu, 3 Sep 2015, Luis R. Rodriguez wrote: > > > On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote: > > > > +@ defines_module_init exists @ > > > > +declarer name module_init; > > > > +identifier init; > > > > +@@ > > > > + > > > > +module_init(init); > > > > + > > > > +@ has_probe depends on defines_module_init @ > > > > +identifier drv_calls, drv_probe; > > > > +type bus_driver; > > > > +identifier probe_op =~ "(probe)"; > > > > +@@ > > > > + > > > > +bus_driver drv_calls = { > > > > + .probe_op = drv_probe, > > > > +}; > > > > > > I'm not sure that this is enough. For example, there is the macro > > > platform_driver_probe that initializes probe fields. There is likewise > > > module_platform_driver, which is a top-level declaration that encapsulates > > > the module_init and the definition of the module_init function, which in > > > turn calls platform_driver_probe. There is also module_platform_driver, > > > which encapsulates the module_init, but not the initialization of the > > > probe > > > field. Are you concerned with any of these cases? > > > > Yes, and also it would seem this would only capture simple one level of > > routine indirection, for instance if probe called bar() and it was within > > bar() that the driver code called a fw request call, that would not be > > picked > > up, correct? > > By default, Coccinelle is not interprocedural. You can encode that in the > script, though. > > Probably the most convenient approach would be to start with the the call, > and then work backward to the entry point. I have code to do this, if and > when it turns out to be useful. FYI folks, thanks to Coccinelle 1.0.5 this is now easily possible with Python integration, a follow up patch will be submitted that uses this mechanism to do a proper full search on the kernel. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 2/4] scripts: add reqs python library
On Wed, Jun 15, 2016 at 09:11:45PM +0200, Michal Marek wrote: > Dne 15.6.2016 v 18:02 Luis R. Rodriguez napsal(a): > > On Wed, Jun 15, 2016 at 09:50:11AM +0200, Michal Marek wrote: > >> On 2016-06-15 00:10, Luis R. Rodriguez wrote: > >>> +weight = (int(rel_specs['VERSION'])<< 32) + \ > >>> + (int(rel_specs['PATCHLEVEL']) << 16) + \ > >>> + (sublevel << 8 ) + \ > >>> + (extra * 60) + (relmod * 2) > >> > >> This is going to silently break as soon as we have a version number with > >> e.g. a time stamp embedded. > > > > Well this is adhering to a linux_version_cmp type, surely we can adjust > > it with alternatives. It just happens that with the common stuff this > > suffices. > > > > Do you have a specific string in mind I can use to test against? > > You can have a look at the git history of scripts/ld-version.sh. Will use that instead. The SmPL patch can then have: // Requires: 1.0.5 This ignores the -dirty stuff. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 4/6] coccicheck: add indexing enhancement options
On Wed, Jun 15, 2016 at 01:20:19PM +0200, SF Markus Elfring wrote: > > +DIR=$(dirname $(readlink -f $0)) > > +DIR="${DIR}/../" > > How do you think about to merge these variable assignments also into one? > > +DIR="$(dirname $(readlink -f $0))/../" Sure. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 4/4] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
On Wed, Jun 15, 2016 at 06:52:27PM +0200, Julia Lawall wrote: > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > > On Wed, Jun 15, 2016 at 06:11:57PM +0200, Julia Lawall wrote: > > > > > > > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > On Wed, Jun 15, 2016 at 05:55:34PM +0200, Julia Lawall wrote: > > > > > > > > > > > > > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > > > > > On Wed, Jun 15, 2016 at 10:43:30AM +0200, Julia Lawall wrote: > > > > > > > How about the following, since Coccinelle knows what its version > > > > > > > is? > > > > > > > This could of course be implemented in python as well. > > > > > > > > > > > > > > julia > > > > > > > > > > > > > > diff --git a/docs/Coccilib.3cocci b/docs/Coccilib.3cocci > > > > > > > index 0e4fbb8..ca5b061 100644 > > > > > > > --- a/docs/Coccilib.3cocci > > > > > > > +++ b/docs/Coccilib.3cocci > > > > > > > @@ -232,6 +232,15 @@ is the empty list if spatch is not currently > > > > > > > working on any file (eg, > > > > > > > in an initialize or finalize rule). > > > > > > > .sp > > > > > > > > > > > > > > +.I val cocci_version > > > > > > > +: > > > > > > > +.B unit -> string > > > > > > > +.sp > > > > > > > +Returns the a string indicating the current version. Note that > > > > > > > if > > > > > > > +Coccinelle has been modified since a release, the version number > > > > > > > will be > > > > > > > +postfixed with "-dirty". > > > > > > > +.sp > > > > > > > + > > > > > > > .I val print_main > > > > > > > : > > > > > > > .B ?color:string -> string -> pos list -> unit > > > > > > > diff --git a/ocaml/coccilib.ml b/ocaml/coccilib.ml > > > > > > > index f60c6b2..2f352d8 100644 > > > > > > > --- a/ocaml/coccilib.ml > > > > > > > +++ b/ocaml/coccilib.ml > > > > > > > @@ -168,6 +168,8 @@ let dir () = !Flag.dir > > > > > > > > > > > > > > let files () = !Flag.currentfiles > > > > > > > > > > > > > > +let cocci_version () = Config.version > > > > > > > + > > > > > > > (* > > > > > > > -- > > > > > > > *) > > > > > > > (* org mode *) > > > > > > > > > > > > > > > > > > > > > > > > > > Anything to *only* get the version instead of a long list is nice, > > > > > > right now > > > > > > spatch --version spits out: > > > > > > > > > > > > spatch version 1.0.5 compiled with OCaml version 4.02.3 > > > > > > Flags passed to the configure script: [none] > > > > > > Python scripting support: yes > > > > > > Syntax of regular expresssions: PCRE > > > > > > > > > > > > The Python library just parses the 3rd item at the top so it can > > > > > > extract > > > > > > the version. But surely if spatch --version-only was available we'd > > > > > > use > > > > > > that instead a well. > > > > > > > > > > > > Other than this though how can we require coccinelle version checks > > > > > > per > > > > > > SmPL file cleanly and also what should we do to make it backward > > > > > > compatible > > > > > > with older versions of coccinelle? > > > > > > > > > > I'm not sure that being backward compatible with older versions of > > > > > Coccinelle is worth adding new libraries to the Linux kernel, and > > > > > adding > > > > > unpleasant python code to semantic patches. > > > > > > > > True. I'm more than happy to not have to add this crap. > > > > > > > > >
Re: [Cocci] scripts: add glimpse.sh for indexing the kernel
On Wed, Jun 15, 2016 at 08:24:03PM +0200, SF Markus Elfring wrote: > > http://webglimpse.net/ > > Its vaguely mentioned there. That's as good as it gets... > > How do you think about to insert the date "2014-09-26" into your commit > message? Sure. > >> https://github.com/gvelez17/glimpse/ > > Sadly the original release didn't compile, I reported the issue in hopes it > > would > > be fixed. Nothing has happened since and I got tired of waiting so I forked > > and fixed it myself. > > I am also curious on further improvements for this software. > How will corresponding constructive feedback evolve? Beats me. As I see it no one cares over this software now, it would seem some folks have used it in proprietary crap and those versions are much better. To me what we can best do is see what we can take out of agrep, and port it to proper GNU grep, then see if git can also enable complex queries as Julia notes. Patches to those projects welcomed I'm sure. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v3 3/6] scripts: add glimpse.sh for indexing the kernel
On Wed, Jun 15, 2016 at 12:26:19PM +0200, SF Markus Elfring wrote: > > Glimpse is a tool you can use to index the kernel. > > The tool was recently open sourced under the ISC license > > and can be obtained at: > > Do you find any official announcement for this software evolution? I had reached out to Udi Manber in 2014, with the help of a few others we helped persuade University of Arizona of an ISC release. http://webglimpse.net/ Its vaguely mentioned there. That's as good as it gets... > Is the wording "recent" really appropriate if you would eventually like to > refer to a repository which was published in the year 2014? > https://github.com/gvelez17/glimpse/ Sadly the original release didn't compile, I reported the issue in hopes it would be fixed. Nothing has happened since and I got tired of waiting so I forked and fixed it myself. I sent a pull request but I don't think its been merged yet. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options
On Wed, Jun 15, 2016 at 05:44:01PM +0200, Julia Lawall wrote: > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > OK thanks. I remove --very-quiet now if --profile is used within SPFLAGS, > > I'll extend > > this to also avoid --very-quiet if --show-trying is used. SPFLAGS is where > > you can > > specify extra options that the script doesn't specifically support. > > If it is more convenient, you don't actually have to remove --very-quiet. > You can just put --quiet before --show-trying or --profile. --quiet will > override --very-quiet. How about just: if [ "$SPFLAGS" == *"--profile"* -o "$SPFLAGS" == "--show-trying" ]; then FLAGS="--quiet $SPFLAGS" else FLAGS="--very-quiet $SPFLAGS" fi > > > > > > > > > Originally our use of parmap made output, specia files based > > > > > > > > > on pids. Maybe this > > > > > > > > > is the default for parmap. I found this completely unusable. > > > > > > > > > I guess one > > > > > > > > > could look at the dates to see which file is the most recent > > > > > > > > > one, but it > > > > > > > > > seems tedious. If you are putting the standard output in > > > > > > > > > x.out, then put > > > > > > > > > the standard error in x.err. > > > > > > > > > > > > > > > > I'll use ${DIR}/coccicheck.$$.err for stderr. > > > > > > > > > > > > > > What is ${DIR}? and what is $$? > > > > > > > > > > > > When you run scripts/coccicheck we take the absolute directory > > > > > > of it and then go down one level of directory, so in this case it > > > > > > would be the base directory of the Linux kernel. > > > > > > > > > > > > $$ is the PID of the bash script. > > > > > > > > > > OK. I still don't find PIDs useful, but I guess if we are talking > > > > > about > > > > > the entire output of coccicheck, there is not much else to do. > > > > > Normally, > > > > > I don't want these files accumulating, and just write over the old > > > > > ones. > > > > > > > > Which is why I would much prefer to instead just redirect in coccicheck > > > > case stderr to stdout from coccinelle. Is that preferred? > > > > > > Then things will be merged in strange ways. > > > > > > Why not just let the user decide what to do with these things? > > > > Sure, what should be the default? > > I would normally just expect standard output and standard error to appear > randomly on the screen. That is, no management effort from the tool at > all. But the thing is, stderr is ignored now given that a shell script is used wrapped over a Makefile so if we want what you describe I think we do have to by default do 2>&1 on the spatch run command. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 4/4] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
On Wed, Jun 15, 2016 at 06:11:57PM +0200, Julia Lawall wrote: > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > > On Wed, Jun 15, 2016 at 05:55:34PM +0200, Julia Lawall wrote: > > > > > > > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > > > > > > On Wed, Jun 15, 2016 at 10:43:30AM +0200, Julia Lawall wrote: > > > > > How about the following, since Coccinelle knows what its version is? > > > > > This could of course be implemented in python as well. > > > > > > > > > > julia > > > > > > > > > > diff --git a/docs/Coccilib.3cocci b/docs/Coccilib.3cocci > > > > > index 0e4fbb8..ca5b061 100644 > > > > > --- a/docs/Coccilib.3cocci > > > > > +++ b/docs/Coccilib.3cocci > > > > > @@ -232,6 +232,15 @@ is the empty list if spatch is not currently > > > > > working on any file (eg, > > > > > in an initialize or finalize rule). > > > > > .sp > > > > > > > > > > +.I val cocci_version > > > > > +: > > > > > +.B unit -> string > > > > > +.sp > > > > > +Returns the a string indicating the current version. Note that if > > > > > +Coccinelle has been modified since a release, the version number > > > > > will be > > > > > +postfixed with "-dirty". > > > > > +.sp > > > > > + > > > > > .I val print_main > > > > > : > > > > > .B ?color:string -> string -> pos list -> unit > > > > > diff --git a/ocaml/coccilib.ml b/ocaml/coccilib.ml > > > > > index f60c6b2..2f352d8 100644 > > > > > --- a/ocaml/coccilib.ml > > > > > +++ b/ocaml/coccilib.ml > > > > > @@ -168,6 +168,8 @@ let dir () = !Flag.dir > > > > > > > > > > let files () = !Flag.currentfiles > > > > > > > > > > +let cocci_version () = Config.version > > > > > + > > > > > (* > > > > > -- > > > > > *) > > > > > (* org mode *) > > > > > > > > > > > > > > > > > > Anything to *only* get the version instead of a long list is nice, > > > > right now > > > > spatch --version spits out: > > > > > > > > spatch version 1.0.5 compiled with OCaml version 4.02.3 > > > > Flags passed to the configure script: [none] > > > > Python scripting support: yes > > > > Syntax of regular expresssions: PCRE > > > > > > > > The Python library just parses the 3rd item at the top so it can extract > > > > the version. But surely if spatch --version-only was available we'd use > > > > that instead a well. > > > > > > > > Other than this though how can we require coccinelle version checks per > > > > SmPL file cleanly and also what should we do to make it backward > > > > compatible > > > > with older versions of coccinelle? > > > > > > I'm not sure that being backward compatible with older versions of > > > Coccinelle is worth adding new libraries to the Linux kernel, and adding > > > unpleasant python code to semantic patches. > > > > True. I'm more than happy to not have to add this crap. > > > > > The above ocaml code just produces eg 1.0.5 or 1.0.5-dirty. I could drop > > > the -dirty at the coccilib level, if that seems desirable. > > > > This is when spatch --cocci_version is passed ? > > Perhaps it wasn't clear enough from the above nroff and ocaml code. I > added a function Coccilib.version() that returns eg either 1.0.5 or > 1.0.5-dirty. Such a function could be implemented for python as well. > > > > > Its still unclear how we can require in a clean way coccinelle version > > requirements in SmPL patches with this. Can you clarify? > > Test the string that it returns and exit. Like you are doing, but no need > for adding new libraries to the kernel. Ah then that's indeed welcome, however another function would be best too: Coccilib.version_reqs() which lets us say what the requirement is and it would return true or false, false when the req is not met. > > If we embrace this or assume we'll get this in the next release we'll have > > to just bump the kernel's coccinelle requirement recommendation, which I > > think > > is far due anyway. > > Yes. Great. Then I'm all for dropping this python jüjü crap. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 4/4] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
On Wed, Jun 15, 2016 at 05:55:34PM +0200, Julia Lawall wrote: > > > On Wed, 15 Jun 2016, Luis R. Rodriguez wrote: > > > On Wed, Jun 15, 2016 at 10:43:30AM +0200, Julia Lawall wrote: > > > How about the following, since Coccinelle knows what its version is? > > > This could of course be implemented in python as well. > > > > > > julia > > > > > > diff --git a/docs/Coccilib.3cocci b/docs/Coccilib.3cocci > > > index 0e4fbb8..ca5b061 100644 > > > --- a/docs/Coccilib.3cocci > > > +++ b/docs/Coccilib.3cocci > > > @@ -232,6 +232,15 @@ is the empty list if spatch is not currently working > > > on any file (eg, > > > in an initialize or finalize rule). > > > .sp > > > > > > +.I val cocci_version > > > +: > > > +.B unit -> string > > > +.sp > > > +Returns the a string indicating the current version. Note that if > > > +Coccinelle has been modified since a release, the version number will be > > > +postfixed with "-dirty". > > > +.sp > > > + > > > .I val print_main > > > : > > > .B ?color:string -> string -> pos list -> unit > > > diff --git a/ocaml/coccilib.ml b/ocaml/coccilib.ml > > > index f60c6b2..2f352d8 100644 > > > --- a/ocaml/coccilib.ml > > > +++ b/ocaml/coccilib.ml > > > @@ -168,6 +168,8 @@ let dir () = !Flag.dir > > > > > > let files () = !Flag.currentfiles > > > > > > +let cocci_version () = Config.version > > > + > > > (* > > > -- *) > > > (* org mode *) > > > > > > > > > > Anything to *only* get the version instead of a long list is nice, right now > > spatch --version spits out: > > > > spatch version 1.0.5 compiled with OCaml version 4.02.3 > > Flags passed to the configure script: [none] > > Python scripting support: yes > > Syntax of regular expresssions: PCRE > > > > The Python library just parses the 3rd item at the top so it can extract > > the version. But surely if spatch --version-only was available we'd use > > that instead a well. > > > > Other than this though how can we require coccinelle version checks per > > SmPL file cleanly and also what should we do to make it backward compatible > > with older versions of coccinelle? > > I'm not sure that being backward compatible with older versions of > Coccinelle is worth adding new libraries to the Linux kernel, and adding > unpleasant python code to semantic patches. True. I'm more than happy to not have to add this crap. > The above ocaml code just produces eg 1.0.5 or 1.0.5-dirty. I could drop > the -dirty at the coccilib level, if that seems desirable. This is when spatch --cocci_version is passed ? Its still unclear how we can require in a clean way coccinelle version requirements in SmPL patches with this. Can you clarify? If we embrace this or assume we'll get this in the next release we'll have to just bump the kernel's coccinelle requirement recommendation, which I think is far due anyway. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 2/4] scripts: add reqs python library
On Wed, Jun 15, 2016 at 08:06:33AM +0200, Julia Lawall wrote: > > > On Tue, 14 Jun 2016, Luis R. Rodriguez wrote: > > > This library can be used in other python scripts to require > > specific binary version requirements. It will be used first > > with coccinelle's python bindings to enable coccinelle SmPL > > files to specify version requirements per cocci file if it > > has any. > > > > Signed-off-by: Luis R. Rodriguez > > --- > > MAINTAINERS | 1 + > > scripts/lib/__init__.py | 1 + > > scripts/lib/reqs.py | 211 > > > > 3 files changed, 213 insertions(+) > > create mode 100644 scripts/lib/__init__.py > > create mode 100644 scripts/lib/reqs.py > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f83e19a2dd97..fdebbb513c1b 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6521,6 +6521,7 @@ F:scripts/Makefile.* > > F: scripts/basic/ > > F: scripts/mk* > > F: scripts/package/ > > +F: scripts/lib/ > > > > KERNEL JANITORS > > L: kernel-janit...@vger.kernel.org > > diff --git a/scripts/lib/__init__.py b/scripts/lib/__init__.py > > new file mode 100644 > > index ..1bb8bf6d7fd4 > > --- /dev/null > > +++ b/scripts/lib/__init__.py > > @@ -0,0 +1 @@ > > +# empty > > diff --git a/scripts/lib/reqs.py b/scripts/lib/reqs.py > > new file mode 100644 > > index ..1325fd21a87a > > --- /dev/null > > +++ b/scripts/lib/reqs.py > > @@ -0,0 +1,211 @@ > > +import subprocess, os, sys, re > > +""" > > +Often enough Python code can grow to depend on binaries > > +on a system, you may also require only specific versions > > +of these. This small library helps with this. It also has > > +helpers for packages which we know to handle already. > > +""" > > + > > +class ReqError(Exception): > > +pass > > +class ExecutionError(ReqError): > > +def __init__(self, errcode): > > +self.error_code = errcode > > + > > +class Req: > > +"To be used for verifying binay package dependencies on Python code" > > binay -> binary Fixed, thanks. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 2/4] scripts: add reqs python library
On Wed, Jun 15, 2016 at 09:50:11AM +0200, Michal Marek wrote: > On 2016-06-15 00:10, Luis R. Rodriguez wrote: > > +weight = (int(rel_specs['VERSION'])<< 32) + \ > > + (int(rel_specs['PATCHLEVEL']) << 16) + \ > > + (sublevel<< 8 ) + \ > > + (extra * 60) + (relmod * 2) > > This is going to silently break as soon as we have a version number with > e.g. a time stamp embedded. Well this is adhering to a linux_version_cmp type, surely we can adjust it with alternatives. It just happens that with the common stuff this suffices. Do you have a specific string in mind I can use to test against? > And there is actually no need to convert the > version string to an integer. You can convert them to arrays of > components and compare the components one by one. You can, however weight is used to as a generic formula to determine in one shot if you have a release >= than what is required. Its inspired by how KERNEL_VERSION() is used: #define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 2/4] scripts: add reqs python library
On Wed, Jun 15, 2016 at 02:01:37PM +0200, SF Markus Elfring wrote: > > +"To be used for verifying binay package dependencies on Python code" > > Would you like to fix a typo here? > > ... binary ... Fixed. Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH 4/4] scripts/coccinelle: require coccinelle >= 1.0.4 on device_node_continue.cocci
On Wed, Jun 15, 2016 at 10:43:30AM +0200, Julia Lawall wrote: > How about the following, since Coccinelle knows what its version is? > This could of course be implemented in python as well. > > julia > > diff --git a/docs/Coccilib.3cocci b/docs/Coccilib.3cocci > index 0e4fbb8..ca5b061 100644 > --- a/docs/Coccilib.3cocci > +++ b/docs/Coccilib.3cocci > @@ -232,6 +232,15 @@ is the empty list if spatch is not currently working on > any file (eg, > in an initialize or finalize rule). > .sp > > +.I val cocci_version > +: > +.B unit -> string > +.sp > +Returns the a string indicating the current version. Note that if > +Coccinelle has been modified since a release, the version number will be > +postfixed with "-dirty". > +.sp > + > .I val print_main > : > .B ?color:string -> string -> pos list -> unit > diff --git a/ocaml/coccilib.ml b/ocaml/coccilib.ml > index f60c6b2..2f352d8 100644 > --- a/ocaml/coccilib.ml > +++ b/ocaml/coccilib.ml > @@ -168,6 +168,8 @@ let dir () = !Flag.dir > > let files () = !Flag.currentfiles > > +let cocci_version () = Config.version > + > (* -- *) > (* org mode *) > > Anything to *only* get the version instead of a long list is nice, right now spatch --version spits out: spatch version 1.0.5 compiled with OCaml version 4.02.3 Flags passed to the configure script: [none] Python scripting support: yes Syntax of regular expresssions: PCRE The Python library just parses the 3rd item at the top so it can extract the version. But surely if spatch --version-only was available we'd use that instead a well. Other than this though how can we require coccinelle version checks per SmPL file cleanly and also what should we do to make it backward compatible with older versions of coccinelle? Luis ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci