[<prev] [next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.20.1803251107190.2488@hadrien>
Date: Sun, 25 Mar 2018 11:07:44 -0400 (EDT)
From: Julia Lawall <julia.lawall@...6.fr>
To: Arushi Singhal <arushisinghal19971997@...il.com>
cc: daniel.vetter@...el.com, Gustavo Padovan <gustavo@...ovan.org>,
Sean Paul <seanpaul@...omium.org>,
David Airlie <airlied@...ux.ie>,
Ben Skeggs <bskeggs@...hat.com>,
dri-devel@...ts.freedesktop.org,
LKML <linux-kernel@...r.kernel.org>,
nouveau@...ts.freedesktop.org,
outreachy-kernel <outreachy-kernel@...glegroups.com>
Subject: Re: [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@...6.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?
No, I didn't have a chance to fix it yet.
julia
>
> Thanks,
> Arushi
>
>
> julia
>
>
> >
> > Signed-off-by: Arushi Singhal
> <arushisinghal19971997@...il.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@...glegroups.com.
> > To post to this group, send email to
> outreachy-kernel@...glegroups.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.
> >
>
>
> --
> 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@...glegroups.com.
> To post to this group, send email to outreachy-kernel@...glegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF9jHrOFL4LosK5Gd
> bAgpmQRd9%3D08eNzvfzYxFaeXk972g%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>
Powered by blists - more mailing lists