Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1

[Eli Cohen <eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]

On Wed, May 12, 2010 at 12:56:58PM -0700, Roland Dreier wrote:
>  > @@ -1017,9 +1020,12 @@ static void ib_sa_add_one(struct ib_device *device)
>  >  	sa_dev->end_port   = e;
>  >  
>  >  	for (i = 0; i <= e - s; ++i) {
>  > +		spin_lock_init(&sa_dev->port[i].ah_lock);
>  > +		if (rdma_port_link_layer(device, i + 1) != IB_LINK_LAYER_INFINIBAND)
>  > +			continue;
> 
> Not sure I understand why you move the initialization of the spinlock up
> here?  It seems we ignore everything that might have to do with spinlock
> if this is an IBoE port.
We need the spinlock initialized for get_src_path_mask() which is
called by ib_init_ah_from_path() which in turn is called for IBoE
ports as well.

> 
> But the larger issue is what if someone calls one of the ib_sa_XXX_query
> functions on an IBoE port?  Seems we just crash on uninitialized
> structures.  I guess we're assuming that the kernel is smart enough not
> to do that?
Yes, we're not calling the SA for IBoE.

> 
> Also I'm wondering why you did the "count" stuff to ignore IBoE-only
> devices in multicast.c but not sa_query.c?
> 
It seems to me the right place to put this logic as the mutlicast code
registers as an IB client and the add_one implemntation is
multicast.c.

--
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