All of lore.kernel.org
 help / color / mirror / Atom feed
* [Cocci] [bug] exists do not work if file group is too big (>49)
@ 2018-05-16 19:16 Jerome Glisse
  2018-05-16 19:37 ` Julia Lawall
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Jerome Glisse @ 2018-05-16 19:16 UTC (permalink / raw)
  To: cocci

When operating on too many files the exists keyword no longer work. For
instance:

@exists@
expression E1;
@@
set_page_dirty(
+NULL,
E1)

Using linux kernel as playground and looking at at fs/f2fs/gc.c where
there is set_page_dirty() calls in nested blocks.

Works:

spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/char/oradax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto


Don't works:

spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/char/oradax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto


Exact same command line except the latter have 49 files before
fs/f2fs/gc.c while the former has 48 (and it works then). Note
that it seems that running the script multiple times even when
they are too many files will eventualy work.

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 19:16 [Cocci] [bug] exists do not work if file group is too big (>49) Jerome Glisse
@ 2018-05-16 19:37 ` Julia Lawall
  2018-05-16 19:54   ` Jerome Glisse
       [not found] ` <128b6146-5368-12bb-ae42-236982ff3494@users.sourceforge.net>
  2018-05-17 20:21 ` [Cocci] [bug] exists do not work if file group is too big (>49) Julia Lawall
  2 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-16 19:37 UTC (permalink / raw)
  To: cocci



On Wed, 16 May 2018, Jerome Glisse wrote:

> When operating on too many files the exists keyword no longer work. For
> instance:
>
> @exists@
> expression E1;
> @@
> set_page_dirty(
> +NULL,
> E1)
>
> Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> there is set_page_dirty() calls in nested blocks.
>
> Works:
>
> spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/!
 char/oradax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
>
>
> Don't works:
>
> spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/!
 char/oradax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
>
>
> Exact same command line except the latter have 49 files before
> fs/f2fs/gc.c while the former has 48 (and it works then). Note
> that it seems that running the script multiple times even when
> they are too many files will eventualy work.

I'm surprised that the number of files would have an impact, unless by not
working you mean that it runs out of memory or doesn't terminate.

In general, you should only give one file name or one directory name to
Coccinelle.  If you give multiple files, then they are all treated at
once, ie their CFGs are all in memory at the same time.  If you give the
name of a directory containing the files, then it will work on one file at
a time, and you can give the option -j XXX to have it run XXX threads in
parallel.  Giving multiple file names on the command line is only for the
case where information collected from one file is needed for the treatment
of another file.

If the set of files you want to work on doesn't correspond to the
directory structure, you can make a file with the names of the files to
treat one by one each separated by a blank line, and then give the command
line argument --file-groups filename.

Actually, exists is not relevant to your rule, because it has no ...
Exists means that there exists a path through the ... that matches the
pattern.  Otherwise, all paths have to match the pattern.

julia

>
> Cheers,
> J?r?me
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 19:37 ` Julia Lawall
@ 2018-05-16 19:54   ` Jerome Glisse
  2018-05-16 20:02     ` Julia Lawall
  0 siblings, 1 reply; 21+ messages in thread
From: Jerome Glisse @ 2018-05-16 19:54 UTC (permalink / raw)
  To: cocci

On Wed, May 16, 2018 at 09:37:39PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 16 May 2018, Jerome Glisse wrote:
> 
> > When operating on too many files the exists keyword no longer work. For
> > instance:
> >
> > @exists@
> > expression E1;
> > @@
> > set_page_dirty(
> > +NULL,
> > E1)
> >
> > Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> > there is set_page_dirty() calls in nested blocks.
> >
> > Works:
> >
> > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/char/orad
>  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
> >
> >
> > Don't works:
> >
> > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/char/orad
>  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
> >
> >
> > Exact same command line except the latter have 49 files before
> > fs/f2fs/gc.c while the former has 48 (and it works then). Note
> > that it seems that running the script multiple times even when
> > they are too many files will eventualy work.
> 
> I'm surprised that the number of files would have an impact, unless by not
> working you mean that it runs out of memory or doesn't terminate.

Don't work == only set_page_dirty in non nested block are modified,
set_page_dirty in nested block are not. So it runs and terminate and
all files are processed (ie it is not a command line size limitation
in the shell).

> 
> In general, you should only give one file name or one directory name to
> Coccinelle.  If you give multiple files, then they are all treated at
> once, ie their CFGs are all in memory at the same time.  If you give the
> name of a directory containing the files, then it will work on one file at
> a time, and you can give the option -j XXX to have it run XXX threads in
> parallel.  Giving multiple file names on the command line is only for the
> case where information collected from one file is needed for the treatment
> of another file.

Saddly for modification i am doing i need to treat many files as one
as otherwise it does not work and even sadder for me is that this groups
can get pretty big.

> 
> If the set of files you want to work on doesn't correspond to the
> directory structure, you can make a file with the names of the files to
> treat one by one each separated by a blank line, and then give the command
> line argument --file-groups filename.

This is _slower_ (by an order of magnitude) and also spit out a lot of
error messages (about trying to cd to directory and failing) than passing
all the files on the command line.

> 
> Actually, exists is not relevant to your rule, because it has no ...
> Exists means that there exists a path through the ... that matches the
> pattern.  Otherwise, all paths have to match the pattern.

Oh i thought i would need it, nonetheless when there is too many files
the above snipet stop at first level blocks and never modify block with
deeper nesting.

