Hi Jason Wang Thanks for your review.
Yes, you're correct. I will update the patch accordingly to address the issues you pointed out. Here is the link to the V2 version: https://lore.kernel.org/all/[email protected]/ Here is the content of the V2 version: ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- In the vp_vdpa_set_status function, when setting the device status to VIRTIO_CONFIG_S_DRIVER_OK, the vp_vdpa_request_irq function may fail. In such cases, the device status should not be set to DRIVER_OK. Add exception printing to remind the user. Signed-off-by: Yuxue Liu <[email protected]> --- V1 -> V2: Remove redundant printouts V1: https://lore.kernel.org/all/[email protected]/ --- drivers/vdpa/virtio_pci/vp_vdpa.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c index df5f4a3bccb5..4caca0517cad 100644 --- a/drivers/vdpa/virtio_pci/vp_vdpa.c +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c @@ -216,7 +216,10 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status) if (status & VIRTIO_CONFIG_S_DRIVER_OK && !(s & VIRTIO_CONFIG_S_DRIVER_OK)) { - vp_vdpa_request_irq(vp_vdpa); + if (vp_vdpa_request_irq(vp_vdpa)) { + WARN_ON(1); + return; + } } vp_modern_set_status(mdev, status); -- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Best regards, Gavin -----Original Message----- From: Jason Wang [email protected] Sent: March 18, 2024 12:25 To: Gavin Liu [email protected] Cc: [email protected]; [email protected]; [email protected]; Angus Chen [email protected]; Gavin Liu [email protected] Subject: Re: [PATCH] vp_vdpa: Fix return value check vp_vdpa_request_irq External Mail: This email originated from OUTSIDE of the organization! Do not click links, open attachments or provide ANY information unless you recognize the sender and know the content is safe. On Fri, Mar 15, 2024 at 6:29 PM gavin.liu <[email protected]> wrote: > > From: Yuxue Liu <[email protected]> > > In the vp_vdpa_set_status function, when setting the device status to > VIRTIO_CONFIG_S_DRIVER_OK, the vp_vdpa_request_irq function may fail. > In such cases, the device status should not be set to DRIVER_OK. Add > exception printing to remind the user. > > Signed-off-by: Yuxue Liu <[email protected]> > --- > drivers/vdpa/virtio_pci/vp_vdpa.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c > b/drivers/vdpa/virtio_pci/vp_vdpa.c > index 281287fae89f..c1270b263867 100644 > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c > @@ -213,10 +213,16 @@ static void vp_vdpa_set_status(struct vdpa_device > *vdpa, u8 status) > struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa); > struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa); > u8 s = vp_vdpa_get_status(vdpa); > + struct pci_dev *pdev = mdev->pci_dev; > > if (status & VIRTIO_CONFIG_S_DRIVER_OK && > !(s & VIRTIO_CONFIG_S_DRIVER_OK)) { > - vp_vdpa_request_irq(vp_vdpa); > + if (vp_vdpa_request_irq(vp_vdpa)) { > + WARN_ON(1); > + dev_err(&pdev->dev, > + "vp_vdpa : fail set status is DRIVER_OK\n"); I think having one of the WARN_ON or dev_err should be sufficient. Thanks > + return; > + } > } > > vp_modern_set_status(mdev, status); > -- > 2.43.0 >
