diff mbox

[v7,04/23] IB/Verbs: Reform IB-core cm

Message ID 1430233823-7075-5-git-send-email-yun.wang@profitbricks.com (mailing list archive)
State Rejected
Headers show

Commit Message

Michael Wang April 28, 2015, 3:10 p.m. UTC 
Use raw management helpers to reform IB-core cm.

Cc: Hal Rosenstock <hal@dev.mellanox.co.il>
Cc: Steve Wise <swise@opengridcomputing.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Signed-off-by: Michael Wang <yun.wang@profitbricks.com>
---
 drivers/infiniband/core/cm.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Or Gerlitz April 28, 2015, 7:02 p.m. UTC | #1 
On Tue, Apr 28, 2015 at 6:10 PM, Michael Wang <yun.wang@profitbricks.com> wrote:
> Use raw management helpers to reform IB-core cm.
>
> Cc: Hal Rosenstock <hal@dev.mellanox.co.il>
> Cc: Steve Wise <swise@opengridcomputing.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Sean Hefty <sean.hefty@intel.com>
> Signed-off-by: Michael Wang <yun.wang@profitbricks.com>
> ---
>  drivers/infiniband/core/cm.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)

Hi Michael,

I don't really see the benefit (e.g for someone doing bisection
1/2/5/10 years from now and landing here) of listing all the group of
reviewers for each of the ~30 patches that make this series, any
special reason that caused you doing so?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Wang April 29, 2015, 7:40 a.m. UTC | #2 
Hi, Or

On 04/28/2015 09:02 PM, Or Gerlitz wrote:
> On Tue, Apr 28, 2015 at 6:10 PM, Michael Wang <yun.wang@profitbricks.com> wrote:
>> Use raw management helpers to reform IB-core cm.
>>
>> Cc: Hal Rosenstock <hal@dev.mellanox.co.il>
>> Cc: Steve Wise <swise@opengridcomputing.com>
>> Cc: Tom Talpey <tom@talpey.com>
>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>> Cc: Doug Ledford <dledford@redhat.com>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Cc: Sean Hefty <sean.hefty@intel.com>
>> Signed-off-by: Michael Wang <yun.wang@profitbricks.com>
>> ---
>>  drivers/infiniband/core/cm.c | 20 +++++++++++++++++---
>>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> Hi Michael,
> 
> I don't really see the benefit (e.g for someone doing bisection
> 1/2/5/10 years from now and landing here) of listing all the group of
> reviewers for each of the ~30 patches that make this series, any
> special reason that caused you doing so?

Those on the CC list are used to help correct some problems or
contributed to the definition/plan :-) They are familiar with
the whole story of this patch set.

As you mentioned, few years later when someone bisect out the patches
and want to learn why it's like that, he could have enough address to
send his question, although few of them may not work on the same aspect
anymore, but the chance to find someone have the story is higher.

I think the CC list is not that big for a patch set covered such a wide
range, isn't it :-P

Regards,
Michael Wang

> 
> Or.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz April 29, 2015, 8:17 a.m. UTC | #3 
On 4/29/2015 10:40 AM, Michael Wang wrote:
> On 04/28/2015 09:02 PM, Or Gerlitz wrote:
>> >On Tue, Apr 28, 2015 at 6:10 PM, Michael Wang<yun.wang@profitbricks.com>  wrote:
>>> >>Use raw management helpers to reform IB-core cm.
>>> >>
>>> >>Cc: Hal Rosenstock<hal@dev.mellanox.co.il>
>>> >>Cc: Steve Wise<swise@opengridcomputing.com>
>>> >>Cc: Tom Talpey<tom@talpey.com>
>>> >>Cc: Jason Gunthorpe<jgunthorpe@obsidianresearch.com>
>>> >>Cc: Doug Ledford<dledford@redhat.com>
>>> >>Cc: Ira Weiny<ira.weiny@intel.com>
>>> >>Cc: Sean Hefty<sean.hefty@intel.com>
>>> >>Signed-off-by: Michael Wang<yun.wang@profitbricks.com>
>>> >>---
>>> >>  drivers/infiniband/core/cm.c | 20 +++++++++++++++++---
>>> >>  1 file changed, 17 insertions(+), 3 deletions(-)
>> >
>> >Hi Michael,
>> >
>> >I don't really see the benefit (e.g for someone doing bisection
>> >1/2/5/10 years from now and landing here) of listing all the group of
>> >reviewers for each of the ~30 patches that make this series, any
>> >special reason that caused you doing so?
> Those on the CC list are used to help correct some problems or
> contributed to the definition/plan:-)  They are familiar with
> the whole story of this patch set.
>
> As you mentioned, few years later when someone bisect out the patches
> and want to learn why it's like that, he could have enough address to
> send his question, although few of them may not work on the same aspect
> anymore, but the chance to find someone have the story is higher.
>

The kernel development model works well for both driver and core 
patches, w.o adding 7-10
people as CC to patches.

> I think the CC list is not that big for a patch set covered such a wide
> range, isn't it:-P

Maybe it's a matter of taste, but for me it look way way too big. If you 
really want to have such
a huge listing, do it in the early patches of the series where you 
introduce the new concepts, and later,on downstream patches, when you 
use it, put one person if they happen to be the author or maintainthat 
area (e.g Sean <-- CM/CMA, Doug/Erez <-- IPoIB Ira, Hal <-- MAD, Steve 
<-- IW_CM, etc)

Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz April 29, 2015, 3:48 p.m. UTC | #4 
On Wed, Apr 29, 2015 at 10:40 AM, Michael Wang
<yun.wang@profitbricks.com> wrote:
> Hi, Or
>
> On 04/28/2015 09:02 PM, Or Gerlitz wrote:
>> On Tue, Apr 28, 2015 at 6:10 PM, Michael Wang <yun.wang@profitbricks.com> wrote:
>>> Use raw management helpers to reform IB-core cm.
>>>
>>> Cc: Hal Rosenstock <hal@dev.mellanox.co.il>
>>> Cc: Steve Wise <swise@opengridcomputing.com>
>>> Cc: Tom Talpey <tom@talpey.com>
>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>> Cc: Doug Ledford <dledford@redhat.com>
>>> Cc: Ira Weiny <ira.weiny@intel.com>
>>> Cc: Sean Hefty <sean.hefty@intel.com>
>>> Signed-off-by: Michael Wang <yun.wang@profitbricks.com>
>>> ---
>>>  drivers/infiniband/core/cm.c | 20 +++++++++++++++++---
>>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> Hi Michael,
>>
>> I don't really see the benefit (e.g for someone doing bisection
>> 1/2/5/10 years from now and landing here) of listing all the group of
>> reviewers for each of the ~30 patches that make this series, any
>> special reason that caused you doing so?
>
> Those on the CC list are used to help correct some problems or
> contributed to the definition/plan :-) They are familiar with
> the whole story of this patch set.
>
> As you mentioned, few years later when someone bisect out the patches
> and want to learn why it's like that, he could have enough address to
> send his question, although few of them may not work on the same aspect
> anymore, but the chance to find someone have the story is higher.

The kernel development model works well for both driver and core
patches, w.o adding 7-10
people as CC to patches.

>
> I think the CC list is not that big for a patch set covered such a wide
> range, isn't it :-P

Maybe it's a matter of taste, but for me it look way way too big. If
you really want to have such
a huge listing, do it in the early patches of the series where you
introduce the new concepts, and later,on downstream patches, when you
use it, put one person if they happen to be the author or maintainthat
area (e.g Sean <-- CM/CMA, Doug/Erez <-- IPoIB Ira, Hal <-- MAD, Steve
<-- IW_CM, etc)

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Wang April 30, 2015, 7:28 a.m. UTC | #5 
On 04/29/2015 05:48 PM, Or Gerlitz wrote:
[snip]
> 
>>
>> I think the CC list is not that big for a patch set covered such a wide
>> range, isn't it :-P
> 
> Maybe it's a matter of taste, but for me it look way way too big. If
> you really want to have such
> a huge listing, do it in the early patches of the series where you
> introduce the new concepts, and later,on downstream patches, when you
> use it, put one person if they happen to be the author or maintainthat
> area (e.g Sean <-- CM/CMA, Doug/Erez <-- IPoIB Ira, Hal <-- MAD, Steve
> <-- IW_CM, etc)

Thanks for the suggestion, I can't callback correctly who participated
on which part of the review accurately... my bad, will take care next
time :-) and will stop add new CC from now on.

BTW, as now folks already familiar with the cap_XX stuff, may be
the last version to be applied could merge all the cap_XX into one,
after all, it's more focus on the description rather than logical,
separate cap_XX won't help easier the review anyway.

Regards,
Michael Wang

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

Patch

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index e28a494..add5e484 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -3760,11 +3760,9 @@  static void cm_add_one(struct ib_device *ib_device)
 	};
 	unsigned long flags;
 	int ret;
+	int count = 0;
 	u8 i;
 
-	if (rdma_node_get_transport(ib_device->node_type) != RDMA_TRANSPORT_IB)
-		return;
-
 	cm_dev = kzalloc(sizeof(*cm_dev) + sizeof(*port) *
 			 ib_device->phys_port_cnt, GFP_KERNEL);
 	if (!cm_dev)
@@ -3783,6 +3781,9 @@  static void cm_add_one(struct ib_device *ib_device)
 
 	set_bit(IB_MGMT_METHOD_SEND, reg_req.method_mask);
 	for (i = 1; i <= ib_device->phys_port_cnt; i++) {
+		if (!rdma_ib_or_iboe(ib_device, i))
+			continue;
+
 		port = kzalloc(sizeof *port, GFP_KERNEL);
 		if (!port)
 			goto error1;
@@ -3809,7 +3810,13 @@  static void cm_add_one(struct ib_device *ib_device)
 		ret = ib_modify_port(ib_device, i, 0, &port_modify);
 		if (ret)
 			goto error3;
+
+		count++;
 	}
+
+	if (!count)
+		goto free;
+
 	ib_set_client_data(ib_device, &cm_client, cm_dev);
 
 	write_lock_irqsave(&cm.device_lock, flags);
@@ -3825,11 +3832,15 @@  error1:
 	port_modify.set_port_cap_mask = 0;
 	port_modify.clr_port_cap_mask = IB_PORT_CM_SUP;
 	while (--i) {
+		if (!rdma_ib_or_iboe(ib_device, i))
+			continue;
+
 		port = cm_dev->port[i-1];
 		ib_modify_port(ib_device, port->port_num, 0, &port_modify);
 		ib_unregister_mad_agent(port->mad_agent);
 		cm_remove_port_fs(port);
 	}
+free:
 	device_unregister(cm_dev->device);
 	kfree(cm_dev);
 }
@@ -3853,6 +3864,9 @@  static void cm_remove_one(struct ib_device *ib_device)
 	write_unlock_irqrestore(&cm.device_lock, flags);
 
 	for (i = 1; i <= ib_device->phys_port_cnt; i++) {
+		if (!rdma_ib_or_iboe(ib_device, i))
+			continue;
+
 		port = cm_dev->port[i-1];
 		ib_modify_port(ib_device, port->port_num, 0, &port_modify);
 		ib_unregister_mad_agent(port->mad_agent);