Thank you for all your feedback :)

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 19:54   ` Jerome Glisse
@ 2018-05-16 20:02     ` Julia Lawall
  2018-05-16 20:15       ` Jerome Glisse
  0 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-16 20:02 UTC (permalink / raw)
  To: cocci



On Wed, 16 May 2018, Jerome Glisse wrote:

> On Wed, May 16, 2018 at 09:37:39PM +0200, Julia Lawall wrote:
> >
> >
> > On Wed, 16 May 2018, Jerome Glisse wrote:
> >
> > > When operating on too many files the exists keyword no longer work. For
> > > instance:
> > >
> > > @exists@
> > > expression E1;
> > > @@
> > > set_page_dirty(
> > > +NULL,
> > > E1)
> > >
> > > Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> > > there is set_page_dirty() calls in nested blocks.
> > >
> > > Works:
> > >
> > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/s!
 bus/char/orad
> >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
> > >
> > >
> > > Don't works:
> > >
> > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/s!
 bus/char/orad
> >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
> > >
> > >
> > > Exact same command line except the latter have 49 files before
> > > fs/f2fs/gc.c while the former has 48 (and it works then). Note
> > > that it seems that running the script multiple times even when
> > > they are too many files will eventualy work.
> >
> > I'm surprised that the number of files would have an impact, unless by not
> > working you mean that it runs out of memory or doesn't terminate.
>
> Don't work == only set_page_dirty in non nested block are modified,
> set_page_dirty in nested block are not. So it runs and terminate and
> all files are processed (ie it is not a command line size limitation
> in the shell).
>
> >
> > In general, you should only give one file name or one directory name to
> > Coccinelle.  If you give multiple files, then they are all treated at
> > once, ie their CFGs are all in memory at the same time.  If you give the
> > name of a directory containing the files, then it will work on one file at
> > a time, and you can give the option -j XXX to have it run XXX threads in
> > parallel.  Giving multiple file names on the command line is only for the
> > case where information collected from one file is needed for the treatment
> > of another file.
>
> Saddly for modification i am doing i need to treat many files as one
> as otherwise it does not work and even sadder for me is that this groups
> can get pretty big.
>
> >
> > If the set of files you want to work on doesn't correspond to the
> > directory structure, you can make a file with the names of the files to
> > treat one by one each separated by a blank line, and then give the command
> > line argument --file-groups filename.
>
> This is _slower_ (by an order of magnitude) and also spit out a lot of
> error messages (about trying to cd to directory and failing) than passing
> all the files on the command line.
>
> >
> > Actually, exists is not relevant to your rule, because it has no ...
> > Exists means that there exists a path through the ... that matches the
> > pattern.  Otherwise, all paths have to match the pattern.
>
> Oh i thought i would need it, nonetheless when there is too many files
> the above snipet stop at first level blocks and never modify block with
> deeper nesting.
>
> Thank you for all your feedback :)

Sorry, but none of what you are saying makes any sense to me.  Coccinele
works one function at a time.  There is no reason why its behavior on
nested calls would be affected by the number of files.  And no reason why
working on one file at a time would be slower than working on all of the
files at once.  I'll have to try your command lines, but I probably can't
do that until tomorrow afternoon...

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 20:02     ` Julia Lawall
@ 2018-05-16 20:15       ` Jerome Glisse
  2018-05-16 20:20         ` Julia Lawall
       [not found]         ` <10d2d384-95c0-dec1-b284-f5afb7d9ce81@users.sourceforge.net>
  0 siblings, 2 replies; 21+ messages in thread
From: Jerome Glisse @ 2018-05-16 20:15 UTC (permalink / raw)
  To: cocci

On Wed, May 16, 2018 at 10:02:08PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 16 May 2018, Jerome Glisse wrote:
> 
> > On Wed, May 16, 2018 at 09:37:39PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Wed, 16 May 2018, Jerome Glisse wrote:
> > >
> > > > When operating on too many files the exists keyword no longer work. For
> > > > instance:
> > > >
> > > > @exists@
> > > > expression E1;
> > > > @@
> > > > set_page_dirty(
> > > > +NULL,
> > > > E1)
> > > >
> > > > Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> > > > there is set_page_dirty() calls in nested blocks.
> > > >
> > > > Works:
> > > >
> > > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/char/
>  orad
> > >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
> > > >
> > > >
> > > > Don't works:
> > > >
> > > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/char/
>  orad
> > >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
> > > >
> > > >
> > > > Exact same command line except the latter have 49 files before
> > > > fs/f2fs/gc.c while the former has 48 (and it works then). Note
> > > > that it seems that running the script multiple times even when
> > > > they are too many files will eventualy work.
> > >
> > > I'm surprised that the number of files would have an impact, unless by not
> > > working you mean that it runs out of memory or doesn't terminate.
> >
> > Don't work == only set_page_dirty in non nested block are modified,
> > set_page_dirty in nested block are not. So it runs and terminate and
> > all files are processed (ie it is not a command line size limitation
> > in the shell).
> >
> > >
> > > In general, you should only give one file name or one directory name to
> > > Coccinelle.  If you give multiple files, then they are all treated at
> > > once, ie their CFGs are all in memory at the same time.  If you give the
> > > name of a directory containing the files, then it will work on one file at
> > > a time, and you can give the option -j XXX to have it run XXX threads in
> > > parallel.  Giving multiple file names on the command line is only for the
> > > case where information collected from one file is needed for the treatment
> > > of another file.
> >
> > Saddly for modification i am doing i need to treat many files as one
> > as otherwise it does not work and even sadder for me is that this groups
> > can get pretty big.
> >
> > >
> > > If the set of files you want to work on doesn't correspond to the
> > > directory structure, you can make a file with the names of the files to
> > > treat one by one each separated by a blank line, and then give the command
> > > line argument --file-groups filename.
> >
> > This is _slower_ (by an order of magnitude) and also spit out a lot of
> > error messages (about trying to cd to directory and failing) than passing
> > all the files on the command line.
> >
> > >
> > > Actually, exists is not relevant to your rule, because it has no ...
> > > Exists means that there exists a path through the ... that matches the
> > > pattern.  Otherwise, all paths have to match the pattern.
> >
> > Oh i thought i would need it, nonetheless when there is too many files
> > the above snipet stop at first level blocks and never modify block with
> > deeper nesting.
> >
> > Thank you for all your feedback :)
> 
> Sorry, but none of what you are saying makes any sense to me.  Coccinele
> works one function at a time.  There is no reason why its behavior on
> nested calls would be affected by the number of files.  And no reason why
> working on one file at a time would be slower than working on all of the
> files at once.  I'll have to try your command lines, but I probably can't
> do that until tomorrow afternoon...

Yes this is weird bug i actually tested again and it only shows if
exists is use. If i remove exists like you adviced it seems to work
no matter how many files i pass as argument.

When the rules has the exists modifier then it only modify inside
nested block if i pass 48 files or less. If i pass 49 files or more
then nested block are not modified but first level block is.

My buest guess is some memory corruption somewhere or some variable
that is not reset properly.

I am on version 1.0.6 (fedora 27).

Note i am in no rush, i just wanted to report this as it is likely a
bug somewhere moreover i have a work around.

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 20:15       ` Jerome Glisse
@ 2018-05-16 20:20         ` Julia Lawall
  2018-05-16 20:24           ` Jerome Glisse
       [not found]         ` <10d2d384-95c0-dec1-b284-f5afb7d9ce81@users.sourceforge.net>
  1 sibling, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-16 20:20 UTC (permalink / raw)
  To: cocci



On Wed, 16 May 2018, Jerome Glisse wrote:

> On Wed, May 16, 2018 at 10:02:08PM +0200, Julia Lawall wrote:
> >
> >
> > On Wed, 16 May 2018, Jerome Glisse wrote:
> >
> > > On Wed, May 16, 2018 at 09:37:39PM +0200, Julia Lawall wrote:
> > > >
> > > >
> > > > On Wed, 16 May 2018, Jerome Glisse wrote:
> > > >
> > > > > When operating on too many files the exists keyword no longer work. For
> > > > > instance:
> > > > >
> > > > > @exists@
> > > > > expression E1;
> > > > > @@
> > > > > set_page_dirty(
> > > > > +NULL,
> > > > > E1)
> > > > >
> > > > > Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> > > > > there is set_page_dirty() calls in nested blocks.
> > > > >
> > > > > Works:
> > > > >
> > > > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drive!
 rs/sbus/char/
> >  orad
> > > >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
> > > > >
> > > > >
> > > > > Don't works:
> > > > >
> > > > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drive!
 rs/sbus/char/
> >  orad
> > > >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
> > > > >
> > > > >
> > > > > Exact same command line except the latter have 49 files before
> > > > > fs/f2fs/gc.c while the former has 48 (and it works then). Note
> > > > > that it seems that running the script multiple times even when
> > > > > they are too many files will eventualy work.
> > > >
> > > > I'm surprised that the number of files would have an impact, unless by not
> > > > working you mean that it runs out of memory or doesn't terminate.
> > >
> > > Don't work == only set_page_dirty in non nested block are modified,
> > > set_page_dirty in nested block are not. So it runs and terminate and
> > > all files are processed (ie it is not a command line size limitation
> > > in the shell).
> > >
> > > >
> > > > In general, you should only give one file name or one directory name to
> > > > Coccinelle.  If you give multiple files, then they are all treated at
> > > > once, ie their CFGs are all in memory at the same time.  If you give the
> > > > name of a directory containing the files, then it will work on one file at
> > > > a time, and you can give the option -j XXX to have it run XXX threads in
> > > > parallel.  Giving multiple file names on the command line is only for the
> > > > case where information collected from one file is needed for the treatment
> > > > of another file.
> > >
> > > Saddly for modification i am doing i need to treat many files as one
> > > as otherwise it does not work and even sadder for me is that this groups
> > > can get pretty big.
> > >
> > > >
> > > > If the set of files you want to work on doesn't correspond to the
> > > > directory structure, you can make a file with the names of the files to
> > > > treat one by one each separated by a blank line, and then give the command
> > > > line argument --file-groups filename.
> > >
> > > This is _slower_ (by an order of magnitude) and also spit out a lot of
> > > error messages (about trying to cd to directory and failing) than passing
> > > all the files on the command line.
> > >
> > > >
> > > > Actually, exists is not relevant to your rule, because it has no ...
> > > > Exists means that there exists a path through the ... that matches the
> > > > pattern.  Otherwise, all paths have to match the pattern.
> > >
> > > Oh i thought i would need it, nonetheless when there is too many files
> > > the above snipet stop at first level blocks and never modify block with
> > > deeper nesting.
> > >
> > > Thank you for all your feedback :)
> >
> > Sorry, but none of what you are saying makes any sense to me.  Coccinele
> > works one function at a time.  There is no reason why its behavior on
> > nested calls would be affected by the number of files.  And no reason why
> > working on one file at a time would be slower than working on all of the
> > files at once.  I'll have to try your command lines, but I probably can't
> > do that until tomorrow afternoon...
>
> Yes this is weird bug i actually tested again and it only shows if
> exists is use. If i remove exists like you adviced it seems to work
> no matter how many files i pass as argument.
>
> When the rules has the exists modifier then it only modify inside
> nested block if i pass 48 files or less. If i pass 49 files or more
> then nested block are not modified but first level block is.
>
> My buest guess is some memory corruption somewhere or some variable
> that is not reset properly.

Are you using the bytecode or the native code version of Coccinelle?

I think you can say make byte-only to get the bytecode version, from the
sources on github.

julia

>
> I am on version 1.0.6 (fedora 27).
>
> Note i am in no rush, i just wanted to report this as it is likely a
> bug somewhere moreover i have a work around.
>
> Cheers,
> J?r?me
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 20:20         ` Julia Lawall
@ 2018-05-16 20:24           ` Jerome Glisse
  2018-05-16 20:29             ` Julia Lawall
  0 siblings, 1 reply; 21+ messages in thread
From: Jerome Glisse @ 2018-05-16 20:24 UTC (permalink / raw)
  To: cocci

On Wed, May 16, 2018 at 10:20:05PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 16 May 2018, Jerome Glisse wrote:
> 
> > On Wed, May 16, 2018 at 10:02:08PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Wed, 16 May 2018, Jerome Glisse wrote:
> > >
> > > > On Wed, May 16, 2018 at 09:37:39PM +0200, Julia Lawall wrote:
> > > > >
> > > > >
> > > > > On Wed, 16 May 2018, Jerome Glisse wrote:
> > > > >
> > > > > > When operating on too many files the exists keyword no longer work. For
> > > > > > instance:
> > > > > >
> > > > > > @exists@
> > > > > > expression E1;
> > > > > > @@
> > > > > > set_page_dirty(
> > > > > > +NULL,
> > > > > > E1)
> > > > > >
> > > > > > Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> > > > > > there is set_page_dirty() calls in nested blocks.
> > > > > >
> > > > > > Works:
> > > > > >
> > > > > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/c
>  har/
> > >  orad
> > > > >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
> > > > > >
> > > > > >
> > > > > > Don't works:
> > > > > >
> > > > > > spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/c
>  har/
> > >  orad
> > > > >  ax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
> > > > > >
> > > > > >
> > > > > > Exact same command line except the latter have 49 files before
> > > > > > fs/f2fs/gc.c while the former has 48 (and it works then). Note
> > > > > > that it seems that running the script multiple times even when
> > > > > > they are too many files will eventualy work.
> > > > >
> > > > > I'm surprised that the number of files would have an impact, unless by not
> > > > > working you mean that it runs out of memory or doesn't terminate.
> > > >
> > > > Don't work == only set_page_dirty in non nested block are modified,
> > > > set_page_dirty in nested block are not. So it runs and terminate and
> > > > all files are processed (ie it is not a command line size limitation
> > > > in the shell).
> > > >
> > > > >
> > > > > In general, you should only give one file name or one directory name to
> > > > > Coccinelle.  If you give multiple files, then they are all treated at
> > > > > once, ie their CFGs are all in memory at the same time.  If you give the
> > > > > name of a directory containing the files, then it will work on one file at
> > > > > a time, and you can give the option -j XXX to have it run XXX threads in
> > > > > parallel.  Giving multiple file names on the command line is only for the
> > > > > case where information collected from one file is needed for the treatment
> > > > > of another file.
> > > >
> > > > Saddly for modification i am doing i need to treat many files as one
> > > > as otherwise it does not work and even sadder for me is that this groups
> > > > can get pretty big.
> > > >
> > > > >
> > > > > If the set of files you want to work on doesn't correspond to the
> > > > > directory structure, you can make a file with the names of the files to
> > > > > treat one by one each separated by a blank line, and then give the command
> > > > > line argument --file-groups filename.
> > > >
> > > > This is _slower_ (by an order of magnitude) and also spit out a lot of
> > > > error messages (about trying to cd to directory and failing) than passing
> > > > all the files on the command line.
> > > >
> > > > >
> > > > > Actually, exists is not relevant to your rule, because it has no ...
> > > > > Exists means that there exists a path through the ... that matches the
> > > > > pattern.  Otherwise, all paths have to match the pattern.
> > > >
> > > > Oh i thought i would need it, nonetheless when there is too many files
> > > > the above snipet stop at first level blocks and never modify block with
> > > > deeper nesting.
> > > >
> > > > Thank you for all your feedback :)
> > >
> > > Sorry, but none of what you are saying makes any sense to me.  Coccinele
> > > works one function at a time.  There is no reason why its behavior on
> > > nested calls would be affected by the number of files.  And no reason why
> > > working on one file at a time would be slower than working on all of the
> > > files at once.  I'll have to try your command lines, but I probably can't
> > > do that until tomorrow afternoon...
> >
> > Yes this is weird bug i actually tested again and it only shows if
> > exists is use. If i remove exists like you adviced it seems to work
> > no matter how many files i pass as argument.
> >
> > When the rules has the exists modifier then it only modify inside
> > nested block if i pass 48 files or less. If i pass 49 files or more
> > then nested block are not modified but first level block is.
> >
> > My buest guess is some memory corruption somewhere or some variable
> > that is not reset properly.
> 
> Are you using the bytecode or the native code version of Coccinelle?
> 
> I think you can say make byte-only to get the bytecode version, from the
> sources on github.

