From: Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
To: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
Cc: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
"linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH 02/06] opensm/perfmgr: clean up: break out redirect processing from pc_recv_process
Date: Tue, 26 Feb 2013 10:34:56 -0500 [thread overview]
Message-ID: <512CD620.80009@dev.mellanox.co.il> (raw)
In-Reply-To: <20130221133341.0f4083019b5b62c4ca5f24a8-i2BcT+NCU+M@public.gmane.org>
On 2/21/2013 4:33 PM, Ira Weiny wrote:
>
>
> Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
> ---
> opensm/osm_perfmgr.c | 200 ++++++++++++++++++++++++--------------------------
> 1 files changed, 96 insertions(+), 104 deletions(-)
>
> diff --git a/opensm/osm_perfmgr.c b/opensm/osm_perfmgr.c
> index c71111f..69b3d77 100644
> --- a/opensm/osm_perfmgr.c
> +++ b/opensm/osm_perfmgr.c
> @@ -1263,6 +1263,96 @@ Exit:
> return pkey_ix;
> }
>
> +static void handle_redirect(osm_perfmgr_t *pm,
> + ib_class_port_info_t *cpi,
> + monitored_node_t *p_mon_node,
> + uint8_t port,
> + osm_madw_context_t *mad_context)
> +{
> + char gid_str[INET6_ADDRSTRLEN];
> + ib_api_status_t status;
> + boolean_t valid = TRUE;
> + int16_t pkey_ix = 0;
> +
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Redirection to LID %u GID %s QP 0x%x received\n",
> + cl_ntoh16(cpi->redir_lid),
> + inet_ntop(AF_INET6, cpi->redir_gid.raw, gid_str,
> + sizeof gid_str), cl_ntoh32(cpi->redir_qp));
> +
> + if (!pm->subn->opt.perfmgr_redir) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Redirection requested but disabled\n");
> + valid = FALSE;
> + }
> +
> + /* valid redirection ? */
> + if (cpi->redir_lid == 0) {
> + if (!ib_gid_is_notzero(&cpi->redir_gid)) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Invalid redirection "
> + "(both redirect LID and GID are zero)\n");
> + valid = FALSE;
> + }
> + }
> + if (cpi->redir_qp == 0) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQP\n");
> + valid = FALSE;
> + }
> + if (cpi->redir_pkey == 0) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectP_Key\n");
> + valid = FALSE;
> + }
> + if (cpi->redir_qkey != IB_QP1_WELL_KNOWN_Q_KEY) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQ_Key\n");
> + valid = FALSE;
> + }
> +
> + pkey_ix = validate_redir_pkey(pm, cpi->redir_pkey);
> + if (pkey_ix == -1) {
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "Index for Pkey 0x%x not found\n",
> + cl_ntoh16(cpi->redir_pkey));
> + valid = FALSE;
> + }
> +
> + if (cpi->redir_lid == 0) {
> + /* GID redirection: get PathRecord information */
> + OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> + "GID redirection not currently supported\n");
> + return;
> + }
> +
Better with:
if (!valid)
return;
here ?
> + /* LID redirection support (easier than GID redirection) */
> + cl_plock_acquire(&pm->osm->lock);
Port number is already validated before handle_redirect is called, right ?
> + p_mon_node->port[port].redirection = TRUE;
> + p_mon_node->port[port].valid = valid;
> + memcpy(&p_mon_node->port[port].gid, &cpi->redir_gid,
> + sizeof(ib_gid_t));
> + p_mon_node->port[port].lid = cpi->redir_lid;
> + p_mon_node->port[port].qp = cpi->redir_qp;
> + p_mon_node->port[port].pkey = cpi->redir_pkey;
> + if (pkey_ix != -1)
> + p_mon_node->port[port].pkey_ix = pkey_ix;
Add:
p_mon_node->port[port].cpi_valid = FALSE;
here
> + cl_plock_release(&pm->osm->lock);
> +
> + if (valid) {
If check for valid above LID redirection comment, don't need if (valid)
clause here.
> + /* Finally, issue a CPI query to the redirected location */
> + cl_plock_acquire(&pm->osm->lock);
> + p_mon_node->port[port].cpi_valid = FALSE;
> + cl_plock_release(&pm->osm->lock);
Eliminate extra lock/unlock and move cpi_valid above where indicated.
-- Hal
> + status = perfmgr_send_cpi_mad(pm, cpi->redir_lid,
> + cpi->redir_qp, pkey_ix,
> + port, mad_context,
> + 0); /* FIXME SL != 0 */
> + if (status != IB_SUCCESS)
> + OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5414: "
> + "Failed to send redirected CPI MAD "
> + "for node %s (0x%" PRIx64 ") port %d\n",
> + p_mon_node->name, p_mon_node->guid, port);
> + }
> +}
> +
> /**********************************************************************
> * The dispatcher uses a thread pool which will call this function when
> * there is a thread available to process the mad received on the wire.
> @@ -1281,8 +1371,6 @@ static void pc_recv_process(void *context, void *data)
> perfmgr_db_data_cnt_reading_t data_reading;
> cl_map_item_t *p_node;
> monitored_node_t *p_mon_node;
> - int16_t pkey_ix = 0;
> - boolean_t valid = TRUE;
> ib_class_port_info_t *cpi = NULL;
>
> OSM_LOG_ENTER(pm->log);
> @@ -1334,112 +1422,15 @@ static void pc_recv_process(void *context, void *data)
> p_mon_node->port[port].cpi_valid = TRUE;
> }
> cl_plock_release(&pm->osm->lock);
> - }
> -
> - /* Response could also be redirection (IBM eHCA PMA does this) */
> - if (p_mad->status & IB_MAD_STATUS_REDIRECT) {
> - char gid_str[INET6_ADDRSTRLEN];
> - ib_api_status_t status;
>
> - CL_ASSERT(cpi); /* Redirect should have returned CPI
> - (processed in previous block) */
> -
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Redirection to LID %u GID %s QP 0x%x received\n",
> - cl_ntoh16(cpi->redir_lid),
> - inet_ntop(AF_INET6, cpi->redir_gid.raw, gid_str,
> - sizeof gid_str), cl_ntoh32(cpi->redir_qp));
> + /* Response could also be redirection (IBM eHCA PMA does this) */
> + if (p_mad->status & IB_MAD_STATUS_REDIRECT)
> + handle_redirect(pm, cpi, p_mon_node, port,
> + mad_context);
>
> - if (!pm->subn->opt.perfmgr_redir) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Redirection requested but disabled\n");
> - valid = FALSE;
> - }
> -
> - /* valid redirection ? */
> - if (cpi->redir_lid == 0) {
> - if (!ib_gid_is_notzero(&cpi->redir_gid)) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Invalid redirection "
> - "(both redirect LID and GID are zero)\n");
> - valid = FALSE;
> - }
> - }
> - if (cpi->redir_qp == 0) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQP\n");
> - valid = FALSE;
> - }
> - if (cpi->redir_pkey == 0) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectP_Key\n");
> - valid = FALSE;
> - }
> - if (cpi->redir_qkey != IB_QP1_WELL_KNOWN_Q_KEY) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE, "Invalid RedirectQ_Key\n");
> - valid = FALSE;
> - }
> -
> - pkey_ix = validate_redir_pkey(pm, cpi->redir_pkey);
> - if (pkey_ix == -1) {
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "Index for Pkey 0x%x not found\n",
> - cl_ntoh16(cpi->redir_pkey));
> - valid = FALSE;
> - }
> -
> - if (cpi->redir_lid == 0) {
> - /* GID redirection: get PathRecord information */
> - OSM_LOG(pm->log, OSM_LOG_VERBOSE,
> - "GID redirection not currently supported\n");
> - goto Exit;
> - }
> -
> - /* LID redirection support (easier than GID redirection) */
> - cl_plock_acquire(&pm->osm->lock);
> - /* Now, validate port number */
> - if (port >= p_mon_node->num_ports) {
> - cl_plock_release(&pm->osm->lock);
> - OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5413: "
> - "Invalid port num %d for GUID 0x%016"
> - PRIx64 " num ports %d\n", port, node_guid,
> - p_mon_node->num_ports);
> - goto Exit;
> - }
> - p_mon_node->port[port].redirection = TRUE;
> - p_mon_node->port[port].valid = valid;
> - memcpy(&p_mon_node->port[port].gid, &cpi->redir_gid,
> - sizeof(ib_gid_t));
> - p_mon_node->port[port].lid = cpi->redir_lid;
> - p_mon_node->port[port].qp = cpi->redir_qp;
> - p_mon_node->port[port].pkey = cpi->redir_pkey;
> - if (pkey_ix != -1)
> - p_mon_node->port[port].pkey_ix = pkey_ix;
> - cl_plock_release(&pm->osm->lock);
> -
> - if (!valid)
> - goto Exit;
> -
> - /* Finally, issue a CPI query to the redirected location */
> - p_mon_node->port[port].cpi_valid = FALSE;
> - status = perfmgr_send_cpi_mad(pm, cpi->redir_lid,
> - cpi->redir_qp, pkey_ix,
> - port, mad_context,
> - 0); /* FIXME SL != 0 */
> - if (status != IB_SUCCESS)
> - OSM_LOG(pm->log, OSM_LOG_ERROR, "ERR 5414: "
> - "Failed to send redirected MAD "
> - "with method 0x%x for node %s "
> - "(NodeGuid 0x%" PRIx64 ") port %d\n",
> - mad_context->perfmgr_context.mad_method,
> - p_mon_node->name, node_guid, port);
> goto Exit;
> }
>
> - /* ClassPortInfo needed to process optional Redirection
> - * now exit normally
> - */
> - if (p_mad->attr_id == IB_MAD_ATTR_CLASS_PORT_INFO)
> - goto Exit;
> -
> perfmgr_db_fill_err_read(wire_read, &err_reading);
> /* FIXME separate query for extended counters if they are supported
> * on the port.
> @@ -1465,7 +1456,8 @@ static void pc_recv_process(void *context, void *data)
> perfmgr_db_clear_prev_dc(pm->db, node_guid, port);
> }
>
> - perfmgr_check_overflow(pm, p_mon_node, pkey_ix, port, wire_read);
> + perfmgr_check_overflow(pm, p_mon_node, p_mon_node->port[port].pkey_ix,
> + port, wire_read);
>
> #ifdef ENABLE_OSM_PERF_MGR_PROFILE
> do {
--
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
next prev parent reply other threads:[~2013-02-26 15:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-21 21:33 [PATCH 02/06] opensm/perfmgr: clean up: break out redirect processing from pc_recv_process Ira Weiny
[not found] ` <20130221133341.0f4083019b5b62c4ca5f24a8-i2BcT+NCU+M@public.gmane.org>
2013-02-26 15:34 ` Hal Rosenstock [this message]
[not found] ` <512CD620.80009-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-02-26 19:34 ` Ira Weiny
[not found] ` <20130226113455.e1a10ece200032d8cdf703aa-i2BcT+NCU+M@public.gmane.org>
2013-02-26 19:58 ` Hal Rosenstock
[not found] ` <512D13C9.4070309-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2013-02-26 21:28 ` Ira Weiny
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=512CD620.80009@dev.mellanox.co.il \
--to=hal-ldsdmyg8hgv8yrgs2mwiifqbs+8scbdb@public.gmane.org \
--cc=hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
--cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=weiny2-i2BcT+NCU+M@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox