public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCHv8 02/11] ib_core: IBoE support only QP1
@ 2010-02-18 17:23 Eli Cohen
  2010-05-05 23:12 ` [ewg] " Roland Dreier
  2010-05-12 19:56 ` Roland Dreier
  0 siblings, 2 replies; 10+ messages in thread
From: Eli Cohen @ 2010-02-18 17:23 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Linux RDMA list, ewg

Since IBoE is using Ethernet as its link layer, there is no central management
entity so there is need for QP0.  QP1 is still needed since it handles
communications between CM agents. This patch will create only QP1 for IBoE
ports.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
---
Changes from v7:

1. Remove "always true" code
2. Fix failure to initialize port ah_lock in ib_sa_add_one


 drivers/infiniband/core/agent.c     |   37 +++++++++++++++++++------------
 drivers/infiniband/core/mad.c       |   27 +++++++++++++++++++---
 drivers/infiniband/core/multicast.c |   25 ++++++++++++++++++---
 drivers/infiniband/core/sa_query.c  |   41 ++++++++++++++++++++++------------
 4 files changed, 93 insertions(+), 37 deletions(-)

diff --git a/drivers/infiniband/core/agent.c b/drivers/infiniband/core/agent.c
index ae7c288..964f4fb 100644
--- a/drivers/infiniband/core/agent.c
+++ b/drivers/infiniband/core/agent.c
@@ -48,6 +48,8 @@
 struct ib_agent_port_private {
 	struct list_head port_list;
 	struct ib_mad_agent *agent[2];
+	struct ib_device    *device;
+	u8		     port_num;
 };
 
 static DEFINE_SPINLOCK(ib_agent_port_list_lock);
@@ -58,11 +60,10 @@ __ib_get_agent_port(struct ib_device *device, int port_num)
 {
 	struct ib_agent_port_private *entry;
 
-	list_for_each_entry(entry, &ib_agent_port_list, port_list) {
-		if (entry->agent[0]->device == device &&
-		    entry->agent[0]->port_num == port_num)
+	list_for_each_entry(entry, &ib_agent_port_list, port_list)
+		if (entry->device == device && entry->port_num == port_num)
 			return entry;
-	}
+
 	return NULL;
 }
 
@@ -155,14 +156,16 @@ int ib_agent_port_open(struct ib_device *device, int port_num)
 		goto error1;
 	}
 
-	/* Obtain send only MAD agent for SMI QP */
-	port_priv->agent[0] = ib_register_mad_agent(device, port_num,
-						    IB_QPT_SMI, NULL, 0,
-						    &agent_send_handler,
-						    NULL, NULL);
-	if (IS_ERR(port_priv->agent[0])) {
-		ret = PTR_ERR(port_priv->agent[0]);
-		goto error2;
+	if (rdma_port_link_layer(device, port_num) == IB_LINK_LAYER_INFINIBAND) {
+		/* Obtain send only MAD agent for SMI QP */
+		port_priv->agent[0] = ib_register_mad_agent(device, port_num,
+							    IB_QPT_SMI, NULL, 0,
+							    &agent_send_handler,
+							    NULL, NULL);
+		if (IS_ERR(port_priv->agent[0])) {
+			ret = PTR_ERR(port_priv->agent[0]);
+			goto error2;
+		}
 	}
 
 	/* Obtain send only MAD agent for GSI QP */
@@ -175,6 +178,9 @@ int ib_agent_port_open(struct ib_device *device, int port_num)
 		goto error3;
 	}
 
+	port_priv->device = device;
+	port_priv->port_num = port_num;
+
 	spin_lock_irqsave(&ib_agent_port_list_lock, flags);
 	list_add_tail(&port_priv->port_list, &ib_agent_port_list);
 	spin_unlock_irqrestore(&ib_agent_port_list_lock, flags);
@@ -182,7 +188,8 @@ int ib_agent_port_open(struct ib_device *device, int port_num)
 	return 0;
 
 error3:
-	ib_unregister_mad_agent(port_priv->agent[0]);
+	if (rdma_port_link_layer(device, port_num) == IB_LINK_LAYER_INFINIBAND)
+		ib_unregister_mad_agent(port_priv->agent[0]);
 error2:
 	kfree(port_priv);
 error1:
@@ -205,7 +212,9 @@ int ib_agent_port_close(struct ib_device *device, int port_num)
 	spin_unlock_irqrestore(&ib_agent_port_list_lock, flags);
 
 	ib_unregister_mad_agent(port_priv->agent[1]);
