Skip to contentSkip to conversation

opensm/osm_ucast_cache.c: Cosmetic change to log message for consistency

PATCH In review Hal Rosenstock 4 participants 8 messages Oct 15, 2009 – Oct 18, 2009 3 messages not in the archive InfiniBand

lore
Share & tools
lore

Hal Rosenstock

author
#0 lore raw

PATCH 1/1 opensm/osm_ucast_cache.c: Cosmetic change to log message for consistency

Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/opensm/osm_ucast_cache.c +1 -1
index 31a5333..8db9df2 100644
@@ -876,7 +876,7 @@ void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr,
 	if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH &&
 	    osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) {
 		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Dropping CA-2-CA link - cache invalid\n");
+			"Dropping CA/RTR-2-CA/RTR link - cache invalid\n");
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

opensm/opensm/osm_ucast_cache.c void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr, · line 879

Quoted 13 lines · Hal Rosenstock
Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/opensm/osm_ucast_cache.c +1 -1
index 31a5333..8db9df2 100644
@@ -876,7 +876,7 @@ void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr,
 	if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH &&
 	    osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) {
 		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Dropping CA-2-CA link - cache invalid\n");
+			"Dropping CA/RTR-2-CA/RTR link - cache invalid\n");

IMHO it looks more confusing after the change.

BTW why should OSM_LOG_INFO level be used here?

Sasha

opensm/opensm/osm_ucast_cache.c void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr, · line 882

Quoted 4 lines
opensm/opensm/osm_ucast_cache.c +0 -0
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 16 lines · Sasha Khapyorsky
Quoted 8 lines · Hal Rosenstock
Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/opensm/osm_ucast_cache.c +0 -0
index 31a5333..8db9df2 100644
@@ -876,7 +876,7 @@ void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr,
Quoted 5 lines

      if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH && osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) { OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-                     "Dropping CA-2-CA link - cache invalid\n");
+                     "Dropping CA/RTR-2-CA/RTR link - cache invalid\n");

IMHO it looks more confusing after the change.

It's more accurate and consistent; There's another log message which says "CA/RTR-2-CA/RTR" rather than "CA-2-CA".

-- Hal

Quoted 4 lines

BTW why should OSM_LOG_INFO level be used here?

Sasha

Quoted 9 lines
Quoted 4 lines

              osm_ucast_cache_invalidate(p_mgr);
              goto Exit;
      }

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): f0e08f230910151243o12ec4fffv70f6a278667f55e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#5 lore raw
Quoted 18 lines · Hal Rosenstock
Quoted 16 lines · Sasha Khapyorsky
Quoted 8 lines · Hal Rosenstock
Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/opensm/osm_ucast_cache.c +0 -0
index 31a5333..8db9df2 100644
@@ -876,7 +876,7 @@ void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr,
Quoted 5 lines

      if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH && osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) { OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-                     "Dropping CA-2-CA link - cache invalid\n");
+                     "Dropping CA/RTR-2-CA/RTR link - cache invalid\n");

IMHO it looks more confusing after the change.

Quoted 1 line

It's more accurate and consistent;

This is a log message, not a spec, and 'CA/RTR-2-CA/RTR' as single link abbreviation looks strange. If you want to make things clearer find a clear single name or abbreviation for CA/RTR.

Quoted 2 lines

There's another log message which
says "CA/RTR-2-CA/RTR" rather than "CA-2-CA".

Likely I missed this in those big patch. Would be better to rename back IMHO.

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw

PATCH 1/1 opensm/osm_ucast_cache: reduce OSM_LOG_INFO debug printouts

In reply to Sasha Khapyorsky

Convert debug log messages to use OSM_LOG_DEBUG level (instead of OSM_LOG_INFO), add cache invalidation verbose message (OSM_LOG_VERBOSE level).

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
---
 opensm/opensm/osm_ucast_cache.c |   91 +++++++++++++++++----------------------
 1 files changed, 40 insertions(+), 51 deletions(-)
