linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Saleem, Shiraz" <shiraz.saleem@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: "dledford@redhat.com" <dledford@redhat.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Ertman, David M" <david.m.ertman@intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
Subject: RE: [PATCH v4 resend 04/23] ice: Register auxiliary device to provide RDMA
Date: Mon, 12 Apr 2021 14:51:23 +0000	[thread overview]
Message-ID: <6722e2809ae44e7cad9832877180ecc3@intel.com> (raw)
In-Reply-To: <20210407174521.GA542400@nvidia.com>

> Subject: Re: [PATCH v4 resend 04/23] ice: Register auxiliary device to provide
> RDMA
> 
> On Tue, Apr 06, 2021 at 07:14:43PM -0500, Shiraz Saleem wrote:
> > +/**
> > + * ice_plug_aux_devs - allocate and register one AUX dev per
> > +cdev_info in PF
> > + * @pf: pointer to PF struct
> > + */
> > +int ice_plug_aux_devs(struct ice_pf *pf) {
> > +	struct iidc_auxiliary_dev *iadev;
> > +	int ret, i;
> > +
> > +	if (!pf->cdev_infos)
> > +		return 0;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ice_cdev_ids); i++) {
> > +		struct iidc_core_dev_info *cdev_info;
> > +		struct auxiliary_device *adev;
> > +
> > +		cdev_info = pf->cdev_infos[i];
> > +		if (!cdev_info)
> > +			continue;
> > +
> > +		iadev = kzalloc(sizeof(*iadev), GFP_KERNEL);
> > +		if (!iadev)
> > +			return -ENOMEM;
> > +
> > +		adev = &iadev->adev;
> > +		cdev_info->adev = adev;
> > +		iadev->cdev_info = cdev_info;
> > +
> > +		if (ice_cdev_ids[i].id == IIDC_RDMA_ID) {
> > +			if (cdev_info->rdma_protocol ==
> > +			    IIDC_RDMA_PROTOCOL_IWARP)
> > +				adev->name = kasprintf(GFP_KERNEL, "%s_%s",
> > +						       ice_cdev_ids[i].name,
> > +						       "iwarp");
> > +			else
> > +				adev->name = kasprintf(GFP_KERNEL, "%s_%s",
> > +						       ice_cdev_ids[i].name,
> > +						       "roce");
> > +		} else {
> > +			adev->name = kasprintf(GFP_KERNEL, "%s",
> > +					       ice_cdev_ids[i].name);
> 
> This never happens, it is dead code, right?
> 
> > +		}
> 
> This is all confused, the adev->name is supposed to be a compile time constant,
> this not be allocating memory and combining the constant "intel_rdma" and the
> trailing "iwarp" "roce"
> 
> Just use the proper define right here.
> 
> Also all these kasprintfs should have their errors checked, which is also why it
> shouldn't be written like this.
> 

Agree to all the feedback here.

  reply	other threads:[~2021-04-12 14:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07  0:14 [PATCH v4 resend 00/23] Add Intel Ethernet Protocol Driver for RDMA (irdma) Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 01/23] iidc: Introduce iidc.h Shiraz Saleem
2021-04-07 18:05   ` Jason Gunthorpe
2021-04-12 14:51     ` Saleem, Shiraz
2021-04-12 16:12       ` Jason Gunthorpe
2021-04-07  0:14 ` [PATCH v4 resend 02/23] ice: Initialize RDMA support Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 03/23] ice: Implement iidc operations Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 04/23] ice: Register auxiliary device to provide RDMA Shiraz Saleem
2021-04-07 17:45   ` Jason Gunthorpe
2021-04-12 14:51     ` Saleem, Shiraz [this message]
2021-04-07  0:14 ` [PATCH v4 resend 05/23] ice: Add devlink params support Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 06/23] i40e: Prep i40e header for aux bus conversion Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 07/23] i40e: Register auxiliary devices to provide RDMA Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 08/23] RDMA/irdma: Register auxiliary driver and implement private channel OPs Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 09/23] RDMA/irdma: Implement device initialization definitions Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 10/23] RDMA/irdma: Implement HW Admin Queue OPs Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 11/23] RDMA/irdma: Add HMC backing store setup functions Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 12/23] RDMA/irdma: Add privileged UDA queue implementation Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 13/23] RDMA/irdma: Add QoS definitions Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 14/23] RDMA/irdma: Add connection manager Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 15/23] RDMA/irdma: Add PBLE resource manager Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 16/23] RDMA/irdma: Implement device supported verb APIs Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 17/23] RDMA/irdma: Add RoCEv2 UD OP support Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 18/23] RDMA/irdma: Add user/kernel shared libraries Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 19/23] RDMA/irdma: Add miscellaneous utility definitions Shiraz Saleem
2021-04-07  0:14 ` [PATCH v4 resend 20/23] RDMA/irdma: Add dynamic tracing for CM Shiraz Saleem
2021-04-07  0:15 ` [PATCH v4 resend 21/23] RDMA/irdma: Add ABI definitions Shiraz Saleem
2021-04-07 15:23   ` Jason Gunthorpe
2021-04-07 20:57     ` Saleem, Shiraz
2021-04-07  0:15 ` [PATCH v4 resend 22/23] RDMA/irdma: Add irdma Kconfig/Makefile and remove i40iw Shiraz Saleem
2021-04-07  0:15 ` [PATCH v4 resend 23/23] RDMA/irdma: Update MAINTAINERS file Shiraz Saleem

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=6722e2809ae44e7cad9832877180ecc3@intel.com \
    --to=shiraz.saleem@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=david.m.ertman@intel.com \
    --cc=dledford@redhat.com \
    --cc=jgg@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).