Re: [Cocci] [PATCH v3 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle

Re: [Cocci] [PATCH v3 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle

[Markus Elfring]

> Convert the last few remaining mmap_sem rwsem calls to use the new
> mmap locking API. These were missed by coccinelle for some reason

Will the clarification of this software situation become more interesting?


> (I think coccinelle does not support some of the preprocessor
> constructs in these files ?)

I suggest to omit this information from the final change description.
Would you like to help any more to find nicer solutions
for remaining open issues?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v3 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle

[Michel Lespinasse]

Hi Markus,

On Sat, Mar 28, 2020 at 12:37 AM Markus Elfring <Markus.Elfring@web.de> wrote:
> > Convert the last few remaining mmap_sem rwsem calls to use the new
> > mmap locking API. These were missed by coccinelle for some reason
>
> Will the clarification of this software situation become more interesting?
>
> > (I think coccinelle does not support some of the preprocessor
> > constructs in these files ?)
>
> I suggest to omit this information from the final change description.
> Would you like to help any more to find nicer solutions
> for remaining open issues?

So, from a practical perspective I think coccinelle has filled its
purpose for me - it got 99% of the job done, and I had to do the last
1% by hand which is not ideal, but really not too bad either. Also, by
using coccinelle I think reviewers can appreciate that the
change is purely mechanical, and reproduce it on their end if needed,
which facilitates the review process greatly.

I would be interested to find out why coccinelle wasn't able to do the
last 1%, but only as part of a long-term learning process on getting
better with coccinelle - I don't consider it a blocker for short-term
progress on this patchset.

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle

[Markus Elfring]

> I would be interested to find out why coccinelle wasn't able to do the
> last 1%, but only as part of a long-term learning process on getting
> better with coccinelle - …

How will corresponding software development resources evolve?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle

[Michel Lespinasse]

On Sat, Mar 28, 2020 at 1:39 AM Markus Elfring <Markus.Elfring@web.de> wrote:
> > I would be interested to find out why coccinelle wasn't able to do the
> > last 1%, but only as part of a long-term learning process on getting
> > better with coccinelle - …
>
> How will corresponding software development resources evolve?

I don't think I understand the question, or, actually, are you asking
me or the coccinelle developers ?

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: convert mmap_sem call sites missed by coccinelle

[Markus Elfring]

>>> I would be interested to find out why coccinelle wasn't able to do the
>>> last 1%, but only as part of a long-term learning process on getting
>>> better with coccinelle - …
>>
>> How will corresponding software development resources evolve?
>
> I don't think I understand the question,

Various aspects can be reconsidered.


> or, actually, are you asking me or the coccinelle developers ?

I am curious how much the change interests will grow by involved
contributors and organisations.
Would you like to discuss affected implementation details any further?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Checking the Coccinelle software

[Markus Elfring]

>> How will corresponding software development resources evolve?
>
> I don't think I understand the question, or, actually, are you asking
> me or the coccinelle developers ?

I hope that another communication approach can eventually increase
the chances for a better common understanding of development challenges.

The code from a mentioned source file can be reduced to the following
test file.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/mm/fault.c?id=69c5eea3128e775fd3c70ecf0098105d96dee330#n34

// deleted part
static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
	unsigned long address)
{
	struct vm_area_struct * vma = NULL;
	struct task_struct *tsk = current;
	struct mm_struct *mm = tsk->mm;
// deleted part
retry:
	down_read(&mm->mmap_sem);
	vma = find_vma(mm, address);
	if (!vma)
		goto bad_area;
// deleted part
}
// deleted part


Application of the software “Coccinelle 1.0.8-00029-ga549b9f0” (OCaml 4.10.0)

elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c do_page_fault-excerpt3.c
…
NB total files = 1; perfect = 1; pbs = 0; timeout = 0; =========> 100%
nb good = 15,  nb passed = 1 =========> 6.25% passed
nb good = 15,  nb bad = 0 =========> 100.00% good or passed


The discussed transformation approach can also be reduced for a test
to the following script for the semantic patch language.

@replacement@
expression x;
@@
-down_read
+mmap_read_lock
 (
- &
  x
- ->mmap_sem
 )


elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch use_mmap_locking_API_3.cocci do_page_fault-excerpt3.c


The desired diff is not generated so far.
How would you like to fix this situation?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Checking the Coccinelle software

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]



On Sat, 28 Mar 2020, Markus Elfring wrote:

> >> How will corresponding software development resources evolve?
> >
> > I don't think I understand the question, or, actually, are you asking
> > me or the coccinelle developers ?
>
> I hope that another communication approach can eventually increase
> the chances for a better common understanding of development challenges.
>
> The code from a mentioned source file can be reduced to the following
> test file.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/mm/fault.c?id=69c5eea3128e775fd3c70ecf0098105d96dee330#n34
>
> // deleted part
> static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
> 	unsigned long address)
> {
> 	struct vm_area_struct * vma = NULL;
> 	struct task_struct *tsk = current;
> 	struct mm_struct *mm = tsk->mm;
> // deleted part
> retry:
> 	down_read(&mm->mmap_sem);
> 	vma = find_vma(mm, address);
> 	if (!vma)
> 		goto bad_area;
> // deleted part
> }
> // deleted part
>
>
> Application of the software “Coccinelle 1.0.8-00029-ga549b9f0” (OCaml 4.10.0)
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c do_page_fault-excerpt3.c
> …
> NB total files = 1; perfect = 1; pbs = 0; timeout = 0; =========> 100%
> nb good = 15,  nb passed = 1 =========> 6.25% passed
> nb good = 15,  nb bad = 0 =========> 100.00% good or passed
>
>
> The discussed transformation approach can also be reduced for a test
> to the following script for the semantic patch language.
>
> @replacement@
> expression x;
> @@
> -down_read
> +mmap_read_lock
>  (
> - &
>   x
> - ->mmap_sem
>  )
>
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch use_mmap_locking_API_3.cocci do_page_fault-excerpt3.c
>
>
> The desired diff is not generated so far.
> How would you like to fix this situation?

Who exactly do you think "you" is?  I will look at it, but it is not very
polite to ask a user of Coccinelle such a question.

julia

>
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Checking the Coccinelle software

[Markus Elfring]

>> How would you like to fix this situation?
>
> Who exactly do you think "you" is?

Every contributor with helpful software development resources for this issue.


> I will look at it,

Thanks for another promising feedback.


> but it is not very polite to ask a user of Coccinelle such a question.

We come along different views for the handling of such a bug report.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Checking the Coccinelle software

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]



On Sat, 28 Mar 2020, Markus Elfring wrote:

> >> How will corresponding software development resources evolve?
> >
> > I don't think I understand the question, or, actually, are you asking
> > me or the coccinelle developers ?
>
> I hope that another communication approach can eventually increase
> the chances for a better common understanding of development challenges.
>
> The code from a mentioned source file can be reduced to the following
> test file.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/mm/fault.c?id=69c5eea3128e775fd3c70ecf0098105d96dee330#n34
>
> // deleted part
> static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
> 	unsigned long address)
> {
> 	struct vm_area_struct * vma = NULL;
> 	struct task_struct *tsk = current;
> 	struct mm_struct *mm = tsk->mm;
> // deleted part
> retry:
> 	down_read(&mm->mmap_sem);
> 	vma = find_vma(mm, address);
> 	if (!vma)
> 		goto bad_area;
> // deleted part
> }
> // deleted part
>
>
> Application of the software “Coccinelle 1.0.8-00029-ga549b9f0” (OCaml 4.10.0)
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c do_page_fault-excerpt3.c
> …
> NB total files = 1; perfect = 1; pbs = 0; timeout = 0; =========> 100%
> nb good = 15,  nb passed = 1 =========> 6.25% passed
> nb good = 15,  nb bad = 0 =========> 100.00% good or passed
>
>
> The discussed transformation approach can also be reduced for a test
> to the following script for the semantic patch language.
>
> @replacement@
> expression x;
> @@
> -down_read
> +mmap_read_lock
>  (
> - &
>   x
> - ->mmap_sem
>  )
>
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch use_mmap_locking_API_3.cocci do_page_fault-excerpt3.c
>
>
> The desired diff is not generated so far.
> How would you like to fix this situation?

The problem is due to a preceding goto where the destination is expressed
as a macro name.  Maybe there should be a warning in this case.

julia

>
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Checking the Coccinelle software

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

> // deleted part
> retry:
> 	down_read(&mm->mmap_sem);
> 	vma = find_vma(mm, address);
> 	if (!vma)
> 		goto bad_area;
> // deleted part
> }
> // deleted part
>
>
> Application of the software “Coccinelle 1.0.8-00029-ga549b9f0” (OCaml 4.10.0)
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c do_page_fault-excerpt3.c
> …
> NB total files = 1; perfect = 1; pbs = 0; timeout = 0; =========> 100%
> nb good = 15,  nb passed = 1 =========> 6.25% passed
> nb good = 15,  nb bad = 0 =========> 100.00% good or passed
>
>
> The discussed transformation approach can also be reduced for a test
> to the following script for the semantic patch language.
>
> @replacement@
> expression x;
> @@
> -down_read
> +mmap_read_lock
>  (
> - &
>   x
> - ->mmap_sem
>  )
>
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch use_mmap_locking_API_3.cocci do_page_fault-excerpt3.c
>
>
> The desired diff is not generated so far.
> How would you like to fix this situation?

The problem can be seen with the --debug option:

FLOW: can't jump to VMALLOC_FAULT_TARGET: because we can't find this label

It's not apparent with the --parse-c option because it's not a parsing
problem.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Checking the Coccinelle software

[Markus Elfring]

> The problem can be seen with the --debug option:
>
> FLOW: can't jump to VMALLOC_FAULT_TARGET: because we can't find this label
>
> It's not apparent with the --parse-c option because it's not a parsing problem.

Thanks for such information.

Can the example be transformed even if extra source code was intentionally deleted
for the easier clarification of the shown software test?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Improving the Coccinelle software

[Markus Elfring]

>> How will corresponding software development resources evolve?
>
> I don't think I understand the question, or, actually, are you asking
> me or the coccinelle developers ?

I hope that more development challenges will be picked up.

The code from a mentioned source file can be reduced to the following
test file.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/mmu/paging_tmpl.h?id=7111951b8d4973bda27ff663f2cf18b663d15b48#n122

// deleted part
static inline int FNAME(is_present_gpte)(unsigned long pte)
{
#if PTTYPE != PTTYPE_EPT
	return pte & PT_PRESENT_MASK;
#else
	return pte & 7;
#endif
}
// deleted part


Application of the software “Coccinelle 1.0.8-00029-ga549b9f0” (OCaml 4.10.0)

elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c paging_tmpl-excerpt1.h
…
(ONCE) CPP-MACRO: found known macro = FNAME
…
parse error
 = File "paging_tmpl-excerpt1.h", line 2, column 41, charpos = 57
  around = 'unsigned',
…
BAD:!!!!! static inline int FNAME(is_present_gpte)(unsigned long pte)
…
NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
nb good = 1,  nb passed = 1 =========> 10.00% passed
nb good = 1,  nb bad = 8 =========> 20.00% good or passed


How would you like to improve the affected software areas?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Improving the Coccinelle software

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]



On Mon, 30 Mar 2020, Markus Elfring wrote:

> >> How will corresponding software development resources evolve?
> >
> > I don't think I understand the question, or, actually, are you asking
> > me or the coccinelle developers ?
>
> I hope that more development challenges will be picked up.
>
> The code from a mentioned source file can be reduced to the following
> test file.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/mmu/paging_tmpl.h?id=7111951b8d4973bda27ff663f2cf18b663d15b48#n122
>
> // deleted part
> static inline int FNAME(is_present_gpte)(unsigned long pte)
> {
> #if PTTYPE != PTTYPE_EPT
> 	return pte & PT_PRESENT_MASK;
> #else
> 	return pte & 7;
> #endif
> }
> // deleted part
>
>
> Application of the software “Coccinelle 1.0.8-00029-ga549b9f0” (OCaml 4.10.0)
>
> elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch --parse-c paging_tmpl-excerpt1.h
> …
> (ONCE) CPP-MACRO: found known macro = FNAME
> …
> parse error
>  = File "paging_tmpl-excerpt1.h", line 2, column 41, charpos = 57
>   around = 'unsigned',
> …
> BAD:!!!!! static inline int FNAME(is_present_gpte)(unsigned long pte)
> …
> NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
> nb good = 1,  nb passed = 1 =========> 10.00% passed
> nb good = 1,  nb bad = 8 =========> 20.00% good or passed
>
>
> How would you like to improve the affected software areas?

This can be addressed by adding a macro definition to standard.h.

But once the change is done, I don't see any reason to bother with this.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Improving the Coccinelle software

[Markus Elfring]

>> …
>> (ONCE) CPP-MACRO: found known macro = FNAME
>> …
>> parse error
…
>> How would you like to improve the affected software areas?
>
> This can be addressed by adding a macro definition to standard.h.

A corresponding specification is used already.
https://github.com/coccinelle/coccinelle/blob/fdf0b68ddd0a518cc6ca64f063bc74ed54e29a7b/standard.h#L508


> But once the change is done, I don't see any reason to bother with this.

How will the support evolve for data processing around the application
of similar macros?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Improving the Coccinelle software

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]



On Mon, 30 Mar 2020, Markus Elfring wrote:

> >> …
> >> (ONCE) CPP-MACRO: found known macro = FNAME
> >> …
> >> parse error
> …
> >> How would you like to improve the affected software areas?
> >
> > This can be addressed by adding a macro definition to standard.h.
>
> A corresponding specification is used already.
> https://github.com/coccinelle/coccinelle/blob/fdf0b68ddd0a518cc6ca64f063bc74ed54e29a7b/standard.h#L508
>
>
> > But once the change is done, I don't see any reason to bother with this.
>
> How will the support evolve for data processing around the application
> of similar macros?

Not at all.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [v3 05/10] mmap locking API: Improving the Coccinelle software

[Markus Elfring]

>> How will the support evolve for data processing around the application
>> of similar macros?
>
> Not at all.

Will such feedback trigger any collateral evolution?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci