On Wed, Sep 01, 2010 at 09:07:07PM +0200, Julia Lawall wrote:
> [cocci mailing list added in CC]
>
> On Wed, 1 Sep 2010, Paul E. McKenney wrote:
>
> > On Wed, Sep 01, 2010 at 09:44:23AM -0700, Paul E. McKenney wrote:
> > > On Wed, Sep 01, 2010 at 08:06:18AM +0200, Julia Lawall wrote:
> >
> > [ . . . ]
> >
> > > Hmmmm... I am getting hit by loops:
> > >
> > > diff =
> > > --- drivers/net/cnic.c 2010-08-16 11:54:39.000000000 -0700
> > > @@ -2101,13 +2101,10 @@ static void service_kcqes(struct cnic_de
> > > goto end;
> > > }
> > >
> > > - rcu_read_lock();
> > > - ulp_ops = rcu_dereference(cp->ulp_ops[ulp_type]);
> > > if (likely(ulp_ops)) {
> > > - ulp_ops->indicate_kcqes(cp->ulp_handle[ulp_type],
> > > cp->completed_kcq + i, j);
> > > }
> > > - rcu_read_unlock();
> > > end:
> > > num_cqes -= j;
> > > i += j;
> > >
> > >
> > > It took me a bit to figure out that the complaint was due to the fact
> > > that the above code is within a loop. ;-)
> >
> > And the obvious fix is to do "... when != p = e2" -- sorry for not
> > figuring this out to start with!!!
>
> Since p is an identifier, this is indeed good enough. Sometimes, however,
> you are interested in an arbitrary expression, and the loop can update a
> subexpression of this expression. For example, if you are interested in
> double calls to a function foo with the same argument, you could write:
>
> @@
> expression E;
> expression E1;
> @@
>
> foo(E)
> ... when != E = E1
> foo(E)
>
> But this is not good enough to avoid, eg:
>
> for(i=0;i!=19;i++) foo(x[i]);
>
> The problem can be solved with two rules:
>
> @r@
> expression E;
> @@
>
> foo(E)
>
> @@
> expression r.E;
> expression E1;
> expression subE <= r.E;
> @@
>
> foo(E)
> ... when != subE = E1
> foo(E)
>
> subE matches any subexpression of r.E. Unfortunately, to keep the
> implementation reasonably simple, this only works when the right argument
> of <= is already bound, which is why two rules are needed.
Ah, got it!
And so if I really had wanted to key off of the initial rcu_read_lock(),
I could have done so as follows, correct?
Thanx, Paul
@rcurscs@
expression r ~= "rcu_read_lock()";
@@
r;
@rcupointerleak@
identifier p;
identifier f1, f2, f3;
expression e1, e2, e3;
expression c1;
@@
* rcurscs.r;
... when any
(
* p = rcu_dereference(e1);
|
* p = rcu_dereference_raw(e1);
|
* p = rcu_dereference_check(e1,c1);
|
* p = rcu_dereference_index_check(e1,c1);
|
* list_for_each_entry_rcu(p,e1,f3)
|
* list_for_each_entry_continue_rcu(p,e1,f3)
|
* hlist_for_each_entry_rcu(p,e1,f3)
|
* hlist_for_each_entry_continue_rcu(p,e1,f3)
)
... when != p = e2
when != atomic_inc_not_zero(&<+...p->f1...+>)
when != atomic_inc_not_zero(&<+...p[e2]...+>)
when != atomic_inc(&<+...p->f1...+>)
when != atomic_inc(&<+...p[e2]...+>)
when != try_module_get(p->owner)
* rcu_read_unlock();
... when != rcurscs.r;
(
* *p
|
* p->f2
|
* p[e3]
)
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)