Not sure what that is here is the output of spatch --version on fedora 27:

[glisse at localhost ~]$ spatch --version
spatch version 1.0.6 compiled with OCaml version 4.05.0
Flags passed to the configure script: --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --enable-release=yes --with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir
Python scripting support: yes
Syntax of regular expresssions: PCRE

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 20:24           ` Jerome Glisse
@ 2018-05-16 20:29             ` Julia Lawall
  2018-05-16 20:35               ` Jerome Glisse
  0 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-16 20:29 UTC (permalink / raw)
  To: cocci

> [glisse at localhost ~]$ spatch --version
> spatch version 1.0.6 compiled with OCaml version 4.05.0
> Flags passed to the configure script: --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --enable-release=yes --with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir
> Python scripting support: yes
> Syntax of regular expresssions: PCRE

I don't think this indicates whether it is bytecode or native code, and I
don't know what the fedora people offer.

My bytecode executable is 41M and my native code executable is 13M.

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 20:29             ` Julia Lawall
@ 2018-05-16 20:35               ` Jerome Glisse
  0 siblings, 0 replies; 21+ messages in thread
From: Jerome Glisse @ 2018-05-16 20:35 UTC (permalink / raw)
  To: cocci

On Wed, May 16, 2018 at 10:29:30PM +0200, Julia Lawall wrote:
> > [glisse at localhost ~]$ spatch --version
> > spatch version 1.0.6 compiled with OCaml version 4.05.0
> > Flags passed to the configure script: --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --enable-release=yes --with-python=/usr/bin/python3 --with-menhir=/usr/bin/menhir
> > Python scripting support: yes
> > Syntax of regular expresssions: PCRE
> 
> I don't think this indicates whether it is bytecode or native code, and I
> don't know what the fedora people offer.
> 
> My bytecode executable is 41M and my native code executable is 13M.

I am guessing native code:
-rwxr-xr-x. 1 root root 1.3K May 10 13:51 /usr/bin/spatch

J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] exists do not work if file group is too big
       [not found] ` <128b6146-5368-12bb-ae42-236982ff3494@users.sourceforge.net>
@ 2018-05-17 14:45   ` Jerome Glisse
  2018-05-17 20:45     ` Julia Lawall
  0 siblings, 1 reply; 21+ messages in thread
From: Jerome Glisse @ 2018-05-17 14:45 UTC (permalink / raw)
  To: cocci

On Thu, May 17, 2018 at 09:33:09AM +0200, SF Markus Elfring wrote:
> > When operating on too many files the exists keyword no longer work.
> 
> How do you think about to show a more precise error message for one approach
> to add the preprocessor symbol ?NULL? as the first parameter in a call
> of the function ?set_page_dirty??

There is no error message in the issue i reported. Nested block are
just silently no longer modified just because there is too many files.

> 
> 
> > spatch ? --include fs/afs/internal.h ?
> 
> * I find the position of this command parameter unusual.

I am using git grep to find files which reference the symbols i am
modifying and i am doing a bit of sed regular expression on its out-
put to prepend header files with --include

In the end i probably will have 40-50 semantics patches and i do not
wish to create by hand a list of files to modify especialy as if i
do so then i loose the big selling point of coccinelle ie not having
to worry about rebasing patches if things move around.

> 
> * Are you sure that you would like to adjust the selected source files
>   by a single command?

So as i am modifying a lot of functions which are close in sources
files, i am getting error with --in-place with headers files, i could
spit a patch and use patch -r - -f ... to force thing but this not only
complexify the whole set of commands needed to run, it also fails in
more sublte way (when doing twice the same modification which can lead
to missing chunk modification).

This is why i am gretting groups of files to modify one function at a
time and i also pass the function name to coccinelle using -D fn=name

I have try many ways to do this and after coupls weeks of iterations
this is the cleanest simplest, fastest solution i converged on.

> 
> * Would you like to pass any file name to a command variant in a loop?

I would not see the benefit in that, i use --no-includes in my command
line and modification are pretty fast so i am roughly happy as it is :)

Hopes this feedback helps. My biggest issue with coccinelle is the lack
of support for function pointer typedef. Otherwise it is a pretty smooth
experience. I have also an issue with python execution but this might be
a fedora packaging issue, i have a local patch that make it works in
fedora 27 but i need to check coccinelle git first to see if it is still
an issue in lastest version. I need to inspect fedora spec file too.

Big kudos to everyone working on this tools.

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] Checking consequences from “exists” usage with big file name selection
       [not found]         ` <10d2d384-95c0-dec1-b284-f5afb7d9ce81@users.sourceforge.net>
@ 2018-05-17 14:50           ` Jerome Glisse
  0 siblings, 0 replies; 21+ messages in thread
From: Jerome Glisse @ 2018-05-17 14:50 UTC (permalink / raw)
  To: cocci

On Thu, May 17, 2018 at 11:11:04AM +0200, SF Markus Elfring wrote:
> > Note i am in no rush, i just wanted to report this as it is likely a bug somewhere
> 
> Thanks for your description of a strange software behaviour.
> 
> How often do you work with the specification ?exists? in other SmPL scripts?

I need to review all of them but i think i can remove most of them
now that Julia explained that i do not need them if there is no ...
between first match and second match in my rules.

> 
> 
> > moreover i have a work around.
> 
> Is the other transformation approach the solution which is really desired?

My work around was to add function with nested block to an extra
group on which i run the same semantic patch again to take care of
nested block when the original group as two big.

This is a minor inconvenience now that i have found why my semantic
patch was not modifying nested block (took me a while to converge
on the number of files as root of the issue).

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-16 19:16 [Cocci] [bug] exists do not work if file group is too big (>49) Jerome Glisse
  2018-05-16 19:37 ` Julia Lawall
       [not found] ` <128b6146-5368-12bb-ae42-236982ff3494@users.sourceforge.net>
@ 2018-05-17 20:21 ` Julia Lawall
  2018-05-17 20:31   ` Julia Lawall
  2 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-17 20:21 UTC (permalink / raw)
  To: cocci



On Wed, 16 May 2018, Jerome Glisse wrote:

> When operating on too many files the exists keyword no longer work. For
> instance:

It has nothing to do with the number of files.  The problem is that some
property of one file causes a parse error in another file, due to the
various heuristics.  You can see this with the --verbose-parsing option.
I'm not yet sure where the bad interaction comes from, but I get the same
bad behavior with the command line:

spatch --verbose-parsing --no-includes --sp-file test.cocci fs/ceph/addr.c
fs/cifs/cifssmb.c fs/f2fs/gc.c

julia

>
> @exists@
> expression E1;
> @@
> set_page_dirty(
> +NULL,
> E1)
>
> Using linux kernel as playground and looking at at fs/f2fs/gc.c where
> there is set_page_dirty() calls in nested blocks.
>
> Works:
>
> spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/!
 char/oradax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/f2fs/gc.c > toto
>
>
> Don't works:
>
> spatch --no-includes --sp-file test.spatch arch/powerpc/kvm/book3s_64_mmu_radix.c arch/powerpc/kvm/e500_mmu.c arch/s390/kvm/interrupt.c arch/x86/kvm/svm.c block/bio.c drivers/block/zram/zram_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c drivers/gpu/drm/drm_gem.c drivers/gpu/drm/exynos/exynos_drm_g2d.c drivers/gpu/drm/i915/i915_gem.c drivers/gpu/drm/i915/i915_gem_fence_reg.c drivers/gpu/drm/i915/i915_gem_userptr.c drivers/gpu/drm/radeon/radeon_ttm.c drivers/gpu/drm/ttm/ttm_tt.c drivers/infiniband/core/umem.c drivers/infiniband/core/umem_odp.c drivers/infiniband/hw/hfi1/user_pages.c drivers/infiniband/hw/qib/qib_user_pages.c drivers/infiniband/hw/usnic/usnic_uiom.c drivers/media/common/videobuf2/videobuf2-dma-contig.c drivers/media/common/videobuf2/videobuf2-dma-sg.c drivers/media/common/videobuf2/videobuf2-vmalloc.c drivers/misc/genwqe/card_utils.c drivers/misc/vmw_vmci/vmci_queue_pair.c drivers/mtd/devices/block2mtd.c drivers/platform/goldfish/goldfish_pipe.c drivers/sbus/!
 char/oradax.c drivers/staging/lustre/lustre/llite/rw26.c drivers/staging/lustre/lustre/llite/vvp_io.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c drivers/vhost/vhost.c drivers/video/fbdev/core/fb_defio.c fs/9p/vfs_addr.c fs/afs/dir.c fs/afs/file.c --include fs/afs/internal.h fs/afs/write.c fs/aio.c fs/block_dev.c fs/btrfs/disk-io.c fs/btrfs/extent_io.c fs/btrfs/file.c fs/btrfs/inode.c fs/btrfs/ioctl.c fs/btrfs/relocation.c fs/buffer.c fs/ceph/addr.c fs/cifs/cifssmb.c fs/f2fs/gc.c > toto
