[PATCH] net: add sparse annotation to ptype_seq_start/stop

[PATCH] net: add sparse annotation to ptype_seq_start/stop

[Stephen Hemminger]

Get rid of some more sparse warnings.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/core/dev.c	2008-01-20 14:25:00.000000000 -0800
+++ b/net/core/dev.c	2008-01-20 14:25:48.000000000 -0800
@@ -2543,6 +2543,7 @@ static void *ptype_get_idx(loff_t pos)
 }
 
 static void *ptype_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(rcu)
 {
 	rcu_read_lock();
 	return *pos ? ptype_get_idx(*pos - 1) : SEQ_START_TOKEN;
@@ -2578,6 +2579,7 @@ found:
 }
 
 static void ptype_seq_stop(struct seq_file *seq, void *v)
+	__releases(rcu)
 {
 	rcu_read_unlock();
 }

Re: [PATCH] net: add sparse annotation to ptype_seq_start/stop

[Paul E. McKenney]

On Sun, Jan 20, 2008 at 03:21:46PM -0800, Stephen Hemminger wrote:
> Get rid of some more sparse warnings.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Adding Josh to CC -- is __acquires() case-insensitive?  If not, this
needs to be __acquires(RCU) and __releases(RCU).

						Thanx, Paul

> --- a/net/core/dev.c	2008-01-20 14:25:00.000000000 -0800
> +++ b/net/core/dev.c	2008-01-20 14:25:48.000000000 -0800
> @@ -2543,6 +2543,7 @@ static void *ptype_get_idx(loff_t pos)
>  }
> 
>  static void *ptype_seq_start(struct seq_file *seq, loff_t *pos)
> +	__acquires(rcu)
>  {
>  	rcu_read_lock();
>  	return *pos ? ptype_get_idx(*pos - 1) : SEQ_START_TOKEN;
> @@ -2578,6 +2579,7 @@ found:
>  }
> 
>  static void ptype_seq_stop(struct seq_file *seq, void *v)
> +	__releases(rcu)
>  {
>  	rcu_read_unlock();
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: add sparse annotation to ptype_seq_start/stop

[Al Viro]

On Sun, Jan 20, 2008 at 09:03:55PM -0800, Paul E. McKenney wrote:
> On Sun, Jan 20, 2008 at 03:21:46PM -0800, Stephen Hemminger wrote:
> > Get rid of some more sparse warnings.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> Adding Josh to CC -- is __acquires() case-insensitive?  If not, this
> needs to be __acquires(RCU) and __releases(RCU).

__acquires(whatever_the_helk_you_put_as_its_argument) is all the same.

Re: [PATCH] net: add sparse annotation to ptype_seq_start/stop

[Paul E. McKenney]

On Mon, Jan 21, 2008 at 05:28:31AM +0000, Al Viro wrote:
> On Sun, Jan 20, 2008 at 09:03:55PM -0800, Paul E. McKenney wrote:
> > On Sun, Jan 20, 2008 at 03:21:46PM -0800, Stephen Hemminger wrote:
> > > Get rid of some more sparse warnings.
> > > 
> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > 
> > Adding Josh to CC -- is __acquires() case-insensitive?  If not, this
> > needs to be __acquires(RCU) and __releases(RCU).
> 
> __acquires(whatever_the_helk_you_put_as_its_argument) is all the same.

The existing calls are __acquires(RCU) and __releases(RCU).  I suppose
that one -could- use __acquires(RCU) in some places and __acquires(rcu)
in others, but that sounds like it would eventually bite us...

							Thanx, Paul

Re: [PATCH] net: add sparse annotation to ptype_seq_start/stop

[Josh Triplett]

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

Paul E. McKenney wrote:
> On Sun, Jan 20, 2008 at 03:21:46PM -0800, Stephen Hemminger wrote:
>> Get rid of some more sparse warnings.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> Adding Josh to CC -- is __acquires() case-insensitive?  If not, this
> needs to be __acquires(RCU) and __releases(RCU).
[...]
>> --- a/net/core/dev.c	2008-01-20 14:25:00.000000000 -0800
>> +++ b/net/core/dev.c	2008-01-20 14:25:48.000000000 -0800
>> @@ -2543,6 +2543,7 @@ static void *ptype_get_idx(loff_t pos)
>>  }
>>
>>  static void *ptype_seq_start(struct seq_file *seq, loff_t *pos)
>> +	__acquires(rcu)
>>  {
>>  	rcu_read_lock();
>>  	return *pos ? ptype_get_idx(*pos - 1) : SEQ_START_TOKEN;
>> @@ -2578,6 +2579,7 @@ found:
>>  }
>>
>>  static void ptype_seq_stop(struct seq_file *seq, void *v)
>> +	__releases(rcu)
>>  {
>>  	rcu_read_unlock();
>>  }

At the moment, Sparse doesn't actually use the context expression.  In
the ideal case when it does, all references to the same lock should
use a context expression which resolves to that lock (whatever that
may mean; hence why Sparse doesn't handle it yet).  For mechanisms
like RCU without a lock variable, this will likely entail some sort of
fake lock expression, which again needs to match between all users of
the same mechanism.  Like any expression in C, case matters; thus,
please match the existing references to "RCU" rather than "rcu".

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] net: add sparse annotation to ptype_seq_start/stop

[David Miller]

From: Josh Triplett <josh@freedesktop.org>
Date: Sun, 20 Jan 2008 23:29:32 -0800

> At the moment, Sparse doesn't actually use the context expression.  In
> the ideal case when it does, all references to the same lock should
> use a context expression which resolves to that lock (whatever that
> may mean; hence why Sparse doesn't handle it yet).  For mechanisms
> like RCU without a lock variable, this will likely entail some sort of
> fake lock expression, which again needs to match between all users of
> the same mechanism.  Like any expression in C, case matters; thus,
> please match the existing references to "RCU" rather than "rcu".

Thus, I've checked in Stephen's patch with __acquires(RCU) and
__releases(RCU).

Thanks.