[PATCH] IB/core: Suppress a sparse warning

[PATCH] IB/core: Suppress a sparse warning

[Bart Van Assche]

Suppress the following sparse warning:

include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/rdma/ib_addr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index ce55906..bf11511 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -184,7 +184,7 @@ static inline void iboe_addr_get_sgid(struct rdma_dev_addr *dev_addr,
 
 	dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
 	if (dev) {
-		ip4 = (struct in_device *)dev->ip_ptr;
+		ip4 = (__force struct in_device *)dev->ip_ptr;
 		if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
 			ipv6_addr_set_v4mapped(ip4->ifa_list->ifa_address,
 					       (struct in6_addr *)gid);
-- 
1.8.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Suppress a sparse warning

[Yann Droneaud]

Le lundi 10 mars 2014 à 13:22 +0100, Bart Van Assche a écrit :
> Suppress the following sparse warning:
> 
> include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression
> 

You should explain why there's a warning here, and why is it safe to
cast. (I believe it's related to RCU domain ?)

Regards.

> Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> Cc: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  include/rdma/ib_addr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
> index ce55906..bf11511 100644
> --- a/include/rdma/ib_addr.h
> +++ b/include/rdma/ib_addr.h
> @@ -184,7 +184,7 @@ static inline void iboe_addr_get_sgid(struct rdma_dev_addr *dev_addr,
>  
>  	dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
>  	if (dev) {
> -		ip4 = (struct in_device *)dev->ip_ptr;
> +		ip4 = (__force struct in_device *)dev->ip_ptr;
>  		if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
>  			ipv6_addr_set_v4mapped(ip4->ifa_list->ifa_address,
>  					       (struct in6_addr *)gid);

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Suppress a sparse warning

[Bart Van Assche]

On 03/10/14 14:33, Yann Droneaud wrote:
> Le lundi 10 mars 2014 à 13:22 +0100, Bart Van Assche a écrit :
>> Suppress the following sparse warning:
>> include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression
> 
> You should explain why there's a warning here, and why is it safe to
> cast. (I believe it's related to RCU domain ?)

Hello Yann,

Now that I've had a closer look at the code in include/rdma/ib_addr.h,
that code probably isn't safe. How about the (untested) patch below ?

diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index ce55906..5a416ac 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -184,7 +184,7 @@ static inline void iboe_addr_get_sgid(struct rdma_dev_addr *dev_addr,
 
 	dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
 	if (dev) {
-		ip4 = (struct in_device *)dev->ip_ptr;
+		ip4 = __in_dev_get_rcu(dev);
 		if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
 			ipv6_addr_set_v4mapped(ip4->ifa_list->ifa_address,
 					       (struct in6_addr *)gid);
-- 
1.8.4.5


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Suppress a sparse warning

[Yann Droneaud]

Hi,

Le lundi 10 mars 2014 à 15:26 +0100, Bart Van Assche a écrit :
> On 03/10/14 14:33, Yann Droneaud wrote:
> > Le lundi 10 mars 2014 à 13:22 +0100, Bart Van Assche a écrit :
> >> Suppress the following sparse warning:
> >> include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression
> > 
> > You should explain why there's a warning here, and why is it safe to
> > cast. (I believe it's related to RCU domain ?)
> 
> Hello Yann,
> 
> Now that I've had a closer look at the code in include/rdma/ib_addr.h,
> that code probably isn't safe. How about the (untested) patch below ?
> 

Thanks for investigating.

I'm not an expert in RCU, but I believe it then miss the RCU annotations
around the RCU reader section (ensure correct ordering if I recall
correctly).

Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

> diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
> index ce55906..5a416ac 100644
> --- a/include/rdma/ib_addr.h
> +++ b/include/rdma/ib_addr.h
> @@ -184,7 +184,7 @@ static inline void iboe_addr_get_sgid(struct rdma_dev_addr *dev_addr,
>  
>  	dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
>  	if (dev) {

+		rcu_read_lock();

> -		ip4 = (struct in_device *)dev->ip_ptr;
> +		ip4 = __in_dev_get_rcu(dev);

>  		if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
>  			ipv6_addr_set_v4mapped(ip4->ifa_list->ifa_address,
>  					       (struct in6_addr *)gid);

+		rcu_read_unlock();


Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Suppress a sparse warning

[Paul E. McKenney]

On Mon, Mar 10, 2014 at 04:02:13PM +0100, Yann Droneaud wrote:
> Hi,
> 
> Le lundi 10 mars 2014 à 15:26 +0100, Bart Van Assche a écrit :
> > On 03/10/14 14:33, Yann Droneaud wrote:
> > > Le lundi 10 mars 2014 à 13:22 +0100, Bart Van Assche a écrit :
> > >> Suppress the following sparse warning:
> > >> include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression
> > > 
> > > You should explain why there's a warning here, and why is it safe to
> > > cast. (I believe it's related to RCU domain ?)
> > 
> > Hello Yann,
> > 
> > Now that I've had a closer look at the code in include/rdma/ib_addr.h,
> > that code probably isn't safe. How about the (untested) patch below ?
> > 
> 
> Thanks for investigating.
> 
> I'm not an expert in RCU, but I believe it then miss the RCU annotations
> around the RCU reader section (ensure correct ordering if I recall
> correctly).
> 
> Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

If the rcu_read_lock() isn't supplied by all callers to this function,
then yes, it needs to be supplied as Yann shows below.

The CONFIG_PROVE_RCU=y Kconfig option can help determine that they are
needed, but of course cannot prove that they are not needed, at least
not unless you have a workload that exercises all possible calls to
this function.

							Thanx, Paul

> > diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
> > index ce55906..5a416ac 100644
> > --- a/include/rdma/ib_addr.h
> > +++ b/include/rdma/ib_addr.h
> > @@ -184,7 +184,7 @@ static inline void iboe_addr_get_sgid(struct rdma_dev_addr *dev_addr,
> >  
> >  	dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
> >  	if (dev) {
> 
> +		rcu_read_lock();
> 
> > -		ip4 = (struct in_device *)dev->ip_ptr;
> > +		ip4 = __in_dev_get_rcu(dev);
> 
> >  		if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
> >  			ipv6_addr_set_v4mapped(ip4->ifa_list->ifa_address,
> >  					       (struct in6_addr *)gid);
> 
> +		rcu_read_unlock();
> 
> 
> Regards.
> 
> -- 
> Yann Droneaud
> OPTEYA
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Suppress a sparse warning

[Bart Van Assche]

On 03/10/14 17:08, Paul E. McKenney wrote:
> On Mon, Mar 10, 2014 at 04:02:13PM +0100, Yann Droneaud wrote:
>> Hi,
>>
>> Le lundi 10 mars 2014 à 15:26 +0100, Bart Van Assche a écrit :
>>> On 03/10/14 14:33, Yann Droneaud wrote:
>>>> Le lundi 10 mars 2014 à 13:22 +0100, Bart Van Assche a écrit :
>>>>> Suppress the following sparse warning:
>>>>> include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression
>>>>
>>>> You should explain why there's a warning here, and why is it safe to
>>>> cast. (I believe it's related to RCU domain ?)
>>>
>>> Hello Yann,
>>>
>>> Now that I've had a closer look at the code in include/rdma/ib_addr.h,
>>> that code probably isn't safe. How about the (untested) patch below ?
>>>
>>
>> Thanks for investigating.
>>
>> I'm not an expert in RCU, but I believe it then miss the RCU annotations
>> around the RCU reader section (ensure correct ordering if I recall
>> correctly).
>>
>> Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> 
> If the rcu_read_lock() isn't supplied by all callers to this function,
> then yes, it needs to be supplied as Yann shows below.
> 
> The CONFIG_PROVE_RCU=y Kconfig option can help determine that they are
> needed, but of course cannot prove that they are not needed, at least
> not unless you have a workload that exercises all possible calls to
> this function.

Hello Moni,

I think this warning got introduced via commit "IB/cma: IBoE (RoCE)
IP-based GID addressing" (7b85627b9f02f9b0fb2ef5f021807f4251135857;
December 12, 2013). Can you follow this up further ?

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Suppress a sparse warning

[Moni Shoua]

On 3/12/2014 12:15 PM, Bart Van Assche wrote:
> On 03/10/14 17:08, Paul E. McKenney wrote:
>> On Mon, Mar 10, 2014 at 04:02:13PM +0100, Yann Droneaud wrote:
>>> Hi,
>>>
>>> Le lundi 10 mars 2014 à 15:26 +0100, Bart Van Assche a écrit :
>>>> On 03/10/14 14:33, Yann Droneaud wrote:
>>>>> Le lundi 10 mars 2014 à 13:22 +0100, Bart Van Assche a écrit :
>>>>>> Suppress the following sparse warning:
>>>>>> include/rdma/ib_addr.h:187:24: warning: cast removes address space of expression
>>>>> You should explain why there's a warning here, and why is it safe to
>>>>> cast. (I believe it's related to RCU domain ?)
>>>> Hello Yann,
>>>>
>>>> Now that I've had a closer look at the code in include/rdma/ib_addr.h,
>>>> that code probably isn't safe. How about the (untested) patch below ?
>>>>
>>> Thanks for investigating.
>>>
>>> I'm not an expert in RCU, but I believe it then miss the RCU annotations
>>> around the RCU reader section (ensure correct ordering if I recall
>>> correctly).
>>>
>>> Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> If the rcu_read_lock() isn't supplied by all callers to this function,
>> then yes, it needs to be supplied as Yann shows below.
>>
>> The CONFIG_PROVE_RCU=y Kconfig option can help determine that they are
>> needed, but of course cannot prove that they are not needed, at least
>> not unless you have a workload that exercises all possible calls to
>> this function.
> Hello Moni,
>
> I think this warning got introduced via commit "IB/cma: IBoE (RoCE)
> IP-based GID addressing" (7b85627b9f02f9b0fb2ef5f021807f4251135857;
> December 12, 2013). Can you follow this up further ?
>
> Thanks,
>
> Bart.
Sure. I'll look into it.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html