lkml.org 
[lkml]   [2018]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Outreachy kernel] [PATCH] gpu: drm: Use list_{next/prev}_entry instead of list_entry


On Sun, 25 Mar 2018, Arushi Singhal wrote:

>
>
> On Mon, Mar 19, 2018 at 12:44 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Mon, 19 Mar 2018, Arushi Singhal wrote:
>
> > This patch replace list_entry with list_{next/prev}_entry as
> it makes
> > the code more clear to read.
> > Done using coccinelle:
> >
> > @@
> > expression e1;
> > identifier e3;
> > type t;
> > @@
> > (
> > - list_entry(e1->e3.next,t,e3)
> > + list_next_entry(e1,e3)
> > |
> > - list_entry(e1->e3.prev,t,e3)
> > + list_prev_entry(e1,e3)
> > )
>
> This looks like a rule that could be nice for the Linux kernel
> in general,
> because the code really is much simpler.
>
> I would suggest to write the rule in a more robust way, as
> follows:
>
> @@
> identifier e3;
> type t;
> t *e1;
> @@
>
> (
> - list_entry(e1->e3.next,t,e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,t,e3)
> + list_prev_entry(e1,e3)
> )
>
> @@
> expression e1;
> identifier e3;
> @@
>
> (
> - list_entry(e1->e3.next,typeof(*e1),e3)
> + list_next_entry(e1,e3)
> |
> - list_entry(e1->e3.prev,typeof(*e1),e3)
> + list_prev_entry(e1,e3)
>
> This checks that the type that is specified corresponds to the
> one on e1.
> It could actually be that the call is getting the first element
> of a list,
> from some different type, and coincidentally the two types have
> the same
> field name for the list element.
>
> Unfortunately, the second rule, with the typeof call, doesn't
> currently
> work in Coccinelle, because the semantic patch language doesn't
> actually
> support typeof, and thinks that it is a function call.  I will
> fix this.
>
> To make a semantic patch for the kernel, you can try running
> spgen on the
> above file and answer the questions that it asks.  You can find
> examples
> in the coccinelle/scripts directory.  Just run
>
> spgen foo.cocci
>
> Then answer the questions.  Then run
>
> spgen foo.cocci > foo_for_kernel.cocci
>
> The second run will use the results of the first run to print
> the semantic
> patch.  Let me know if you have any questions.  You can always
> adjust the
> semantic patch that is generated by hand afterwards if needed.
>
>
> Hi Julia,
>
> I tried spgen and found that second rule is still not working. It's not able
> to detect the second rule.
> Is it working for you?

If you get the latest version of Coccinelle from github, it shoudl be
working now.

julia

>
> Thanks,
> Arushi
>
>
> julia
>
>
> >
> > Signed-off-by: Arushi Singhal
> <arushisinghal19971997@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_lease.c                    | 2 +-
> >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_lease.c
> b/drivers/gpu/drm/drm_lease.c
> > index 1402c0e..4dcfb5f 100644
> > --- a/drivers/gpu/drm/drm_lease.c
> > +++ b/drivers/gpu/drm/drm_lease.c
> > @@ -340,7 +340,7 @@ static void _drm_lease_revoke(struct
> drm_master *top)
> >                               break;
> >
> >                       /* Over */
> > -                     master =
> list_entry(master->lessee_list.next, struct drm_master,
> lessee_list);
> > +                     master = list_next_entry(master,
> lessee_list);
> >               }
> >       }
> >  }
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > index e4c8d31..81c3567 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk
> *clk, struct nvkm_pstate *pstate,
> >                              nvkm_volt_map(volt,
> volt->max2_id, clk->temp));
> >
> >       for (cstate = start; &cstate->head != &pstate->list;
> > -          cstate = list_entry(cstate->head.prev,
> typeof(*cstate), head)) {
> > +          cstate = list_prev_entry(cstate, head)) {
> >               if (nvkm_cstate_valid(clk, cstate, max_volt,
> clk->temp))
> >                       break;
> >       }
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to
> outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20180319050530.GA25589%4
> 0seema-Inspiron-15-3567.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
>
>
>
\
 
 \ /
  Last update: 2018-03-26 00:46    [W:0.090 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site