>
>
> Exact same command line except the latter have 49 files before
> fs/f2fs/gc.c while the former has 48 (and it works then). Note
> that it seems that running the script multiple times even when
> they are too many files will eventualy work.
>
> Cheers,
> J?r?me
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-17 20:21 ` [Cocci] [bug] exists do not work if file group is too big (>49) Julia Lawall
@ 2018-05-17 20:31   ` Julia Lawall
  2018-05-17 21:04     ` Jerome Glisse
  0 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-17 20:31 UTC (permalink / raw)
  To: cocci

Concretely, the problem is with LOCK_REQ, which is used as a typedef name
in cifs and as a enum constant in f2fs.

Would there be any chance of getting rid of the LOCK_REQ typedef?
Typedefs for structure types are discouraged by the Linux kernel coding
style.

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] exists do not work if file group is too big
  2018-05-17 14:45   ` [Cocci] exists do not work if file group is too big Jerome Glisse
@ 2018-05-17 20:45     ` Julia Lawall
  2018-05-17 21:00       ` Jerome Glisse
       [not found]       ` <d1902859-7b9b-4dea-0e5b-8d4876f90495@users.sourceforge.net>
  0 siblings, 2 replies; 21+ messages in thread
From: Julia Lawall @ 2018-05-17 20:45 UTC (permalink / raw)
  To: cocci

In terms of the running time, I get a running time of 11 seconds with the
command line of 48 files and I get a running time of 22 seconds if I just
run spatch on the entire kernel.  That seems slower, but if I use the
command line of 48 files, I get changes in 27 files, while if I run it on
the entire kernel, I get changes in 75 files.  If I run it on the entire
kernel using 40 cores (-j 40), I get changes in 75 files in 1.7 seconds.

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] exists do not work if file group is too big
  2018-05-17 20:45     ` Julia Lawall
@ 2018-05-17 21:00       ` Jerome Glisse
  2018-05-17 21:11         ` Julia Lawall
       [not found]       ` <d1902859-7b9b-4dea-0e5b-8d4876f90495@users.sourceforge.net>
  1 sibling, 1 reply; 21+ messages in thread
From: Jerome Glisse @ 2018-05-17 21:00 UTC (permalink / raw)
  To: cocci

On Thu, May 17, 2018 at 10:45:29PM +0200, Julia Lawall wrote:
> In terms of the running time, I get a running time of 11 seconds with the
> command line of 48 files and I get a running time of 22 seconds if I just
> run spatch on the entire kernel.  That seems slower, but if I use the
> command line of 48 files, I get changes in 27 files, while if I run it on
> the entire kernel, I get changes in 75 files.  If I run it on the entire
> kernel using 40 cores (-j 40), I get changes in 75 files in 1.7 seconds.

In this simple test case this would work but in my real spatch i have to
change one function at a time as otherwise there is conflict on header
update. Moreover i also have to use --recursive-headers if i don't do the
git grep to provide file list which is also one of the reasons why it
was much slower.

Ideally if header files could be updated --in-place even when there is
conflict i might first add proper #include as a first step to avoid the
recursive flag.


As an example if i want to add a new argument to both:

int set_page_dirty(struct page *page);
int set_page_dirty_lock(struct page *page);

then because inside include/linux/mm.h they are declare one after the
other --in-place will fails. This case is not the worse, i can split
this in 2 spatches and it would work. I can't do that when updating
callback which have same pattern ie 2 callback prototype declare one
after the other.

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] [bug] exists do not work if file group is too big (>49)
  2018-05-17 20:31   ` Julia Lawall
@ 2018-05-17 21:04     ` Jerome Glisse
  0 siblings, 0 replies; 21+ messages in thread
From: Jerome Glisse @ 2018-05-17 21:04 UTC (permalink / raw)
  To: cocci

On Thu, May 17, 2018 at 10:31:52PM +0200, Julia Lawall wrote:
> Concretely, the problem is with LOCK_REQ, which is used as a typedef name
> in cifs and as a enum constant in f2fs.
> 
> Would there be any chance of getting rid of the LOCK_REQ typedef?
> Typedefs for structure types are discouraged by the Linux kernel coding
> style.
> 
> julia

I will try to make patches for that, thought not anytime soon i am going
on PTO so not before i get back.

Thank you for debugging this further.

Cheers,
J?r?me

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] exists do not work if file group is too big
  2018-05-17 21:00       ` Jerome Glisse
@ 2018-05-17 21:11         ` Julia Lawall
  2018-05-17 21:32           ` Jerome Glisse
  0 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-17 21:11 UTC (permalink / raw)
  To: cocci



On Thu, 17 May 2018, Jerome Glisse wrote:

> On Thu, May 17, 2018 at 10:45:29PM +0200, Julia Lawall wrote:
> > In terms of the running time, I get a running time of 11 seconds with the
> > command line of 48 files and I get a running time of 22 seconds if I just
> > run spatch on the entire kernel.  That seems slower, but if I use the
> > command line of 48 files, I get changes in 27 files, while if I run it on
> > the entire kernel, I get changes in 75 files.  If I run it on the entire
> > kernel using 40 cores (-j 40), I get changes in 75 files in 1.7 seconds.
>
> In this simple test case this would work but in my real spatch i have to
> change one function at a time as otherwise there is conflict on header
> update. Moreover i also have to use --recursive-headers if i don't do the
> git grep to provide file list which is also one of the reasons why it
> was much slower.
>
> Ideally if header files could be updated --in-place even when there is
> conflict i might first add proper #include as a first step to avoid the
> recursive flag.

I'm not sure to understand any of the above.  At the moment you have
--no-includes, so no headers are consulted.  What do you need from header
files?  Do you need type information?  Do you need to change the files?
Do the changes in the header files interact with the changes in the .c
file?

If it just happens that there are also calls to set_page_dirty in the
header files that you want to update, then you can give the options
--no-includes --include-headers.

If the only thing you want from the header files is type information, then
you can say eg --recursive-includes --include-headers-for-types.  That
would be much faster, because the transformation rules wouldn't be applied
over and over to the header files.

If you want to do both, the above options can be combined.

>
>
> As an example if i want to add a new argument to both:
>
> int set_page_dirty(struct page *page);
> int set_page_dirty_lock(struct page *page);
>
> then because inside include/linux/mm.h they are declare one after the
> other --in-place will fails. This case is not the worse, i can split
> this in 2 spatches and it would work. I can't do that when updating
> callback which have same pattern ie 2 callback prototype declare one
> after the other.

Do you actually need to automate this part?

I think I could be more helpful if I had an example that more competely
shows what you want to do.

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] exists do not work if file group is too big
  2018-05-17 21:11         ` Julia Lawall
@ 2018-05-17 21:32           ` Jerome Glisse
  0 siblings, 0 replies; 21+ messages in thread
From: Jerome Glisse @ 2018-05-17 21:32 UTC (permalink / raw)
  To: cocci

On Thu, May 17, 2018 at 11:11:12PM +0200, Julia Lawall wrote:
> 
> 
> On Thu, 17 May 2018, Jerome Glisse wrote:
> 
> > On Thu, May 17, 2018 at 10:45:29PM +0200, Julia Lawall wrote:
> > > In terms of the running time, I get a running time of 11 seconds with the
> > > command line of 48 files and I get a running time of 22 seconds if I just
> > > run spatch on the entire kernel.  That seems slower, but if I use the
> > > command line of 48 files, I get changes in 27 files, while if I run it on
> > > the entire kernel, I get changes in 75 files.  If I run it on the entire
> > > kernel using 40 cores (-j 40), I get changes in 75 files in 1.7 seconds.
> >
> > In this simple test case this would work but in my real spatch i have to
> > change one function at a time as otherwise there is conflict on header
> > update. Moreover i also have to use --recursive-headers if i don't do the
> > git grep to provide file list which is also one of the reasons why it
> > was much slower.
> >
> > Ideally if header files could be updated --in-place even when there is
> > conflict i might first add proper #include as a first step to avoid the
> > recursive flag.
> 
> I'm not sure to understand any of the above.  At the moment you have
> --no-includes, so no headers are consulted.  What do you need from header
> files?  Do you need type information?  Do you need to change the files?
> Do the changes in the header files interact with the changes in the .c
> file?
> 
> If it just happens that there are also calls to set_page_dirty in the
> header files that you want to update, then you can give the options
> --no-includes --include-headers.
> 
> If the only thing you want from the header files is type information, then
> you can say eg --recursive-includes --include-headers-for-types.  That
> would be much faster, because the transformation rules wouldn't be applied
> over and over to the header files.
> 
> If you want to do both, the above options can be combined.

I need to update function prototype and one semantic patch might update
multiples function and those function might have function prototype in
same header files just couple lines of each others in which case this does
not work. This is an issue mostly when updating callback for which i do
not know the function name at time of writing the semantic patch.

The recursive is needed because in many intance the function prototype is
define in a header file that is not directly included by the c file but is
included through another include or a complex include chains.


> >
> >
> > As an example if i want to add a new argument to both:
> >
> > int set_page_dirty(struct page *page);
> > int set_page_dirty_lock(struct page *page);
> >
> > then because inside include/linux/mm.h they are declare one after the
> > other --in-place will fails. This case is not the worse, i can split
> > this in 2 spatches and it would work. I can't do that when updating
> > callback which have same pattern ie 2 callback prototype declare one
> > after the other.
> 
> Do you actually need to automate this part?
> 
> I think I could be more helpful if I had an example that more competely
> shows what you want to do.


Below is an actual example, i run it with:

spatch  --dir . --include-headers -D grep --sp-file spatch

cat /tmp/unicorn-functions | while read line ; do git grep -l \
$line -- '*.[ch]' | sed -e "s/\(.\+\)\.h/--include \1.h/g" \
> /tmp/unicorn-group ; echo "--include include/linux/fs.h" \
>> /tmp/unicorn-group ; echo "--include include/linux/mm.h" \
>> /tmp/unicorn-group ; cat /tmp/unicorn-group | tr '\n' ' ' ; \
echo ; done > /tmp/unicorn-groups

cat /tmp/unicorn-groups | while read line ; do spatch --in-place \
--no-includes --sp-file spatch $line ; done


spatch:

// First part of this file is only executed when -D grep argument is
// provided to spatch. It use to grep all function name that needs to
// be modified. Collected function name are written into a temporary
// file (/tmp/unicorn-functions) and git grep is use to find all the
// files that will be modified using second part of this semantic
// patch
virtual grep

// initialize file where we collect all function name (erase it)
@initialize:python depends on grep@
@@
file=open('/tmp/unicorn-functions', 'w')
file.close()

// match function name use as a callback
@G1 depends on grep@
identifier I1, FN;
@@
struct address_space_operations I1 = {..., .set_page_dirty = FN, ...};

@script:python depends on G1@
funcname << G1.FN;
@@
if funcname != "NULL":
  file=open('/tmp/unicorn-functions', 'a')
  file.write(funcname + '\n')
  file.close()

// Also collect all function name that use a_ops->set_page_dirty()
@G2 depends on grep exists@
expression E1, E2;
identifier FN;
type T1;
@@
T1 FN(...) { ...
(
E1.a_ops->set_page_dirty(E2)
|
E1->a_ops->set_page_dirty(E2)
)
... }

@script:python depends on G2@
funcname << G2.FN;
@@
file=open('/tmp/unicorn-functions', 'a')
file.write(funcname + '\n')
print(funcname)
file.close()


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

// Below is the actual modification. We use the fn argument (provided
// by passing -D fn=value to spatch). To identify which function is
// being modified in this run. Semantic patch is run once per function
// because of conflict when updating multiple functions in one run.

// Update the address_space_operations structure (should happens only
// once with the first group of files)
@depends on !grep@
@@
struct address_space_operations { ... int (*set_page_dirty)(
+struct address_space *,
struct page *page); ... };

// Update caller of address_space_operation.set_page_dirty (pass NULL)
@depends on !grep@
expression E1, E2;
@@
E1.a_ops->set_page_dirty(
+MAPPING_NULL,
E2)

@depends on !grep@
expression E1, E2;
@@
E1->a_ops->set_page_dirty(
+MAPPING_NULL,
E2)

// Find function name use for the set_page_dirty callback (does not change
// anything, this just find the name)
@R1 depends on !grep exists@
identifier I1, virtual.fn;
@@
struct address_space_operations I1 = {..., .set_page_dirty = fn ,...};

// Add address_space argument to the function (set_page_dirty callback one)
@depends on R1@
type T1;
identifier I1;
identifier virtual.fn;
@@
int fn(
+struct address_space *__mapping,
T1 I1) { ... }