-	ib_unregister_mad_agent(port_priv->agent[0]);
+	if (rdma_port_link_layer(device, port_num) == IB_LINK_LAYER_INFINIBAND)
+		ib_unregister_mad_agent(port_priv->agent[0]);
+
 	kfree(port_priv);
 	return 0;
 }
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 7522008..f546ab7 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -2610,6 +2610,9 @@ static void cleanup_recv_queue(struct ib_mad_qp_info *qp_info)
 	struct ib_mad_private *recv;
 	struct ib_mad_list_head *mad_list;
 
+	if (!qp_info->qp)
+		return;
+
 	while (!list_empty(&qp_info->recv_queue.list)) {
 
 		mad_list = list_entry(qp_info->recv_queue.list.next,
@@ -2651,6 +2654,9 @@ static int ib_mad_port_start(struct ib_mad_port_private *port_priv)
 
 	for (i = 0; i < IB_MAD_QPS_CORE; i++) {
 		qp = port_priv->qp_info[i].qp;
+		if (!qp)
+			continue;
+
 		/*
 		 * PKey index for QP1 is irrelevant but
 		 * one is needed for the Reset to Init transition
@@ -2692,6 +2698,9 @@ static int ib_mad_port_start(struct ib_mad_port_private *port_priv)
 	}
 
 	for (i = 0; i < IB_MAD_QPS_CORE; i++) {
+		if (!port_priv->qp_info[i].qp)
+			continue;
+
 		ret = ib_mad_post_receive_mads(&port_priv->qp_info[i], NULL);
 		if (ret) {
 			printk(KERN_ERR PFX "Couldn't post receive WRs\n");
@@ -2770,6 +2779,9 @@ error:
 
 static void destroy_mad_qp(struct ib_mad_qp_info *qp_info)
 {
+	if (!qp_info->qp)
+		return;
+
 	ib_destroy_qp(qp_info->qp);
 	kfree(qp_info->snoop_table);
 }
@@ -2785,6 +2797,7 @@ static int ib_mad_port_open(struct ib_device *device,
 	struct ib_mad_port_private *port_priv;
 	unsigned long flags;
 	char name[sizeof "ib_mad123"];
+	int has_smi;
 
 	/* Create new device info */
 	port_priv = kzalloc(sizeof *port_priv, GFP_KERNEL);
@@ -2800,7 +2813,11 @@ static int ib_mad_port_open(struct ib_device *device,
 	init_mad_qp(port_priv, &port_priv->qp_info[0]);
 	init_mad_qp(port_priv, &port_priv->qp_info[1]);
 
-	cq_size = (mad_sendq_size + mad_recvq_size) * 2;
+	cq_size = mad_sendq_size + mad_recvq_size;
+	has_smi = rdma_port_link_layer(device, port_num) == IB_LINK_LAYER_INFINIBAND;
+	if (has_smi)
+		cq_size *= 2;
+
 	port_priv->cq = ib_create_cq(port_priv->device,
 				     ib_mad_thread_completion_handler,
 				     NULL, port_priv, cq_size, 0);
@@ -2824,9 +2841,11 @@ static int ib_mad_port_open(struct ib_device *device,
 		goto error5;
 	}
 
-	ret = create_mad_qp(&port_priv->qp_info[0], IB_QPT_SMI);
-	if (ret)
-		goto error6;
+	if (has_smi) {
+		ret = create_mad_qp(&port_priv->qp_info[0], IB_QPT_SMI);
+		if (ret)
+			goto error6;
+	}
 	ret = create_mad_qp(&port_priv->qp_info[1], IB_QPT_GSI);
 	if (ret)
 		goto error7;
diff --git a/drivers/infiniband/core/multicast.c b/drivers/infiniband/core/multicast.c
index 8d82ba1..55f24ff 100644
--- a/drivers/infiniband/core/multicast.c
+++ b/drivers/infiniband/core/multicast.c
@@ -773,6 +773,10 @@ static void mcast_event_handler(struct ib_event_handler *handler,
 	int index;
 
 	dev = container_of(handler, struct mcast_device, event_handler);
+	if (rdma_port_link_layer(dev->device, event->element.port_num) !=
+	    IB_LINK_LAYER_INFINIBAND)
+		return;
+
 	index = event->element.port_num - dev->start_port;
 
 	switch (event->event) {
@@ -795,11 +799,12 @@ static void mcast_add_one(struct ib_device *device)
 	struct mcast_device *dev;
 	struct mcast_port *port;
 	int i;
+	int count = 0;
 
 	if (rdma_node_get_transport(device->node_type) != RDMA_TRANSPORT_IB)
 		return;
 
-	dev = kmalloc(sizeof *dev + device->phys_port_cnt * sizeof *port,
+	dev = kzalloc(sizeof *dev + device->phys_port_cnt * sizeof *port,
 		      GFP_KERNEL);
 	if (!dev)
 		return;
@@ -812,6 +817,9 @@ static void mcast_add_one(struct ib_device *device)
 	}
 
 	for (i = 0; i <= dev->end_port - dev->start_port; i++) {
+		if (rdma_port_link_layer(device, dev->start_port + i) !=
+		    IB_LINK_LAYER_INFINIBAND)
+			continue;
 		port = &dev->port[i];
 		port->dev = dev;
 		port->port_num = dev->start_port + i;
@@ -819,6 +827,12 @@ static void mcast_add_one(struct ib_device *device)
 		port->table = RB_ROOT;
 		init_completion(&port->comp);
 		atomic_set(&port->refcount, 1);
+		++count;
+	}
+
+	if (!count) {
+		kfree(dev);
+		return;
 	}
 
 	dev->device = device;
@@ -842,9 +856,12 @@ static void mcast_remove_one(struct ib_device *device)
 	flush_workqueue(mcast_wq);
 
 	for (i = 0; i <= dev->end_port - dev->start_port; i++) {
-		port = &dev->port[i];
-		deref_port(port);
-		wait_for_completion(&port->comp);
+		if (rdma_port_link_layer(device, dev->start_port + i) ==
+		    IB_LINK_LAYER_INFINIBAND) {
+			port = &dev->port[i];
+			deref_port(port);
+			wait_for_completion(&port->comp);
+		}
 	}
 
 	kfree(dev);
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 7e1ffd8..106284b 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -416,14 +416,17 @@ static void ib_sa_event(struct ib_event_handler *handler, struct ib_event *event
 		struct ib_sa_port *port =
 			&sa_dev->port[event->element.port_num - sa_dev->start_port];
 
-		spin_lock_irqsave(&port->ah_lock, flags);
-		if (port->sm_ah)
-			kref_put(&port->sm_ah->ref, free_sm_ah);
-		port->sm_ah = NULL;
-		spin_unlock_irqrestore(&port->ah_lock, flags);
-
-		schedule_work(&sa_dev->port[event->element.port_num -
-					    sa_dev->start_port].update_task);
+		if (rdma_port_link_layer(handler->device, port->port_num) == IB_LINK_LAYER_INFINIBAND) {
+			spin_lock_irqsave(&port->ah_lock, flags);
+			if (port->sm_ah)
+				kref_put(&port->sm_ah->ref, free_sm_ah);
+			port->sm_ah = NULL;
+			spin_unlock_irqrestore(&port->ah_lock, flags);
+
+			schedule_work(&sa_dev->port[event->element.port_num -
+				      sa_dev->start_port].update_task);
+		}
+
 	}
 }
 
@@ -1007,7 +1010,7 @@ static void ib_sa_add_one(struct ib_device *device)
 		e = device->phys_port_cnt;
 	}
 
-	sa_dev = kmalloc(sizeof *sa_dev +
+	sa_dev = kzalloc(sizeof *sa_dev +
 			 (e - s + 1) * sizeof (struct ib_sa_port),
 			 GFP_KERNEL);
 	if (!sa_dev)
@@ -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;
+
 		sa_dev->port[i].sm_ah    = NULL;
 		sa_dev->port[i].port_num = i + s;
-		spin_lock_init(&sa_dev->port[i].ah_lock);
 
 		sa_dev->port[i].agent =
 			ib_register_mad_agent(device, i + s, IB_QPT_GSI,
@@ -1045,13 +1051,15 @@ static void ib_sa_add_one(struct ib_device *device)
 		goto err;
 
 	for (i = 0; i <= e - s; ++i)
-		update_sm_ah(&sa_dev->port[i].update_task);
+		if (rdma_port_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND)
+			update_sm_ah(&sa_dev->port[i].update_task);
 
 	return;
 
 err:
 	while (--i >= 0)
-		ib_unregister_mad_agent(sa_dev->port[i].agent);
+		if (rdma_port_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND)
+			ib_unregister_mad_agent(sa_dev->port[i].agent);
 
 	kfree(sa_dev);
 
@@ -1071,9 +1079,12 @@ static void ib_sa_remove_one(struct ib_device *device)
 	flush_scheduled_work();
 
 	for (i = 0; i <= sa_dev->end_port - sa_dev->start_port; ++i) {
-		ib_unregister_mad_agent(sa_dev->port[i].agent);
-		if (sa_dev->port[i].sm_ah)
-			kref_put(&sa_dev->port[i].sm_ah->ref, free_sm_ah);
+		if (rdma_port_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND) {
+			ib_unregister_mad_agent(sa_dev->port[i].agent);
+			if (sa_dev->port[i].sm_ah)
+				kref_put(&sa_dev->port[i].sm_ah->ref, free_sm_ah);
+		}
+
 	}
 
 	kfree(sa_dev);
-- 
1.7.0

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
  2010-02-18 17:23 [PATCHv8 02/11] ib_core: IBoE support only QP1 Eli Cohen
@ 2010-05-05 23:12 ` Roland Dreier
       [not found]   ` <ada1vdqaqpc.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
  2010-05-12 19:56 ` Roland Dreier
  1 sibling, 1 reply; 10+ messages in thread
From: Roland Dreier @ 2010-05-05 23:12 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Linux RDMA list, ewg

 > @@ -795,11 +799,12 @@ static void mcast_add_one(struct ib_device *device)
 >  	struct mcast_device *dev;
 >  	struct mcast_port *port;
 >  	int i;
 > +	int count = 0;
 >  
 >  	if (rdma_node_get_transport(device->node_type) != RDMA_TRANSPORT_IB)
 >  		return;
 >  
 > -	dev = kmalloc(sizeof *dev + device->phys_port_cnt * sizeof *port,
 > +	dev = kzalloc(sizeof *dev + device->phys_port_cnt * sizeof *port,

 > @@ -1007,7 +1010,7 @@ static void ib_sa_add_one(struct ib_device *device)
 >  		e = device->phys_port_cnt;
 >  	}
 >  
 > -	sa_dev = kmalloc(sizeof *sa_dev +
 > +	sa_dev = kzalloc(sizeof *sa_dev +

Do you happen to remember why you needed these kmalloc -> kzalloc conversions?
-- 
Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]   ` <ada1vdqaqpc.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
@ 2010-05-06 14:28     ` Eli Cohen
       [not found]       ` <20100506142849.GC29792-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Cohen @ 2010-05-06 14:28 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Eli Cohen, Linux RDMA list, ewg

On Wed, May 05, 2010 at 04:12:15PM -0700, Roland Dreier wrote:
>  > @@ -795,11 +799,12 @@ static void mcast_add_one(struct ib_device *device)
>  >  	struct mcast_device *dev;
>  >  	struct mcast_port *port;
>  >  	int i;
>  > +	int count = 0;
>  >  
>  >  	if (rdma_node_get_transport(device->node_type) != RDMA_TRANSPORT_IB)
>  >  		return;
>  >  
>  > -	dev = kmalloc(sizeof *dev + device->phys_port_cnt * sizeof *port,
>  > +	dev = kzalloc(sizeof *dev + device->phys_port_cnt * sizeof *port,
> 
>  > @@ -1007,7 +1010,7 @@ static void ib_sa_add_one(struct ib_device *device)
>  >  		e = device->phys_port_cnt;
>  >  	}
>  >  
>  > -	sa_dev = kmalloc(sizeof *sa_dev +
>  > +	sa_dev = kzalloc(sizeof *sa_dev +
> 
> Do you happen to remember why you needed these kmalloc -> kzalloc conversions?

I can't remember why. I do have this habbit of prefering kzalloc
over kmalloc because it saves troubles sometimes.
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
  2010-02-18 17:23 [PATCHv8 02/11] ib_core: IBoE support only QP1 Eli Cohen
  2010-05-05 23:12 ` [ewg] " Roland Dreier
@ 2010-05-12 19:56 ` Roland Dreier
       [not found]   ` <ada632svqph.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Roland Dreier @ 2010-05-12 19:56 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Linux RDMA list, ewg

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

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?

Also I'm wondering why you did the "count" stuff to ignore IBoE-only
devices in multicast.c but not sa_query.c?

 - R.
-- 
Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]   ` <ada632svqph.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
@ 2010-05-13  6:59     ` Eli Cohen
       [not found]       ` <20100513065917.GA16073-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Cohen @ 2010-05-13  6:59 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Eli Cohen, Linux RDMA list, ewg

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]       ` <20100513065917.GA16073-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
@ 2010-05-13 15:48         ` Roland Dreier
       [not found]           ` <adatyqbsszt.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Roland Dreier @ 2010-05-13 15:48 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Eli Cohen, Linux RDMA list, ewg

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

Right, I'm not saying it shouldn't be in multicast.c.  I'm just
wondering why you don't have the same thing for sa_query.c.

 - R.
-- 
Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]           ` <adatyqbsszt.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
@ 2010-05-13 16:52             ` Eli Cohen
       [not found]               ` <20100513165201.GA19438-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Cohen @ 2010-05-13 16:52 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Eli Cohen, Linux RDMA list, ewg

On Thu, May 13, 2010 at 08:48:06AM -0700, Roland Dreier wrote:
> 
> Right, I'm not saying it shouldn't be in multicast.c.  I'm just
> wondering why you don't have the same thing for sa_query.c.
> 
One reason is that get_src_path_mask() may access ah_lock.
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]               ` <20100513165201.GA19438-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
@ 2010-05-13 17:35                 ` Roland Dreier
       [not found]                   ` <adahbmbso19.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Roland Dreier @ 2010-05-13 17:35 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Eli Cohen, Linux RDMA list, ewg

 > One reason is that get_src_path_mask() may access ah_lock.

I don't see that:

static u8 get_src_path_mask(struct ib_device *device, u8 port_num)
{
	struct ib_sa_device *sa_dev;
	struct ib_sa_port   *port;
	unsigned long flags;
	u8 src_path_mask;

	sa_dev = ib_get_client_data(device, &sa_client);
	if (!sa_dev)
		return 0x7f;

so if we never add the sa_client structure it just returns.

 - R.
-- 
Roland Dreier <rolandd-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]       ` <20100506142849.GC29792-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
@ 2010-05-16  5:17         ` Or Gerlitz
  0 siblings, 0 replies; 10+ messages in thread
From: Or Gerlitz @ 2010-05-16  5:17 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Roland Dreier, Linux RDMA list

Eli Cohen wrote:
> Roland Dreier wrote:
>   
>>  > @@ -1007,7 +1010,7 @@ static void ib_sa_add_one(struct ib_device *device)
>>  > -	sa_dev = kmalloc(sizeof *sa_dev +
>>  > +	sa_dev = kzalloc(sizeof *sa_dev +
>>
>> Do you happen to remember why you needed these kmalloc -> kzalloc conversions?
>>     
> I can't remember why. I do have this habbit of prefering kzalloc over kmalloc because it saves troubles sometimes.
>   
Hi Eli, just a friendly comment, best if such cleanup is done in a 
separate patch, else later someone attempting to debug/bisect (who might 
be yourself btw) could spend a hell of time wondering why it was done 
here and in the framework of this patch...

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [ewg] [PATCHv8 02/11] ib_core: IBoE support only QP1
       [not found]                   ` <adahbmbso19.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
@ 2010-05-17 11:59                     ` Eli Cohen
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Cohen @ 2010-05-17 11:59 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Eli Cohen, Linux RDMA list, ewg

On Thu, May 13, 2010 at 10:35:14AM -0700, Roland Dreier wrote:
>  > One reason is that get_src_path_mask() may access ah_lock.
> 
> I don't see that:
> 
> static u8 get_src_path_mask(struct ib_device *device, u8 port_num)
> {
> 	struct ib_sa_device *sa_dev;
> 	struct ib_sa_port   *port;
> 	unsigned long flags;
> 	u8 src_path_mask;
> 
> 	sa_dev = ib_get_client_data(device, &sa_client);
> 	if (!sa_dev)
> 		return 0x7f;
> 
> so if we never add the sa_client structure it just returns.
> 
I see... so I have no good reason to not treat sa_query.c in the same
manner I did for multicast.c.
By the way, how do you prefer to work out all these comments.  Would
you prefer a new patch set or would you prefer to push the fixed
versions to your tree?
Also, there are a few fixes that I pushed to OFED that were pushed
after I sent V8. How would you like me to communicate them?
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-05-17 11:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-18 17:23 [PATCHv8 02/11] ib_core: IBoE support only QP1 Eli Cohen
2010-05-05 23:12 ` [ewg] " Roland Dreier
     [not found]   ` <ada1vdqaqpc.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
2010-05-06 14:28     ` Eli Cohen
     [not found]       ` <20100506142849.GC29792-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
2010-05-16  5:17         ` Or Gerlitz
2010-05-12 19:56 ` Roland Dreier
     [not found]   ` <ada632svqph.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
2010-05-13  6:59     ` Eli Cohen
     [not found]       ` <20100513065917.GA16073-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
2010-05-13 15:48         ` Roland Dreier
     [not found]           ` <adatyqbsszt.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
2010-05-13 16:52             ` Eli Cohen
     [not found]               ` <20100513165201.GA19438-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
2010-05-13 17:35                 ` Roland Dreier
     [not found]                   ` <adahbmbso19.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
2010-05-17 11:59                     ` Eli Cohen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox