From: "Saleem, Shiraz" <shiraz.saleem@intel.com>
To: Joe Perches <joe@perches.com>,
"Ismail, Mustafa" <mustafa.ismail@intel.com>
Cc: Doug Ledford <dledford@redhat.com>,
Jason Gunthorpe <jgg@ziepe.ca>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: RE: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
Date: Tue, 8 Jun 2021 16:18:03 +0000 [thread overview]
Message-ID: <38e0a7b9c19543c2aee679bdd346c5a9@intel.com> (raw)
In-Reply-To: <6147b2a2a5992525482f2cf61d2939b110062d8c.camel@perches.com>
> Subject: Re: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
>
> On Mon, 2021-06-07 at 22:40 +0000, Saleem, Shiraz wrote:
> > > Subject: Re: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
> > >
> > > On Mon, 2021-06-07 at 14:54 +0000, Saleem, Shiraz wrote:
> > > > > Subject: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
> > > > >
> > > > > There are some odd mismatches in field and access index.
> > > > > These may be simple cut/paste typos.
> > > > >
> > > > > Are these intentional?
> > > >
> > > > No. Accidental. Likely cut/copy mistake. I will send a fix. Thanks
> > > > Joe!
> > >
> > > Reason is I was refactoring what I thought was rather templated and
> > > overly verbose code.
> > >
> > > Here's a possible patch (with probable corrections to these index
> > > typos):
> >
> > Looks reasonable to me. Wondering if irdma_update_stats should also
> > use your IRDMA_STAT macro.
>
> Probably, and it could really a macro or two of its own.
>
> And it looks like that block has it's own copy/paste defects.
>
> Maybe:
> ---
> drivers/infiniband/hw/irdma/ctrl.c | 219 +++++++++----------------------------
> 1 file changed, 51 insertions(+), 168 deletions(-)
>
> diff --git a/drivers/infiniband/hw/irdma/ctrl.c b/drivers/infiniband/hw/irdma/ctrl.c
> index 5aa112067bce8..90c6f82ea0c8d 100644
> --- a/drivers/infiniband/hw/irdma/ctrl.c
> +++ b/drivers/infiniband/hw/irdma/ctrl.c
> @@ -5484,174 +5484,57 @@ void irdma_update_stats(struct irdma_dev_hw_stats
> *hw_stats, {
> u64 *stats_val = hw_stats->stats_val_32;
>
> - stats_val[IRDMA_HW_STAT_INDEX_RXVLANERR] +=
> - IRDMA_STATS_DELTA(gather_stats->rxvlanerr,
> - last_gather_stats->rxvlanerr,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4RXDISCARD] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4rxdiscard,
> - last_gather_stats->ip4rxdiscard,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4RXTRUNC] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4rxtrunc,
> - last_gather_stats->ip4rxtrunc,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4TXNOROUTE] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4txnoroute,
> - last_gather_stats->ip4txnoroute,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6RXDISCARD] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6rxdiscard,
> - last_gather_stats->ip6rxdiscard,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6RXTRUNC] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6rxtrunc,
> - last_gather_stats->ip6rxtrunc,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6TXNOROUTE] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6txnoroute,
> - last_gather_stats->ip6txnoroute,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_TCPRTXSEG] +=
> - IRDMA_STATS_DELTA(gather_stats->tcprtxseg,
> - last_gather_stats->tcprtxseg,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_TCPRXOPTERR] +=
> - IRDMA_STATS_DELTA(gather_stats->tcprxopterr,
> - last_gather_stats->tcprxopterr,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_TCPRXPROTOERR] +=
> - IRDMA_STATS_DELTA(gather_stats->tcprxprotoerr,
> - last_gather_stats->tcprxprotoerr,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_RXRPCNPHANDLED] +=
> - IRDMA_STATS_DELTA(gather_stats->rxrpcnphandled,
> - last_gather_stats->rxrpcnphandled,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_RXRPCNPIGNORED] +=
> - IRDMA_STATS_DELTA(gather_stats->rxrpcnpignored,
> - last_gather_stats->rxrpcnpignored,
> - IRDMA_MAX_STATS_32);
> - stats_val[IRDMA_HW_STAT_INDEX_TXNPCNPSENT] +=
> - IRDMA_STATS_DELTA(gather_stats->txnpcnpsent,
> - last_gather_stats->txnpcnpsent,
> - IRDMA_MAX_STATS_32);
> +#define irdma_update_stat(index, var, size) \
Maybe var --> name. Other than that looks good.
> + stats_val[IRDMA_STAT(index)] += \
> + IRDMA_STATS_DELTA(gather_stats->var, \
> + last_gather_stats->var, \
> + IRDMA_MAX_STATS_##size)
> +
> + irdma_update_stat(RXVLANERR, rxvlanerr, 32);
> + irdma_update_stat(IP4RXDISCARD, ip4rxdiscard, 32);
> + irdma_update_stat(IP4RXTRUNC, ip4rxtrunc, 32);
> + irdma_update_stat(IP4TXNOROUTE, ip4txnoroute, 32);
> + irdma_update_stat(IP6RXDISCARD, ip6rxdiscard, 32);
> + irdma_update_stat(IP6RXTRUNC, ip6rxtrunc, 32);
> + irdma_update_stat(IP6TXNOROUTE, ip6txnoroute, 32);
> + irdma_update_stat(TCPRTXSEG, tcprtxseg, 32);
> + irdma_update_stat(TCPRXOPTERR, tcprxopterr, 32);
> + irdma_update_stat(TCPRXPROTOERR, tcprxprotoerr, 32);
> + irdma_update_stat(RXRPCNPHANDLED, rxrpcnphandled, 32);
> + irdma_update_stat(RXRPCNPIGNORED, rxrpcnpignored, 32);
> + irdma_update_stat(TXNPCNPSENT, txnpcnpsent, 32);
> +
> stats_val = hw_stats->stats_val_64;
> - stats_val[IRDMA_HW_STAT_INDEX_IP4RXOCTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4rxocts,
> - last_gather_stats->ip4rxocts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4RXPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4rxpkts,
> - last_gather_stats->ip4rxpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4RXFRAGS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4txfrag,
> - last_gather_stats->ip4txfrag,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4RXMCPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4rxmcpkts,
> - last_gather_stats->ip4rxmcpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4TXOCTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4txocts,
> - last_gather_stats->ip4txocts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4TXPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4txpkts,
> - last_gather_stats->ip4txpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4TXFRAGS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4txfrag,
> - last_gather_stats->ip4txfrag,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP4TXMCPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip4txmcpkts,
> - last_gather_stats->ip4txmcpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6RXOCTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6rxocts,
> - last_gather_stats->ip6rxocts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6RXPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6rxpkts,
> - last_gather_stats->ip6rxpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6RXFRAGS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6txfrags,
> - last_gather_stats->ip6txfrags,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6RXMCPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6rxmcpkts,
> - last_gather_stats->ip6rxmcpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6TXOCTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6txocts,
> - last_gather_stats->ip6txocts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6TXPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6txpkts,
> - last_gather_stats->ip6txpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6TXFRAGS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6txfrags,
> - last_gather_stats->ip6txfrags,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_IP6TXMCPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->ip6txmcpkts,
> - last_gather_stats->ip6txmcpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_TCPRXSEGS] +=
> - IRDMA_STATS_DELTA(gather_stats->tcprxsegs,
> - last_gather_stats->tcprxsegs,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_TCPTXSEG] +=
> - IRDMA_STATS_DELTA(gather_stats->tcptxsegs,
> - last_gather_stats->tcptxsegs,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMARXRDS] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmarxrds,
> - last_gather_stats->rdmarxrds,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMARXSNDS] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmarxsnds,
> - last_gather_stats->rdmarxsnds,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMARXWRS] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmarxwrs,
> - last_gather_stats->rdmarxwrs,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMATXRDS] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmatxrds,
> - last_gather_stats->rdmatxrds,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMATXSNDS] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmatxsnds,
> - last_gather_stats->rdmatxsnds,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMATXWRS] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmatxwrs,
> - last_gather_stats->rdmatxwrs,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMAVBND] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmavbn,
> - last_gather_stats->rdmavbn,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RDMAVINV] +=
> - IRDMA_STATS_DELTA(gather_stats->rdmavinv,
> - last_gather_stats->rdmavinv,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_UDPRXPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->udprxpkts,
> - last_gather_stats->udprxpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_UDPTXPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->udptxpkts,
> - last_gather_stats->udptxpkts,
> - IRDMA_MAX_STATS_48);
> - stats_val[IRDMA_HW_STAT_INDEX_RXNPECNMARKEDPKTS] +=
> - IRDMA_STATS_DELTA(gather_stats->rxnpecnmrkpkts,
> - last_gather_stats->rxnpecnmrkpkts,
> - IRDMA_MAX_STATS_48);
> +
> + irdma_update_stat(IP4RXOCTS, ip4rxocts, 48);
> + irdma_update_stat(IP4RXPKTS, ip4rxpkts, 48);
> + irdma_update_stat(IP4RXFRAGS, ip4rxfrags, 48);
> + irdma_update_stat(IP4RXMCPKTS, ip4rxmcpkts, 48);
> + irdma_update_stat(IP4TXOCTS, ip4txocts, 48);
> + irdma_update_stat(IP4TXPKTS, ip4txpkts, 48);
> + irdma_update_stat(IP4TXFRAGS, ip4txfrag, 48);
> + irdma_update_stat(IP4TXMCPKTS, ip4txmcpkts, 48);
> + irdma_update_stat(IP6RXOCTS, ip6rxocts, 48);
> + irdma_update_stat(IP6RXPKTS, ip6rxpkts, 48);
> + irdma_update_stat(IP6RXFRAGS, ip6rxfrags, 48);
> + irdma_update_stat(IP6RXMCPKTS, ip6rxmcpkts, 48);
> + irdma_update_stat(IP6TXOCTS, ip6txocts, 48);
> + irdma_update_stat(IP6TXPKTS, ip6txpkts, 48);
> + irdma_update_stat(IP6TXFRAGS, ip6txfrags, 48);
> + irdma_update_stat(IP6TXMCPKTS, ip6txmcpkts, 48);
> + irdma_update_stat(TCPRXSEGS, tcprxsegs, 48);
> + irdma_update_stat(TCPTXSEG, tcptxsegs, 48);
> + irdma_update_stat(RDMARXRDS, rdmarxrds, 48);
> + irdma_update_stat(RDMARXSNDS, rdmarxsnds, 48);
> + irdma_update_stat(RDMARXWRS, rdmarxwrs, 48);
> + irdma_update_stat(RDMATXRDS, rdmatxrds, 48);
> + irdma_update_stat(RDMATXSNDS, rdmatxsnds, 48);
> + irdma_update_stat(RDMATXWRS, rdmatxwrs, 48);
> + irdma_update_stat(RDMAVBND, rdmavbn, 48);
> + irdma_update_stat(RDMAVINV, rdmavinv, 48);
> + irdma_update_stat(UDPRXPKTS, udprxpkts, 48);
> + irdma_update_stat(UDPTXPKTS, udptxpkts, 48);
> + irdma_update_stat(RXNPECNMARKEDPKTS, rxnpecnmrkpkts, 48);
> +
> memcpy(last_gather_stats, gather_stats, sizeof(*last_gather_stats)); }
next prev parent reply other threads:[~2021-06-08 16:18 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-06 1:10 irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ? Joe Perches
2021-06-07 14:54 ` Saleem, Shiraz
2021-06-07 15:07 ` Joe Perches
2021-06-07 22:40 ` Saleem, Shiraz
2021-06-07 23:37 ` Joe Perches
2021-06-08 16:18 ` Saleem, Shiraz [this message]
2021-06-08 16:25 ` Joe Perches
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=38e0a7b9c19543c2aee679bdd346c5a9@intel.com \
--to=shiraz.saleem@intel.com \
--cc=dledford@redhat.com \
--cc=jgg@ziepe.ca \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mustafa.ismail@intel.com \
/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