// Modify every place the function (use for set_page_dirty callback) is call
// to pass NULL as for the new address_space argument.
@depends on R1@
identifier virtual.fn;
expression E1;
@@
fn(
+MAPPING_NULL,
E1)

// Modify local variable use to store pointer to callback
@R2 depends on !grep exists@
expression E1;
identifier I1;
@@
int (*I1)(
+struct address_space *,
struct page *) = E1->a_ops->set_page_dirty;

@depends on R2 exists@
expression E1;
identifier R2.I1;
@@
-(*I1)(E1)
+I1(MAPPING_NULL, E1)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] Checking run times for transformation of Linux source code with SmPL
       [not found]       ` <d1902859-7b9b-4dea-0e5b-8d4876f90495@users.sourceforge.net>
@ 2018-05-18 16:03         ` Julia Lawall
       [not found]           ` <416a7972-d6ce-38b2-9983-ce0446b5ab61@users.sourceforge.net>
  0 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-18 16:03 UTC (permalink / raw)
  To: cocci



On Fri, 18 May 2018, SF Markus Elfring wrote:

> > In terms of the running time, I get a running time of 11 seconds with the
> > command line of 48 files and I get a running time of 22 seconds if I just
> > run spatch on the entire kernel.
>
> * Can such a comparison result be amazing?

No, I don't see anything suprising about it.  More work takes more time.

>
> * How do you think about share any more information about the concrete
>   test environment?
>
>
> > That seems slower, but if I use the command line of 48 files, I get changes
> > in 27 files, while if I run it on the entire kernel, I get changes in 75 files.
>
> Did this try work without the command parameter ?--file-groups??

Yes.  --file-groups is not needed here.

>
>
> > If I run it on the entire kernel using 40 cores (-j 40), I get changes
> > in 75 files in 1.7 seconds.
>
> Have you got access to computation resources which are so powerful?

Yes.

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] Checking run times for transformation of Linux source code with SmPL
       [not found]           ` <416a7972-d6ce-38b2-9983-ce0446b5ab61@users.sourceforge.net>
@ 2018-05-18 16:49             ` Julia Lawall
       [not found]               ` <f9b0c961-7182-86ea-bb39-486ffc732db0@users.sourceforge.net>
  0 siblings, 1 reply; 21+ messages in thread
From: Julia Lawall @ 2018-05-18 16:49 UTC (permalink / raw)
  To: cocci



On Fri, 18 May 2018, SF Markus Elfring wrote:

> >>> In terms of the running time, I get a running time of 11 seconds with the
> >>> command line of 48 files and I get a running time of 22 seconds if I just
> >>> run spatch on the entire kernel.
> >>
> >> * Can such a comparison result be amazing?
> >
> > No, I don't see anything suprising about it.
>
> I suggest to take another look at these performance numbers from your
> powerful test environment. All Linux source files were checked to some degree
> according to another specific search pattern.
> The half of this execution time was taken by only a tiny fraction
> of update candidates.

The 48 files are all parsed.  Coccinelle doesn't parse every file in the
kernel.  It only parses the files that contain the name of the function
being modified.

julia

>
>
> > More work takes more time.
>
> This usual in principle. It seems that I got a bit of understanding difficulties
> for this simple test report.
> How reasonable is it that a lot of more work could be performed somehow
> in the double time range?
>
> Regards,
> Markus
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Cocci] Checking run times for transformation of Linux source code with SmPL
       [not found]               ` <f9b0c961-7182-86ea-bb39-486ffc732db0@users.sourceforge.net>
@ 2018-05-18 17:13                 ` Julia Lawall
  0 siblings, 0 replies; 21+ messages in thread
From: Julia Lawall @ 2018-05-18 17:13 UTC (permalink / raw)
  To: cocci



On Fri, 18 May 2018, SF Markus Elfring wrote:

> > Coccinelle doesn't parse every file in the kernel.
>
> I find this information strange.
>
>
> > It only parses the files that contain the name of the function being modified.
>
> Do you interpret the required source code parsing efforts in a special way?

Coccinelle searches the semantic patch for the important words.  Then it
implements something like grep to find files that contain those imprtant
words.  Or maybe it just uses git grep by default.

julia

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-05-18 17:13 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16 19:16 [Cocci] [bug] exists do not work if file group is too big (>49) Jerome Glisse
2018-05-16 19:37 ` Julia Lawall
2018-05-16 19:54   ` Jerome Glisse
2018-05-16 20:02     ` Julia Lawall
2018-05-16 20:15       ` Jerome Glisse
2018-05-16 20:20         ` Julia Lawall
2018-05-16 20:24           ` Jerome Glisse
2018-05-16 20:29             ` Julia Lawall
2018-05-16 20:35               ` Jerome Glisse
     [not found]         ` <10d2d384-95c0-dec1-b284-f5afb7d9ce81@users.sourceforge.net>
2018-05-17 14:50           ` [Cocci] Checking consequences from “exists” usage with big file name selection Jerome Glisse
     [not found] ` <128b6146-5368-12bb-ae42-236982ff3494@users.sourceforge.net>
2018-05-17 14:45   ` [Cocci] exists do not work if file group is too big Jerome Glisse
2018-05-17 20:45     ` Julia Lawall
2018-05-17 21:00       ` Jerome Glisse
2018-05-17 21:11         ` Julia Lawall
2018-05-17 21:32           ` Jerome Glisse
     [not found]       ` <d1902859-7b9b-4dea-0e5b-8d4876f90495@users.sourceforge.net>
2018-05-18 16:03         ` [Cocci] Checking run times for transformation of Linux source code with SmPL Julia Lawall
     [not found]           ` <416a7972-d6ce-38b2-9983-ce0446b5ab61@users.sourceforge.net>
2018-05-18 16:49             ` Julia Lawall
     [not found]               ` <f9b0c961-7182-86ea-bb39-486ffc732db0@users.sourceforge.net>
2018-05-18 17:13                 ` Julia Lawall
2018-05-17 20:21 ` [Cocci] [bug] exists do not work if file group is too big (>49) Julia Lawall
2018-05-17 20:31   ` Julia Lawall
2018-05-17 21:04     ` Jerome Glisse

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.