[cocci] Need some help with spatch

[cocci] Need some help with spatch

[Vlastimil Babka]

Hi all,

I'm working on a patchset originally by Matthew Wilcox that replaces "struct
page" with "struct slab" in slab allocators [1].
Because a lot of the changes there are mechanistic, I thought it would be a
good idea to perform them via coccinelle semantic patch. But it's my first
attempt at large real usage of spatch, and it probably shows.

Overal I have managed to do what I wanted, but encountered some issues that
required workarounds so I will list them here and would welcome any help. I
assume most are from my inexperience and it's possible to adjust the
semantic patches so the workarounds are not needed.

The current development snapshot of the series is at [2], git clone url [3]
branch slab-struct_slab-v1r8

There are 3 semantic patches used in the series. For easier development, the
respective commits add a .cocci file in addition to applying it (when
finalized, the .cocci content would be moved to changelog as usual):

184d9494c1a4 ("mm/slub: mass conversion of struct page to struct slab by
spatch") adds slub.cocci

ccd7da3f3888 ("mm/slab: mass conversion of struct page to struct slab by
spatch") adds slab.cocci

0c60a4fcc1b2 ("mm: spatch conversion of struct page to struct slab for
subsystems interacting with slab") adds slab_shared.cocci

So here are the issues I encountered:

1) patching complex variable declarations;

I need to perform changes such as:
-struct page *page;
+struct slab *slab;

This I was able to somehow specify in the .cocci, but not for situations
where the original declarations looks like this:

struct page *page, *page2;

or

struct page *page = NULL;

Therefore, for 2 of the 3 semantic patches I have a "prerequisite" commits
that change those declarations to split multiple variables or initial assinment:
f15698e1c920 ("mm/slub: prepare for struct patch conversion by spatch")
8a685ebb3b2a ("mm/slab: prepare for struct patch conversion by spatch")

It would be nice to eliminate those, if possible, but I don't know how
to adjust the respective .cocci for that.

2) for the SLUB cocci patch, some places where the semantic patch should
change something are "missed" even though other similar places are not

It can be seen in this commit that fixes them up:

d12d641854a8 ("mm/slub: fix up places that spatch missed")

for example the cocci patch is supposed to change all "page" to "slab", but
some lines are patched like this:

-       __free_slab(page->slab_cache, page);
+       __free_slab(page->slab_cache, slab);

so it missed the first 'page' and commit d12d641854a8 has to fix that up. I
don't know why. It seems it's mainly places where two different changes
would be applicable to a single line? Maybe the .cocci rules should be
written differently to handle this?

3) eliminated whitespace including newlines

This is stylistic but will require some postprocessing fixups if not solved.
There seem to be several cases like this where whitespace including newlines
is eliminated between two lines that are patched by the spatch:

-       struct page *page;
-
-       page = container_of(h, struct page, rcu_head);
+       struct slab *slab;slab = container_of(h, struct page, rcu_head);

4) unnecessary indentation changes for multiline function declarations or calls

This is the least critical, but maybe there's a way to preserve original
indentation and not do reindenting like this?

-static inline bool cmpxchg_double_slab(struct kmem_cache *s, struct page *page,
-               void *freelist_old, unsigned long counters_old,
-               void *freelist_new, unsigned long counters_new,
-               const char *n)
+static inline bool cmpxchg_double_slab(struct kmem_cache *s,
+                                      struct slab *slab,
+                                      void *freelist_old,
+                                      unsigned long counters_old,
+                                      void *freelist_new,
+                                      unsigned long counters_new,
+                                      const char *n)

Thanks in advance for any hints!

[1] https://lwn.net/Articles/871982/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=slab-struct_slab-v1r8
[3] git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git

Re: [cocci] Need some help with spatch

[Luis Chamberlain]

On Fri, Nov 05, 2021 at 05:12:09PM +0100, Vlastimil Babka wrote:
> 1) patching complex variable declarations;
> 
> I need to perform changes such as:
> -struct page *page;
> +struct slab *slab;
> 
> This I was able to somehow specify in the .cocci, but not for situations
> where the original declarations looks like this:
> 
> struct page *page, *page2;
> 
> or
> 
> struct page *page = NULL;

For this, this seems to work well:

@@
@@

-struct page
+struct slab

Re: [cocci] Need some help with spatch

[Julia Lawall]

> 4) unnecessary indentation changes for multiline function declarations or calls
>
> This is the least critical, but maybe there's a way to preserve original
> indentation and not do reindenting like this?
>
> -static inline bool cmpxchg_double_slab(struct kmem_cache *s, struct page *page,
> -               void *freelist_old, unsigned long counters_old,
> -               void *freelist_new, unsigned long counters_new,
> -               const char *n)
> +static inline bool cmpxchg_double_slab(struct kmem_cache *s,
> +                                      struct slab *slab,
> +                                      void *freelist_old,
> +                                      unsigned long counters_old,
> +                                      void *freelist_new,
> +                                      unsigned long counters_new,
> +                                      const char *n)

You can try the --smpl-spacing argument.  The original idea of that was
that people might like to put eg

+ if ( x )

Without the --smpl-spacing argument, Coccinelle would drop the spaces
around x.  With the argument, the code will be written as is.  But for
your case, it seems that --smpl-spacing also eliminates the reindentation
of function headers and function calls, when one of the
parameters/arguments changes.

julia

Re: [cocci] Need some help with spatch

[Julia Lawall]

> for example the cocci patch is supposed to change all "page" to "slab", but
> some lines are patched like this:
>
> -       __free_slab(page->slab_cache, page);
> +       __free_slab(page->slab_cache, slab);

I assume that here you are using the semantic patch in 184d9494c1a4.  I
get that both occurrences of page are changed.  What version of Coccinelle
do you have?

julia

Re: [cocci] Need some help with spatch

[Vlastimil Babka]

On 11/5/21 22:13, Julia Lawall wrote:
>> for example the cocci patch is supposed to change all "page" to "slab", but
>> some lines are patched like this:
>>
>> -       __free_slab(page->slab_cache, page);
>> +       __free_slab(page->slab_cache, slab);
> 
> I assume that here you are using the semantic patch in 184d9494c1a4.  I

Yes, that's the slub.cocci one from that commit.

> get that both occurrences of page are changed.  What version of Coccinelle
> do you have?

It's 1.1.0 in openSUSE Tumbleweed, so it should be the latest?

But that reminds me, yesterday I adjusted just that rule in the semantic
patch a bit and suddenly it converted almost everything. Today I re-ran
spatch for some rebasing reason and it again needed the fixup. I assumed
there was some git snafu on my part and the fixup was already applied.
But if it works differently for you, I'm not sure now. Is it possible
for the result to be non-deterministic? E.g. some internal state of
coccinelle is based on, say, hash table and in some cases the
non-deterministic order matters?

And just to be sure, does .config matter for coccinelle (through effects
on #ifdefs etc)? I assume it doesn't, if I don't use that switch to
perform preprocessing?

Thanks,
Vlastimil

> julia
> 

Re: [cocci] Need some help with spatch

[Julia Lawall]

On Fri, 5 Nov 2021, Vlastimil Babka wrote:

> On 11/5/21 22:13, Julia Lawall wrote:
> >> for example the cocci patch is supposed to change all "page" to "slab", but
> >> some lines are patched like this:
> >>
> >> -       __free_slab(page->slab_cache, page);
> >> +       __free_slab(page->slab_cache, slab);
> >
> > I assume that here you are using the semantic patch in 184d9494c1a4.  I
>
> Yes, that's the slub.cocci one from that commit.
>
> > get that both occurrences of page are changed.  What version of Coccinelle
> > do you have?
>
> It's 1.1.0 in openSUSE Tumbleweed, so it should be the latest?
>
> But that reminds me, yesterday I adjusted just that rule in the semantic
> patch a bit and suddenly it converted almost everything. Today I re-ran
> spatch for some rebasing reason and it again needed the fixup. I assumed
> there was some git snafu on my part and the fixup was already applied.
> But if it works differently for you, I'm not sure now. Is it possible
> for the result to be non-deterministic? E.g. some internal state of
> coccinelle is based on, say, hash table and in some cases the
> non-deterministic order matters?

I haven't seen that before.

>
> And just to be sure, does .config matter for coccinelle (through effects
> on #ifdefs etc)? I assume it doesn't, if I don't use that switch to
> perform preprocessing?

No, the configuration doesn't matter.

julia

Re: [cocci] Need some help with spatch

[Julia Lawall]

> 3) eliminated whitespace including newlines
>
> This is stylistic but will require some postprocessing fixups if not solved.
> There seem to be several cases like this where whitespace including newlines
> is eliminated between two lines that are patched by the spatch:
>
> -       struct page *page;
> -
> -       page = container_of(h, struct page, rcu_head);
> +       struct slab *slab;slab = container_of(h, struct page, rcu_head);

Which semantic patch causes this problem?

julia

Re: [cocci] Need some help with spatch

[Vlastimil Babka]

On 11/5/21 22:31, Julia Lawall wrote:
>> 3) eliminated whitespace including newlines
>>
>> This is stylistic but will require some postprocessing fixups if not solved.
>> There seem to be several cases like this where whitespace including newlines
>> is eliminated between two lines that are patched by the spatch:
>>
>> -       struct page *page;
>> -
>> -       page = container_of(h, struct page, rcu_head);
>> +       struct slab *slab;slab = container_of(h, struct page, rcu_head);
> 
> Which semantic patch causes this problem?

