All of lore.kernel.org
 help / color / mirror / Atom feed
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));  }


  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.