Re: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
On 4/17/2011 9:26 PM, Weiny, Ira K. wrote:
On Apr 17, 2011, at 8:21 AM, Hal Rosenstock wrote:
On 4/15/2011 6:17 PM, Ira Weiny wrote:
Subject: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid
port. Otherwise leave it as 0
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 6a1fa63..3e1575a 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,7 +454,9 @@ static void collect_guids(cl_map_item_t * p_map_item,
void *context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
- mon_node-port[port].orig_lid = get_base_lid(node,
port);
+ mon_node-port[port].orig_lid = 0;
+ if (osm_physp_is_valid(node-physp_table[port]))
+ mon_node-port[port].orig_lid =
get_base_lid(node, port);
mon_node-port[port].valid = TRUE;
}
There are several other calls to get_base_lid. Is it also an issue there
too or is port always valid there ? If it's not always valid for those
cases, why not just move this check into get_base_lid ?
I believe this is a special case because we are looping through the ports of
a CA node and not all of it's ports are valid on this fabric. Most of the
other places where get_base_lid is called I believe the port is known to be
good, therefore there is an assertion in there just to protect us.
I think the better way to fix this would probably be to change the perfmgr
to track ports rather than nodes.[*]
Why would tracking ports be better than tracking nodes ? Just to
eliminate this issue ?
Unfortunately this will take a significant amount of coding effort. I think
the best thing to do is just check if the physical port is valid as I did
above.
As to Alex's comment I think the port should be marked invalid as well. (I
should have CC'ed the list on my response to him.) I will resend the patch
with that change.
Ira
[*] At the time I coded the perfmgr it seemed to make more sense to track
nodes rather than ports.
In hindsight this might have been a mistake but for this small place I don't
see a reason to re-architect it all yet.
I'm not following what you're thinking here other than this small issue.
Is there something more behind this ?
-- Hal
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
On Mon, 18 Apr 2011 05:42:56 -0700 Hal Rosenstock h...@dev.mellanox.co.il wrote: On 4/17/2011 9:26 PM, Weiny, Ira K. wrote: On Apr 17, 2011, at 8:21 AM, Hal Rosenstock wrote: On 4/15/2011 6:17 PM, Ira Weiny wrote: [snip] There are several other calls to get_base_lid. Is it also an issue there too or is port always valid there ? If it's not always valid for those cases, why not just move this check into get_base_lid ? I believe this is a special case because we are looping through the ports of a CA node and not all of it's ports are valid on this fabric. Most of the other places where get_base_lid is called I believe the port is known to be good, therefore there is an assertion in there just to protect us. I think the better way to fix this would probably be to change the perfmgr to track ports rather than nodes.[*] Why would tracking ports be better than tracking nodes ? Just to eliminate this issue ? Unfortunately this will take a significant amount of coding effort. I think the best thing to do is just check if the physical port is valid as I did above. As to Alex's comment I think the port should be marked invalid as well. (I should have CC'ed the list on my response to him.) I will resend the patch with that change. Ira [*] At the time I coded the perfmgr it seemed to make more sense to track nodes rather than ports. In hindsight this might have been a mistake but for this small place I don't see a reason to re-architect it all yet. I'm not following what you're thinking here other than this small issue. Is there something more behind this ? I agree that it would not be worth the effort to fix this minor issue. My thoughts were: we are tracking values which are provided by ports not nodes. (They are PortCounters after all.) So in some ways it makes sense to keep track of all the ports in the system. However, I did think about it more after my email and tracking ports may have other disadvantages. One example I could think of was that the perfmgr keeps it's own list of monitored nodes. This list would be much larger if we were tracking ports. So finding the monitored port would be slower when processing response mads. Would that be a big deal? Perhaps not. I would have to do some performance tests. In the end I just wanted to acknowledge that there might be another way. :-D Ira -- Hal -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Ira Weiny Math Programmer/Computer Scientist Lawrence Livermore National Lab 925-423-8008 wei...@llnl.gov -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
Hi Ira,
On 15:17 Fri 15 Apr , Ira Weiny wrote:
Subject: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid
port. Otherwise leave it as 0
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 6a1fa63..3e1575a 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,7 +454,9 @@ static void collect_guids(cl_map_item_t * p_map_item,
void *context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
- mon_node-port[port].orig_lid = get_base_lid(node,
port);
+ mon_node-port[port].orig_lid = 0;
+ if (osm_physp_is_valid(node-physp_table[port]))
+ mon_node-port[port].orig_lid =
get_base_lid(node, port);
mon_node-port[port].valid = TRUE;
Shouldn't this port marked with mon_node-port[port].valid = FALSE ?
-- Alex
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
On 4/15/2011 6:17 PM, Ira Weiny wrote:
Subject: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid
port. Otherwise leave it as 0
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 6a1fa63..3e1575a 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,7 +454,9 @@ static void collect_guids(cl_map_item_t * p_map_item,
void *context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
- mon_node-port[port].orig_lid = get_base_lid(node,
port);
+ mon_node-port[port].orig_lid = 0;
+ if (osm_physp_is_valid(node-physp_table[port]))
+ mon_node-port[port].orig_lid =
get_base_lid(node, port);
mon_node-port[port].valid = TRUE;
}
There are several other calls to get_base_lid. Is it also an issue there
too or is port always valid there ? If it's not always valid for those
cases, why not just move this check into get_base_lid ?
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
On Apr 17, 2011, at 8:21 AM, Hal Rosenstock wrote:
On 4/15/2011 6:17 PM, Ira Weiny wrote:
Subject: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid
port. Otherwise leave it as 0
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 6a1fa63..3e1575a 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,7 +454,9 @@ static void collect_guids(cl_map_item_t * p_map_item,
void *context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
-mon_node-port[port].orig_lid = get_base_lid(node,
port);
+mon_node-port[port].orig_lid = 0;
+if (osm_physp_is_valid(node-physp_table[port]))
+mon_node-port[port].orig_lid =
get_base_lid(node, port);
mon_node-port[port].valid = TRUE;
}
There are several other calls to get_base_lid. Is it also an issue there
too or is port always valid there ? If it's not always valid for those
cases, why not just move this check into get_base_lid ?
I believe this is a special case because we are looping through the ports of a
CA node and not all of it's ports are valid on this fabric. Most of the other
places where get_base_lid is called I believe the port is known to be good,
therefore there is an assertion in there just to protect us.
I think the better way to fix this would probably be to change the perfmgr to
track ports rather than nodes.[*] Unfortunately this will take a significant
amount of coding effort. I think the best thing to do is just check if the
physical port is valid as I did above.
As to Alex's comment I think the port should be marked invalid as well. (I
should have CC'ed the list on my response to him.) I will resend the patch
with that change.
Ira
[*] At the time I coded the perfmgr it seemed to make more sense to track nodes
rather than ports. In hindsight this might have been a mistake but for this
small place I don't see a reason to re-architect it all yet.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
On Apr 17, 2011, at 8:06 AM, Alex Netes wrote:
Hi Ira,
On 15:17 Fri 15 Apr , Ira Weiny wrote:
Subject: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid
port. Otherwise leave it as 0
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 6a1fa63..3e1575a 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,7 +454,9 @@ static void collect_guids(cl_map_item_t * p_map_item,
void *context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
-mon_node-port[port].orig_lid = get_base_lid(node,
port);
+mon_node-port[port].orig_lid = 0;
+if (osm_physp_is_valid(node-physp_table[port]))
+mon_node-port[port].orig_lid =
get_base_lid(node, port);
mon_node-port[port].valid = TRUE;
Shouldn't this port marked with mon_node-port[port].valid = FALSE ?
New patch below.
Subject: [PATCH 1/5] opensm/perfmgr: set redirect orig_lid and valid flag only
when we have a valid port.
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 5758587..3578e13 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,8 +454,12 @@ static void collect_guids(cl_map_item_t * p_map_item, void
*context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
- mon_node-port[port].orig_lid = get_base_lid(node,
port);
- mon_node-port[port].valid = TRUE;
+ mon_node-port[port].orig_lid = 0;
+ mon_node-port[port].valid = FALSE;
+ if (osm_physp_is_valid(node-physp_table[port])) {
+ mon_node-port[port].orig_lid =
get_base_lid(node, port);
+ mon_node-port[port].valid = TRUE;
+ }
}
cl_qmap_insert(pm-monitored_map, node_guid,
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] opensm: perfmgr, only set orig_lid when we have a valid port. Otherwise leave it as 0
Subject: [PATCH] opensm: perfmgr, only set orig_lid when we have a valid port.
Otherwise leave it as 0
Signed-off-by: Ira Weiny wei...@llnl.gov
---
opensm/osm_perfmgr.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
index 6a1fa63..3e1575a 100644
--- a/opensm/osm_perfmgr.c
+++ b/opensm/osm_perfmgr.c
@@ -454,7 +454,9 @@ static void collect_guids(cl_map_item_t * p_map_item, void
*context)
ib_switch_info_is_enhanced_port0(node-sw-
switch_info));
for (port = mon_node-esp0 ? 0 : 1; port num_ports; port++) {
- mon_node-port[port].orig_lid = get_base_lid(node,
port);
+ mon_node-port[port].orig_lid = 0;
+ if (osm_physp_is_valid(node-physp_table[port]))
+ mon_node-port[port].orig_lid =
get_base_lid(node, port);
mon_node-port[port].valid = TRUE;
}
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