That's also 184d9494c1a4 slub.cocci

Re: [cocci] Need some help with spatch

[Julia Lawall]

> for example the cocci patch is supposed to change all "page" to "slab", but
> some lines are patched like this:
>
> -       __free_slab(page->slab_cache, page);
> +       __free_slab(page->slab_cache, slab);

OK, I can now reproduce this problem.  I think we both made the mistake of
not taking the code as it was before the 184d9494c1a4 commit.

On the other hand, the dropped newline problem in the same function has
been fixed.  The fix should be in the (slightly premature) recent release
of 1.1.1.

julia

Re: [cocci] Need some help with spatch

[Julia Lawall]

> for example the cocci patch is supposed to change all "page" to "slab", but
> some lines are patched like this:
>
> -       __free_slab(page->slab_cache, page);
> +       __free_slab(page->slab_cache, slab);

In the original definition of rcu_free_slab, the second argument has the
form page_slab(page).  This triggers the first disjunct of:

(
- page_slab(page)
+ slab
|
- compound_order(page)
+ slab_order(slab)
|
- PageSlab(page)
+ PageSlab(slab_page(slab))
|
- page_address(page)
+ slab_address(slab)
|
- page_size(page)
+ slab_size(slab)
|
- page_to_nid(page)
+ slab_nid(slab)
|
- page->pages
+ slab->slabs
|
- page
+ slab
)


Once a disjuntc has been chosen for a particular statement, then the
remaining ones are not taken into account.  So -page + slab is not taken
into account.

I wonder if you could just have one rule to change all occurences of page
to slab, and then have individual rules to change eg page_address(slab) to
slab_address(slab).  Then all the cases should be taken into account.

julia

Re: [cocci] Need some help with spatch

[Julia Lawall]

On Fri, 5 Nov 2021, Luis Chamberlain wrote:

> On Fri, Nov 05, 2021 at 05:12:09PM +0100, Vlastimil Babka wrote:
> > 1) patching complex variable declarations;
> >
> > I need to perform changes such as:
> > -struct page *page;
> > +struct slab *slab;
> >
> > This I was able to somehow specify in the .cocci, but not for situations
> > where the original declarations looks like this:
> >
> > struct page *page, *page2;
> >
> > or
> >
> > struct page *page = NULL;
>
> For this, this seems to work well:
>
> @@
> @@
>
> -struct page
> +struct slab

Agreed.

julia

Re: [cocci] mm/slab: mass conversion of struct page to struct slab by spatch

[Markus Elfring]

> Thanks in advance for any hints!

I have also taken another look at the proposed SmPL script file.
https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/diff/slab.cocci?h=slab-struct_slab-v1r8&id=ccd7da3f3888df4acd82448f861445d4593a9fcb

I have noticed then a few opportunities for a bit of fine-tuning
for the presented code according to the semantic patch language.


@convert_callers@
@@
(
(
-account_slab_page
+account_slab
|
-unaccount_slab_page
+unaccount_slab
)
  (
- page
+ slab
  , ...)
|
(
-PageSlabPfmemalloc
+slab_test_pfmemalloc
|
-ClearPageSlabPfmemalloc
+slab_clear_pfmemalloc
)
  (
- page
+ slab
  )
)

@convert_list_entry@
identifier fn =~ "^list_\w*entry(?:_or_null)?";
@@
-page
+slab
       = fn(...,
-          struct page
+          struct slab
            , ...)

@convert_param_struct_page_ptr@
identifier fn;
expression E;
@@
  fn(...,
-   struct page *page
+   struct slab *slab
     , ...)
  {
  <...
(
-page_node
+slab_node
|
-page_slab(page)
+slab
|
(
-page_address
+slab_address
|
-page_size
+slab_size
|
-page_to_nid
+slab_nid
)
  (
- page
+ slab
  )
|
-virt_to_head_page
+virt_to_slab
  (E)
|
-page
+slab
)
  ...>
  }

@convert_param_page_node@
identifier fn;
expression E;
@@
  fn(...,
     int
-       page_node
+       slab_node
     , ...)
  {
  <...
-page_node
+slab_node
  ...>
  }

@remove_slab_page_wrappers@
expression E;
@@
  index_to_obj(...,
-             slab_page(
                         E
-                      )
+             , ...)

@rename_called_function_parameter@
@@
  TODO(...,
-     page
+     slab
       , ...)

@convert_return_struct_page@
identifier fn =~ "^(?:cache_grow_begin|get_(?:valid_)?first_slab)";
expression E;
@@
  static
-struct page *
+struct slab *
  fn(...)
  {
  <...
-slab_page(
            E
-         )
  ...>
  }

@convert_local_struct_page@
identifier fn !~ "^kmem_(?:get|free)pages", tmp;
expression E;
@@
  fn(...)
  {
  <...
(
-struct page *page
+struct slab *slab
  ;
|
-struct page *
+struct slab *
  tmp;
|
-page_slab(page)
+slab
|
  kasan_poison_slab(
-                  page
+                  slab_page(slab)
                   )
|
(
-page_address
+slab_address
|
-page_size
+slab_size
|
-page_to_nid
+slab_nid
)
  (
- page
+ slab
  )
|
-page->pages
+slab->slabs
|
-page = virt_to_head_page
+slab = virt_to_slab
  (E)
|
-page
+slab
)
  ...>
  }


Regards,
Markus

Re: [cocci] mm/slab: prepare for struct patch conversion by spatch

[Markus Elfring]

> Because a lot of the changes there are mechanistic, I thought it would be a
> good idea to perform them via coccinelle semantic patch. But it's my first
> attempt at large real usage of spatch, and it probably shows.

I guess that your clarification request will trigger further collateral evolution.
Thus I suggest to reconsider also another commit message from your software
development branch accordingly.
https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/commit/?h=slab-struct_slab-v1r8&id=8a685ebb3b2a10c1d781d5b3e6ad0315ce9323d6

“This patch splits several declarations of multiple variables, or declarations
that include initial assignments, to separate lines, due to my inability to
make a coccinelle semantic patch match on those declarations.”

* How do you think about to specify desirable change preparations by the means of
   the semantic patch language?

* Will any guidance help to find nicer solutions?

Regards,
Markus

Re: [cocci] mm/slab: prepare for struct patch conversion by spatch

[Julia Lawall]

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



On Sun, 7 Nov 2021, Markus Elfring wrote:

> > Because a lot of the changes there are mechanistic, I thought it would be a
> > good idea to perform them via coccinelle semantic patch. But it's my first
> > attempt at large real usage of spatch, and it probably shows.
>
> I guess that your clarification request will trigger further collateral
> evolution.
> Thus I suggest to reconsider also another commit message from your software
> development branch accordingly.
> https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/commit/?h=slab-struct_slab-v1r8&id=8a685ebb3b2a10c1d781d5b3e6ad0315ce9323d6
>
> “This patch splits several declarations of multiple variables, or declarations
> that include initial assignments, to separate lines, due to my inability to
> make a coccinelle semantic patch match on those declarations.”
>
> * How do you think about to specify desirable change preparations by the means
> of
>   the semantic patch language?
>
> * Will any guidance help to find nicer solutions?

Luis already proposed a perfectly reasonable solution.

julia

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Markus Elfring]

>> * Will any guidance help to find nicer solutions?
>
> Luis already proposed a perfectly reasonable solution.

His proposal can look a bit too terse (for affected implementation details),
can't it?
https://sympa.inria.fr/sympa/arc/cocci/2021-11/msg00017.html
https://lore.kernel.org/cocci/YYVZva1hfwanXdyB@bombadil.infradead.org/

@@
@@
-struct page
+struct slab


Regards,
Markus

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Julia Lawall]

On Sun, 7 Nov 2021, Markus Elfring wrote:

> > > * Will any guidance help to find nicer solutions?
> >
> > Luis already proposed a perfectly reasonable solution.
>
> His proposal can look a bit too terse (for affected implementation details),
> can't it?
> https://sympa.inria.fr/sympa/arc/cocci/2021-11/msg00017.html
> https://lore.kernel.org/cocci/YYVZva1hfwanXdyB@bombadil.infradead.org/
>
> @@
> @@
> -struct page
> +struct slab

It looks fine to me.

julia

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Markus Elfring]

>> @@
>> @@
>> -struct page
>> +struct slab
>
> It looks fine to me.

Such a transformation pattern can be applied to some degree.
I imagine that there are more source code adjustments to consider
so that the desired software update will become reasonably safe.

Regards,
Markus

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Vlastimil Babka]

On 11/7/21 13:50, Markus Elfring wrote:
>>> * Will any guidance help to find nicer solutions?
>>
>> Luis already proposed a perfectly reasonable solution.
> 
> His proposal can look a bit too terse (for affected implementation details),
> can't it?
> https://sympa.inria.fr/sympa/arc/cocci/2021-11/msg00017.html
> https://lore.kernel.org/cocci/YYVZva1hfwanXdyB@bombadil.infradead.org/

I don't mind terse, I wasn't asking for complete fixup of my patches,
but for hints, and I got plenty from all of you, thanks! Will try to
apply everything tomorrow.

> @@
> @@
> -struct page
> +struct slab

The above is itself incomplete to go from "struct page *page" to "struct
slab *slab", but will results in "struct slab *page".

So my attempts included:
- struct page *page
+ struct slab *slab

But that for some reason resulted in spatch reporting (IIRC) syntax
errors, so I had to change it to this:

- struct page *page;
+ struct slab *slab;

And that worked in most cases, but didn't handle the multiple variables
being declared nor declaration+assignment.

But I assume Luis' suggestion is supposed to work together with the rule
that does simply -page +slab. I'll see how it works out, and also
Julia's suggestion "just have one rule to change all occurences of page
to slab, and then have individual rules to change eg page_address(slab)
to slab_address(slab)."

Thanks,
Vlastimil

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Julia Lawall]

On Sun, 7 Nov 2021, Vlastimil Babka wrote:

> On 11/7/21 13:50, Markus Elfring wrote:
> >>> * Will any guidance help to find nicer solutions?
> >>
> >> Luis already proposed a perfectly reasonable solution.
> >
> > His proposal can look a bit too terse (for affected implementation details),
> > can't it?
> > https://sympa.inria.fr/sympa/arc/cocci/2021-11/msg00017.html
> > https://lore.kernel.org/cocci/YYVZva1hfwanXdyB@bombadil.infradead.org/
>
> I don't mind terse, I wasn't asking for complete fixup of my patches,
> but for hints, and I got plenty from all of you, thanks! Will try to
> apply everything tomorrow.
>
> > @@
> > @@
> > -struct page
> > +struct slab
>
> The above is itself incomplete to go from "struct page *page" to "struct
> slab *slab", but will results in "struct slab *page".
>
> So my attempts included:
> - struct page *page
> + struct slab *slab
>
> But that for some reason resulted in spatch reporting (IIRC) syntax

A variable declaration needs a semicolon.

> errors, so I had to change it to this:
>
> - struct page *page;
> + struct slab *slab;

If you first rename struct page as strict slab and then do

struct slab *
- page
+ slab
 ;

it might work even for the multiple declaration case.

julia

> And that worked in most cases, but didn't handle the multiple variables
> being declared nor declaration+assignment.
>
> But I assume Luis' suggestion is supposed to work together with the rule
> that does simply -page +slab. I'll see how it works out, and also
> Julia's suggestion "just have one rule to change all occurences of page
> to slab, and then have individual rules to change eg page_address(slab)
> to slab_address(slab)."
>
> Thanks,
> Vlastimil
>

Re: [cocci] Need some help with spatch

[Vlastimil Babka]

On 11/5/21 17:20, Luis Chamberlain wrote:
> On Fri, Nov 05, 2021 at 05:12:09PM +0100, Vlastimil Babka wrote:
>> 1) patching complex variable declarations;
>> 
>> I need to perform changes such as:
>> -struct page *page;
>> +struct slab *slab;
>> 
>> This I was able to somehow specify in the .cocci, but not for situations
>> where the original declarations looks like this:
>> 
>> struct page *page, *page2;
>> 
>> or
>> 
>> struct page *page = NULL;
> 
> For this, this seems to work well:
> 
> @@
> @@
> 
> -struct page
> +struct slab

Written exactly like above it works, but then it affects code in all
functions, and I need some exceptions.

But when I adjust it as below, I get
minus: parse error:
  File "slub-new.cocci", line 161, column 0, charpos = 3152
  around = '...>',
  whole content = ...>

@@
identifier fn !~
"nearest_obj|obj_to_index|objs_per_slab_page|__slab_(un)*lock|__free_slab|free_nonslab_page|kmalloc_large_node";
@@
fn(...)
{
<...
-struct page
+struct slab
...>
}

Re: [cocci] Need some help with spatch

[Vlastimil Babka]

On 11/8/21 00:08, Vlastimil Babka wrote:
> On 11/5/21 17:20, Luis Chamberlain wrote:
>> On Fri, Nov 05, 2021 at 05:12:09PM +0100, Vlastimil Babka wrote:
>>> 1) patching complex variable declarations;
>>> 
>>> I need to perform changes such as:
>>> -struct page *page;
>>> +struct slab *slab;
>>> 
>>> This I was able to somehow specify in the .cocci, but not for situations
>>> where the original declarations looks like this:
>>> 
>>> struct page *page, *page2;
>>> 
>>> or
>>> 
>>> struct page *page = NULL;
>> 
>> For this, this seems to work well:
>> 
>> @@
>> @@
>> 
>> -struct page
>> +struct slab
> 
> Written exactly like above it works, but then it affects code in all
> functions, and I need some exceptions.

Still not full success, but I was able to make work at least some
combinations of declaration and initial assignement, such as

struct page *page = NULL;

by a rule like this:

@@
expression E;
@@
- struct page *page = E;
+ struct slab *slab = E;

But still not luck for e.g.
struct page *page, *page2;

where rules such as

- struct page *page, *page2;
+ struct slab *slab, *slab2;

don't work. Also no luck for this case:

struct page *page, *discard_page = NULL;

And thanks to the hints from Julia I was able to split up the rules so that
I no longer have unconverted expressions at places where two changes apply
to a single line.

I'll now try to build 1.1.1 to deal with the whitespace issues.

Re: [cocci] Need some help with spatch

[Julia Lawall]

On Mon, 8 Nov 2021, Vlastimil Babka wrote:

> On 11/8/21 00:08, Vlastimil Babka wrote:
> > On 11/5/21 17:20, Luis Chamberlain wrote:
> >> On Fri, Nov 05, 2021 at 05:12:09PM +0100, Vlastimil Babka wrote:
> >>> 1) patching complex variable declarations;
> >>>
> >>> I need to perform changes such as:
> >>> -struct page *page;
> >>> +struct slab *slab;
> >>>
> >>> This I was able to somehow specify in the .cocci, but not for situations
> >>> where the original declarations looks like this:
> >>>
> >>> struct page *page, *page2;
> >>>
> >>> or
> >>>
> >>> struct page *page = NULL;
> >>
> >> For this, this seems to work well:
> >>
> >> @@
> >> @@
> >>
> >> -struct page
> >> +struct slab
> >
> > Written exactly like above it works, but then it affects code in all
> > functions, and I need some exceptions.
>
> Still not full success, but I was able to make work at least some
> combinations of declaration and initial assignement, such as
>
> struct page *page = NULL;
>
> by a rule like this:
>
> @@
> expression E;
> @@
> - struct page *page = E;
> + struct slab *slab = E;
>
> But still not luck for e.g.
> struct page *page, *page2;
>
> where rules such as
>
> - struct page *page, *page2;
> + struct slab *slab, *slab2;
>
> don't work. Also no luck for this case:
>
> struct page *page, *discard_page = NULL;
>
> And thanks to the hints from Julia I was able to split up the rules so that
> I no longer have unconverted expressions at places where two changes apply
> to a single line.
>
> I'll now try to build 1.1.1 to deal with the whitespace issues.

