On Mon, Mar 14, 2022 at 10:22:03AM +0800, Zhu, Lingshan
wrote:> Hello Dan,
>
> Thanks for your suggestions and this auto-testing efforts!
> On handling the vector for device config interrupt, there are three
> possibilities:
> (1)it has a dedicated vector(2)it shares a vector with datapath(3)no
> vectors.
>
> So in these code below, it handles the three cases, or it should be
-EINVAL,
> so IMHO we don't need
> an else there, just leave it -EINVAL.
I'm confused about why you're talking about -EINVAL... There is no
-EINVAL in this function.
This code is not necessarily buggy. Right now we have GCC uninitialized
variable warnings turned off so it also doesn't cause a build issue.
But I think we should try to work towards a future where we can
re-enable the GCC warning. GCC catches a lot of stupid uninitialized
variable bugs and it's better if we can catch them earlier instead of
relying on the kbuild-bot.
regards,
dan carpenter
>
> Thanks for your efforts!
> Zhu Lingshan
>
> On 3/11/2022 5:00 PM, Dan Carpenter wrote:
> > Hello Zhu Lingshan,
> >
> > The patch 79333575b8bd: "vDPA/ifcvf: implement shared IRQ
feature"
> > from Feb 22, 2022, leads to the following Smatch static checker
> > warning:
> >
> > drivers/vdpa/ifcvf/ifcvf_main.c:306 ifcvf_request_config_irq()
> > error: uninitialized symbol 'config_vector'.> >
> > drivers/vdpa/ifcvf/ifcvf_main.c
> > 287 static int ifcvf_request_config_irq(struct ifcvf_adapter
*adapter)
> > 288 {
> > 289 struct pci_dev *pdev = adapter->pdev;
> > 290 struct ifcvf_hw *vf = &adapter->vf;
> > 291 int config_vector, ret;
> > 292
> > 293 if (vf->msix_vector_status ==
MSIX_VECTOR_DEV_SHARED)
> > 294 return 0;
> > 295
> > 296 if (vf->msix_vector_status ==
MSIX_VECTOR_PER_VQ_AND_CONFIG)
> > 297 /* vector 0 ~ vf->nr_vring for vqs, num
vf->nr_vring vector for config interrupt */
> > 298 config_vector = vf->nr_vring;
> >
> > Set here.
> >
> > 299
> > 300 if (vf->msix_vector_status ==
MSIX_VECTOR_SHARED_VQ_AND_CONFIG)
> > 301 /* vector 0 for vqs and 1 for config
interrupt */
> > 302 config_vector = 1;
> >
> > And here. But no else path.
> >
> > 303
> > 304 snprintf(vf->config_msix_name, 256,
"ifcvf[%s]-config\n",
> > 305 pci_name(pdev));
> > --> 306 vf->config_irq = pci_irq_vector(pdev,
config_vector);
> > 307 ret = devm_request_irq(&pdev->dev,
vf->config_irq,
> > 308 ifcvf_config_changed, 0,
> > 309 vf->config_msix_name, vf);
> > 310 if (ret) {
> > 311 IFCVF_ERR(pdev, "Failed to request
config irq\n");
> > 312 goto err;
> > 313 }
> > 314
> > 315 ret = ifcvf_set_config_vector(vf, config_vector);
> > 316 if (ret == VIRTIO_MSI_NO_VECTOR) {
> > 317 IFCVF_ERR(pdev, "No msix vector for
device config\n");
> > 318 goto err;
> > 319 }
> > 320
> > 321 return 0;
> > 322 err:
> > 323 ifcvf_free_irq(adapter);
> > 324
> > 325 return -EFAULT;
> > 326 }
> >
> > regards,
> > dan carpenter