opensm/opensm/osm_ucast_cache.c +40 -51
index 31a5333..4700c5f 100644
@@ -256,8 +256,8 @@ cache_check_link_change(osm_ucast_mgr_t * p_mgr,
 	     p_physp_1->p_remote_physp->p_remote_physp) ||
 	    (p_physp_2->p_remote_physp &&
 	     p_physp_2->p_remote_physp->p_remote_physp)) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Link location change discovered - cache is invalid\n");
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+			"Link location change discovered\n");
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

@@ -281,27 +281,26 @@ static void cache_remove_port(osm_ucast_mgr_t * p_mgr, uint16_t lid_ho,
 
 	p_cache_sw = cache_get_sw(p_mgr, lid_ho);
 	if (!p_cache_sw) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Found uncached switch/link (lid %u, port %u) - "
-			"cache is invalid\n", lid_ho, port_num);
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+			"Found uncached switch/link (lid %u, port %u)\n",
+			lid_ho, port_num);
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}
 
 	if (port_num >= p_cache_sw->num_ports ||
 	    !p_cache_sw->ports[port_num].remote_lid_ho) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Found uncached switch link (lid %u, port %u) - "
-			"cache is invalid\n", lid_ho, port_num);
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+			"Found uncached switch link (lid %u, port %u)\n",
+			lid_ho, port_num);
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}
 
 	if (p_cache_sw->ports[port_num].remote_lid_ho != remote_lid_ho) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
 			"Remote lid change on switch lid %u, port %u "
-			"(was %u, now %u) - cache is invalid\n",
-			lid_ho, port_num,
+			"(was %u, now %u)\n", lid_ho, port_num,
 			p_cache_sw->ports[port_num].remote_lid_ho,
 			remote_lid_ho);
 		osm_ucast_cache_invalidate(p_mgr);

@@ -310,9 +309,9 @@ static void cache_remove_port(osm_ucast_mgr_t * p_mgr, uint16_t lid_ho,
 
 	if ((p_cache_sw->ports[port_num].is_leaf && !is_ca) ||
 	    (!p_cache_sw->ports[port_num].is_leaf && is_ca)) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Remote node type change on switch lid %u, port %u - "
-			"cache is invalid\n", lid_ho, port_num);
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+			"Remote node type change on switch lid %u, port %u\n",
+			lid_ho, port_num);
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

@@ -406,7 +405,6 @@ void osm_ucast_cache_invalidate(osm_ucast_mgr_t * p_mgr)
 	cache_switch_t *p_next_sw;
 
 	OSM_LOG_ENTER(p_mgr->p_log);
-	OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG, "Invalidating unicast cache\n");
 
 	if (!p_mgr->cache_valid)
 		goto Exit;

@@ -421,6 +419,8 @@ void osm_ucast_cache_invalidate(osm_ucast_mgr_t * p_mgr)
 		cache_sw_destroy(p_sw);
 	}
 	cl_qmap_remove_all(&p_mgr->cache_sw_tbl);
+
+	OSM_LOG(p_mgr->p_log, OSM_LOG_VERBOSE, "Unicast Cache Invalidated.\n");
 Exit:
 	OSM_LOG_EXIT(p_mgr->p_log);
 }

@@ -563,9 +563,9 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 				 *    then its port should have is_new flag on.
 				 */
 				if (p_sw->need_update == 2) {
-					OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-						"New switch found (lid %u) - "
-						"cache is invalid\n", lid_ho);
+					OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+						"New switch found (lid %u)\n",
+						lid_ho);
 					osm_ucast_cache_invalidate(p_mgr);
 					goto Exit;
 				}

@@ -580,9 +580,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 						   additional link that it
 						   didn't have before */
 						OSM_LOG(p_mgr->p_log,
-							OSM_LOG_INFO,
-							"New switch/link found (lid %u) - "
-							"cache is invalid\n",
+							OSM_LOG_DEBUG,
+							"New switch/link found (lid %u)\n",
 							remote_lid_ho);
 						osm_ucast_cache_invalidate
 						    (p_mgr);

@@ -600,9 +599,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 								 (p_remote_physp));
 					if (p_remote_port->is_new) {
 						OSM_LOG(p_mgr->p_log,
-							OSM_LOG_INFO,
-							"New CA/RTR found (lid %u) - "
-							"cache is invalid\n",
+							OSM_LOG_DEBUG,
+							"New CA/RTR found (lid %u)\n",
 							remote_lid_ho);
 						osm_ucast_cache_invalidate
 						    (p_mgr);

@@ -626,9 +624,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 				     remote_node_type == IB_NODE_TYPE_SWITCH) ||
 				    (!p_cache_sw->ports[port_num].is_leaf &&
 				     remote_node_type != IB_NODE_TYPE_SWITCH)) {
-					OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-						"Remote node type change on switch lid %u, port %u - "
-						"cache is invalid\n",
+					OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+						"Remote node type change on switch lid %u, port %u\n",
 						lid_ho, port_num);
 					osm_ucast_cache_invalidate(p_mgr);
 					goto Exit;

@@ -636,9 +633,9 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 
 				if (p_cache_sw->ports[port_num].remote_lid_ho !=
 				    remote_lid_ho) {
-					OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
+					OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
 						"Remote lid change on switch lid %u, port %u"
-						"(was %u, now %u) - cache is invalid\n",
+						"(was %u, now %u)\n",
 						lid_ho, port_num,
 						p_cache_sw->ports[port_num].
 						remote_lid_ho, remote_lid_ho);

@@ -696,9 +693,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 
 		if (p_cache_sw->dropped) {
 			if (!cache_sw_is_leaf(p_cache_sw)) {
-				OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-					"Missing non-leaf switch (lid %u) - "
-					"cache is invalid\n",
+				OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+					"Missing non-leaf switch (lid %u)\n",
 					cache_sw_get_base_lid_ho(p_cache_sw));
 				osm_ucast_cache_invalidate(p_mgr);
 				goto Exit;

@@ -731,9 +727,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 							 remote_lid_ho);
 
 			if (!p_remote_cache_sw || !p_remote_cache_sw->dropped) {
-				OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-					"Switch lid %u, port %u: missing link to existing switch - "
-					"cache is invalid\n",
+				OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+					"Switch lid %u, port %u: missing link to existing switch\n",
 					cache_sw_get_base_lid_ho(p_cache_sw),
 					port_num);
 				osm_ucast_cache_invalidate(p_mgr);

@@ -741,9 +736,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 			}
 
 			if (!cache_sw_is_leaf(p_remote_cache_sw)) {
-				OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-					"Switch lid %u, port %u: missing link to non-leaf switch - "
-					"cache is invalid\n",
+				OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+					"Switch lid %u, port %u: missing link to non-leaf switch\n",
 					cache_sw_get_base_lid_ho(p_cache_sw),
 					port_num);
 				osm_ucast_cache_invalidate(p_mgr);

@@ -761,9 +755,8 @@ static void ucast_cache_validate(osm_ucast_mgr_t * p_mgr)
 			 */
 			if (cache_sw_is_leaf(p_cache_sw) &&
 			    cache_sw_is_leaf(p_remote_cache_sw)) {
-				OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-					"Switch lid %u, port %u: missing leaf-2-leaf link - "
-					"cache is invalid\n",
+				OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+					"Switch lid %u, port %u: missing leaf-2-leaf link\n",
 					cache_sw_get_base_lid_ho(p_cache_sw),
 					port_num);
 				osm_ucast_cache_invalidate(p_mgr);

@@ -808,8 +801,7 @@ void osm_ucast_cache_check_new_link(osm_ucast_mgr_t * p_mgr,
 
 	if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH &&
 	    osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Found CA/RTR-2-CA/RTR link - cache is invalid\n");
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG, "Found CA-2-CA link\n");
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

@@ -875,8 +867,7 @@ void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr,
 
 	if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH &&
 	    osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) {
-		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Dropping CA-2-CA link - cache invalid\n");
+		OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG, "Dropping CA-2-CA link\n");
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

@@ -993,9 +984,8 @@ void osm_ucast_cache_add_node(osm_ucast_mgr_t * p_mgr, osm_node_t * p_node)
 		CL_ASSERT(p_cache_sw);
 
 		if (!cache_sw_is_leaf(p_cache_sw)) {
-			OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-				"Dropped non-leaf switch (lid %u) - "
-				"cache is invalid\n", lid_ho);
+			OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+				"Dropped non-leaf switch (lid %u)\n", lid_ho);
 			osm_ucast_cache_invalidate(p_mgr);
 			goto Exit;
 		}

@@ -1003,9 +993,8 @@ void osm_ucast_cache_add_node(osm_ucast_mgr_t * p_mgr, osm_node_t * p_node)
 		p_cache_sw->dropped = TRUE;
 
 		if (!p_node->sw->num_hops || !p_node->sw->hops) {
-			OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-				"No LID matrices for switch lid %u - "
-				"cache is invalid\n", lid_ho);
+			OSM_LOG(p_mgr->p_log, OSM_LOG_DEBUG,
+				"No LID matrices for switch lid %u\n", lid_ho);
 			osm_ucast_cache_invalidate(p_mgr);
 			goto Exit;
 		}
Signature · 6 lines
1.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#7 lore raw
In reply to Sasha Khapyorsky
Quoted 16 lines · Sasha Khapyorsky

opensm/opensm/osm_ucast_cache.c void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr, · line 879

Quoted 12 lines · Hal Rosenstock
Signed-off-by Hal Rosenstock <[email protected]>

---

opensm/opensm/osm_ucast_cache.c +1 -1
index 31a5333..8db9df2 100644
@@ -876,7 +876,7 @@ void osm_ucast_cache_add_link(osm_ucast_mgr_t * p_mgr,
 	if (osm_node_get_type(p_node_1) != IB_NODE_TYPE_SWITCH &&
 	    osm_node_get_type(p_node_2) != IB_NODE_TYPE_SWITCH) {
 		OSM_LOG(p_mgr->p_log, OSM_LOG_INFO,
-			"Dropping CA-2-CA link - cache invalid\n");
+			"Dropping CA/RTR-2-CA/RTR link - cache invalid\n");

IMHO it looks more confusing after the change.

Quoted 1 line

BTW why should OSM_LOG_INFO level be used here?

If I remember it correctly, there's an INFO message in the ucast mgr saying something like "switches configured for unicast" or "<routing engine name> tables configured", or something like that (I don't have access to the code right now).
So I wanted to see one more line that says that cache is not applicable.

-- Yevgeny

Quoted 7 lines

Sasha

Quoted 4 lines
opensm/opensm/osm_ucast_cache.c +0 -0
 		osm_ucast_cache_invalidate(p_mgr);
 		goto Exit;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw
Quoted 6 lines · Yevgeny Kliteynik

If I remember it correctly, there's an INFO message in the ucast mgr saying something like "switches configured for unicast" or "<routing engine name> tables configured", or something like that (I don't have access to the code right now).

Yes, and there a needs of INFO level is also questionable IMHO.

Quoted 1 line

So I wanted to see one more line

There are more than one:

$ grep OSM_LOG_INFO opensm/osm_ucast_cache.c | wc -l 
19
Quoted 2 lines

that says that
cache is not applicable.

We don't need an extra lines, one line instead (as I did in the posted patch) is enough.

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#10 lore raw
Quoted 7 lines · Sasha Khapyorsky
Quoted 5 lines · Yevgeny Kliteynik

If I remember it correctly, there's an INFO message in the ucast mgr saying something like "switches configured for unicast" or "<routing engine name> tables configured", or something like that (I don't have access to the code right now).

Quoted 7 lines

Yes, and there a needs of INFO level is also questionable IMHO.

Quoted 1 line

So I wanted to see one more line

There are more than one:

$ grep OSM_LOG_INFO opensm/osm_ucast_cache.c | wc -l 19

That's true, but only one of them may be printed
each time the ucast manager runs.

-- Yevgeny

Quoted 8 lines
Quoted 2 lines

that says that
cache is not applicable.

We don't need an extra lines, one line instead (as I did in the posted patch) is enough.

Sasha

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html