Sorry, I got interrupted on this.

@initialize:ocaml@
@@

let ok_function p =
  not (List.mem (List.hd p).current_element ["functions";"to";"ignore"])

@@
position p : script:ocaml() { ok_function p };
@@

- struct page@p
+ struct slub

julia

Re: [cocci] Need some help with spatch

[Vlastimil Babka]

On 11/8/21 15:45, Julia Lawall wrote:
>> I'll now try to build 1.1.1 to deal with the whitespace issues.
> 
> Sorry, I got interrupted on this.
> 
> @initialize:ocaml@
> @@
> 
> let ok_function p =
>   not (List.mem (List.hd p).current_element ["functions";"to";"ignore"])
> 
> @@
> position p : script:ocaml() { ok_function p };
> @@
> 
> - struct page@p
> + struct slub
> 
> julia

Thanks a lot, that did the trick!

Note that this still patched also the struct page *page; field in struct
kmem_cache_cpu definition, which is not part of any function, but as I was
patching it anyway, it's no problem.

Re: [cocci] Need some help with spatch

[Julia Lawall]

On Mon, 8 Nov 2021, Vlastimil Babka wrote:

> On 11/8/21 15:45, Julia Lawall wrote:
> >> I'll now try to build 1.1.1 to deal with the whitespace issues.
> >
> > Sorry, I got interrupted on this.
> >
> > @initialize:ocaml@
> > @@
> >
> > let ok_function p =
> >   not (List.mem (List.hd p).current_element ["functions";"to";"ignore"])
> >
> > @@
> > position p : script:ocaml() { ok_function p };
> > @@
> >
> > - struct page@p
> > + struct slub
> >
> > julia
>
> Thanks a lot, that did the trick!
>
> Note that this still patched also the struct page *page; field in struct
> kmem_cache_cpu definition, which is not part of any function, but as I was
> patching it anyway, it's no problem.

I'm not sure what is the current element name for a structure definition.
Maybe something not so helpful like "something else".

julia

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Markus Elfring]

>> - struct page *page;
>> + struct slab *slab;
>
> If you first rename struct page as strict slab and then do
>
> struct slab *
> - page
> + slab
>   ;
>
> it might work even for the multiple declaration case.

Will there are need occur to adjust any more variable names?
(Would different suffixes matter for corresponding identifiers?)

Regards,
Markus

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Julia Lawall]

On Mon, 8 Nov 2021, Markus Elfring wrote:

> > > - struct page *page;
> > > + struct slab *slab;
> >
> > If you first rename struct page as strict slab and then do
> >
> > struct slab *
> > - page
> > + slab
> >   ;
> >
> > it might work even for the multiple declaration case.
>
> Will there are need occur to adjust any more variable names?
> (Would different suffixes matter for corresponding identifiers?)

Something could be done with python/ocaml.  But if the result is
satisfactory already, then there is no need for that.

julia

Re: [cocci] mm/slab: prepare for struct page conversion by spatch

[Vlastimil Babka]

On 11/8/21 18:35, Markus Elfring wrote:
>>> - struct page *page;
>>> + struct slab *slab;
>>
>> If you first rename struct page as strict slab and then do
>>
>> struct slab *
>> - page
>> + slab
>>   ;
>>
>> it might work even for the multiple declaration case.
> 
> Will there are need occur to adjust any more variable names?
> (Would different suffixes matter for corresponding identifiers?)

I was suprised that it works, which means the matching is more complex than
I thought. So I was able for example to achieve this:

-       struct page *page, *discard_page = NULL;
+       struct slab *slab, *slab_to_discard = NULL;

by the following rules (with ok_function per Julia's other e-mail):

@@
position p : script:ocaml() { ok_function p };
@@
- struct page@p
+ struct slab

@@
@@
struct slab *
- page
+ slab
;

@@
expression E;
@@
struct slab *
- discard_page
+ slab_to_discard
= E;

Thanks,
Vlastimil