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
[PATCH 1/2] opensm/osm_state_mgr.c: Don't rely on PortInfo:PortState for base SP0
For base SP0, PortState in SM PortInfo attribute is not used and base SP0 is always active. Signed-off-by: Hal Rosenstock h...@mellanox.com --- diff --git a/opensm/osm_state_mgr.c b/opensm/osm_state_mgr.c index 2e1ef94..dd308f2 100644 --- a/opensm/osm_state_mgr.c +++ b/opensm/osm_state_mgr.c @@ -352,7 +352,11 @@ static boolean_t state_mgr_is_sm_port_down(IN osm_sm_t * sm) CL_ASSERT(p_physp); - state = osm_physp_get_port_state(p_physp); + if (p_port-p_node-sw + !ib_switch_info_is_enhanced_port0(p_port-p_node-sw-switch_info)) + state = IB_LINK_ACTIVE; /* base SP0 */ + else + state = osm_physp_get_port_state(p_physp); CL_PLOCK_RELEASE(sm-p_lock); Exit: -- 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 2/2] opensm/osm_perfmgr.c: Don't rely on PortInfo:PortState for base SP0
For base SP0, PortState in SM PortInfo attribute is not used and base SP0 is always active. Signed-off-by: Hal Rosenstock h...@mellanox.com --- diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c index 6a1fa63..86efde6 100644 --- a/opensm/osm_perfmgr.c +++ b/opensm/osm_perfmgr.c @@ -684,6 +684,10 @@ static unsigned is_sm_port_down(osm_sm_t * sm) } CL_PLOCK_RELEASE(sm-p_lock); + if (p_port-p_node-sw + !ib_switch_info_is_enhanced_port0(p_port-p_node-sw-switch_info)) + return 0; /* base SP0 */ + return osm_physp_get_port_state(p_port-p_physp) == IB_LINK_DOWN; } -- 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: Issues with patchwork?
On Sun, Apr 17, 2011 at 3:47 AM, Weiny, Ira K. wei...@llnl.gov wrote: I just checked patchwork and it looks like those patches which were submitted since ~2/25/2011 are no longer there? This includes 3 patches which were in my todo list yesterday. Does anyone know what the issue is? Does look like we lost some data. J.H. -- did the patchwork database get messed up possibly? -- 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
