* Re: [PATCH 1/2] eni_vdpa: Fix an error handling path in 'eni_vdpa_probe()' [not found] <f6b2d087ca3840604b4e711a208d35b5d6285cb4.1636301587.git.christophe.jaillet@wanadoo.fr> @ 2021-11-08 5:55 ` Jason Wang [not found] ` <393fb7b7-653b-eae6-16bd-5ffc7d600619@wanadoo.fr> [not found] ` <02045bdcbbb25f79bae4827f66029cfcddc90381.1636301587.git.christophe.jaillet@wanadoo.fr> 1 sibling, 1 reply; 5+ messages in thread From: Jason Wang @ 2021-11-08 5:55 UTC (permalink / raw) To: Christophe JAILLET Cc: Arnd Bergmann, mst, kernel-janitors, linux-kernel, virtualization, Wu Zongyong On Mon, Nov 8, 2021 at 12:15 AM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > In the error handling path, a successful 'vp_legacy_probe()' should be > balanced by a corresponding 'vp_legacy_remove()' call, as already done in > the remove function. > > Add the missing call and update gotos accordingly. > > Fixes: e85087beedca ("eni_vdpa: add vDPA driver for Alibaba ENI") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/vdpa/alibaba/eni_vdpa.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c > index 3f788794571a..12b3db6b4517 100644 > --- a/drivers/vdpa/alibaba/eni_vdpa.c > +++ b/drivers/vdpa/alibaba/eni_vdpa.c > @@ -501,7 +501,7 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > if (!eni_vdpa->vring) { > ret = -ENOMEM; > ENI_ERR(pdev, "failed to allocate virtqueues\n"); > - goto err; > + goto err_remove_vp_legacy; > } > > for (i = 0; i < eni_vdpa->queues; i++) { > @@ -513,11 +513,13 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > ret = vdpa_register_device(&eni_vdpa->vdpa, eni_vdpa->queues); > if (ret) { > ENI_ERR(pdev, "failed to register to vdpa bus\n"); > - goto err; > + goto err_remove_vp_legacy; > } > > return 0; > > +err_remove_vp_legacy: > + vp_legacy_remove(&eni_vdpa->ldev); Won't vp_legacy_remove() be triggered by the put_devic() below? Thanks > err: > put_device(&eni_vdpa->vdpa.dev); > return ret; > -- > 2.30.2 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <393fb7b7-653b-eae6-16bd-5ffc7d600619@wanadoo.fr>]
* Re: [PATCH 1/2] eni_vdpa: Fix an error handling path in 'eni_vdpa_probe()' [not found] ` <393fb7b7-653b-eae6-16bd-5ffc7d600619@wanadoo.fr> @ 2021-11-09 2:54 ` Jason Wang [not found] ` <069ed3fb-ef99-ff36-136d-a0221ba85a96@wanadoo.fr> 0 siblings, 1 reply; 5+ messages in thread From: Jason Wang @ 2021-11-09 2:54 UTC (permalink / raw) To: Christophe JAILLET Cc: Arnd Bergmann, mst, kernel-janitors, linux-kernel, virtualization, Wu Zongyong On Tue, Nov 9, 2021 at 3:32 AM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > Le 08/11/2021 à 06:55, Jason Wang a écrit : > > On Mon, Nov 8, 2021 at 12:15 AM Christophe JAILLET > > <christophe.jaillet@wanadoo.fr> wrote: > >> > >> In the error handling path, a successful 'vp_legacy_probe()' should be > >> balanced by a corresponding 'vp_legacy_remove()' call, as already done in > >> the remove function. > >> > >> Add the missing call and update gotos accordingly. > >> > >> Fixes: e85087beedca ("eni_vdpa: add vDPA driver for Alibaba ENI") > >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > >> --- > >> drivers/vdpa/alibaba/eni_vdpa.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c > >> index 3f788794571a..12b3db6b4517 100644 > >> --- a/drivers/vdpa/alibaba/eni_vdpa.c > >> +++ b/drivers/vdpa/alibaba/eni_vdpa.c > >> @@ -501,7 +501,7 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > >> if (!eni_vdpa->vring) { > >> ret = -ENOMEM; > >> ENI_ERR(pdev, "failed to allocate virtqueues\n"); > >> - goto err; > >> + goto err_remove_vp_legacy; > >> } > >> > >> for (i = 0; i < eni_vdpa->queues; i++) { > >> @@ -513,11 +513,13 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > >> ret = vdpa_register_device(&eni_vdpa->vdpa, eni_vdpa->queues); > >> if (ret) { > >> ENI_ERR(pdev, "failed to register to vdpa bus\n"); > >> - goto err; > >> + goto err_remove_vp_legacy; > >> } > >> > >> return 0; > >> > >> +err_remove_vp_legacy: > >> + vp_legacy_remove(&eni_vdpa->ldev); > > > > Won't vp_legacy_remove() be triggered by the put_devic() below? > > Hi, I'm sorry but i don't see how. > > My understanding is that: > - on "put_device(&eni_vdpa->vdpa.dev);", the corresponding release > function will be called. > > - This release function is the one recorded in the > '__vdpa_alloc_device()' function. > > - So it should be 'vdpa_release_dev()'. > > - This function, AFAIU, has no knowledge of 'vp_legacy_remove()' or > anything that could call it. > > > Unless I misread something or miss something obvious, I don't see how > 'vp_legacy_remove() would be called. > > > Could you elaborate? I think the device should release the driver (see device_release_driver()) during during its deleting. Thanks > > CJ > > > > > Thanks > > > >> err: > >> put_device(&eni_vdpa->vdpa.dev); > >> return ret; > >> -- > >> 2.30.2 > >> > > > > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <069ed3fb-ef99-ff36-136d-a0221ba85a96@wanadoo.fr>]
* Re: [PATCH 1/2] eni_vdpa: Fix an error handling path in 'eni_vdpa_probe()' [not found] ` <069ed3fb-ef99-ff36-136d-a0221ba85a96@wanadoo.fr> @ 2021-11-24 23:58 ` Michael S. Tsirkin [not found] ` <131b9250-f338-c931-39e6-ca3c1e3a9b66@wanadoo.fr> 0 siblings, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2021-11-24 23:58 UTC (permalink / raw) To: Christophe JAILLET Cc: Arnd Bergmann, kernel-janitors, linux-kernel, virtualization, Wu Zongyong On Tue, Nov 09, 2021 at 10:21:44PM +0100, Christophe JAILLET wrote: > Le 09/11/2021 à 03:54, Jason Wang a écrit : > > On Tue, Nov 9, 2021 at 3:32 AM Christophe JAILLET > > <christophe.jaillet@wanadoo.fr> wrote: > > > > > > Le 08/11/2021 à 06:55, Jason Wang a écrit : > > > > On Mon, Nov 8, 2021 at 12:15 AM Christophe JAILLET > > > > <christophe.jaillet@wanadoo.fr> wrote: > > > > > > > > > > In the error handling path, a successful 'vp_legacy_probe()' should be > > > > > balanced by a corresponding 'vp_legacy_remove()' call, as already done in > > > > > the remove function. > > > > > > > > > > Add the missing call and update gotos accordingly. > > > > > > > > > > Fixes: e85087beedca ("eni_vdpa: add vDPA driver for Alibaba ENI") > > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > > > --- > > > > > drivers/vdpa/alibaba/eni_vdpa.c | 6 ++++-- > > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c > > > > > index 3f788794571a..12b3db6b4517 100644 > > > > > --- a/drivers/vdpa/alibaba/eni_vdpa.c > > > > > +++ b/drivers/vdpa/alibaba/eni_vdpa.c > > > > > @@ -501,7 +501,7 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > > > if (!eni_vdpa->vring) { > > > > > ret = -ENOMEM; > > > > > ENI_ERR(pdev, "failed to allocate virtqueues\n"); > > > > > - goto err; > > > > > + goto err_remove_vp_legacy; > > > > > } > > > > > > > > > > for (i = 0; i < eni_vdpa->queues; i++) { > > > > > @@ -513,11 +513,13 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > > > ret = vdpa_register_device(&eni_vdpa->vdpa, eni_vdpa->queues); > > > > > if (ret) { > > > > > ENI_ERR(pdev, "failed to register to vdpa bus\n"); > > > > > - goto err; > > > > > + goto err_remove_vp_legacy; > > > > > } > > > > > > > > > > return 0; > > > > > > > > > > +err_remove_vp_legacy: > > > > > + vp_legacy_remove(&eni_vdpa->ldev); > > > > > > > > Won't vp_legacy_remove() be triggered by the put_devic() below? > > > > > > Hi, I'm sorry but i don't see how. > > > > > > My understanding is that: > > > - on "put_device(&eni_vdpa->vdpa.dev);", the corresponding release > > > function will be called. > > > > > > - This release function is the one recorded in the > > > '__vdpa_alloc_device()' function. > > > > > > - So it should be 'vdpa_release_dev()'. > > > > > > - This function, AFAIU, has no knowledge of 'vp_legacy_remove()' or > > > anything that could call it. > > > > > > > > > Unless I misread something or miss something obvious, I don't see how > > > 'vp_legacy_remove() would be called. > > > > > > > > > Could you elaborate? > > > > I think the device should release the driver (see > > device_release_driver()) during during its deleting. > > Hi, I still don't follow the logic and I don't understand how > 'vp_legacy_remove()' will finely be called, but it is not that important. > > If it's fine for you, it's fine for me :) > > CJ So pls post just patch 2? > > > > Thanks > > > > > > > > CJ > > > > > > > > > > > Thanks > > > > > > > > > err: > > > > > put_device(&eni_vdpa->vdpa.dev); > > > > > return ret; > > > > > -- > > > > > 2.30.2 > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > Virtualization mailing list > > Virtualization@lists.linux-foundation.org > > https://lists.linuxfoundation.org/mailman/listinfo/virtualization > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <131b9250-f338-c931-39e6-ca3c1e3a9b66@wanadoo.fr>]
* Re: [PATCH 1/2] eni_vdpa: Fix an error handling path in 'eni_vdpa_probe()' [not found] ` <131b9250-f338-c931-39e6-ca3c1e3a9b66@wanadoo.fr> @ 2021-11-25 7:02 ` Michael S. Tsirkin 0 siblings, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2021-11-25 7:02 UTC (permalink / raw) To: Christophe JAILLET Cc: Arnd Bergmann, kernel-janitors, linux-kernel, virtualization, Wu Zongyong On Thu, Nov 25, 2021 at 07:34:02AM +0100, Christophe JAILLET wrote: > Le 25/11/2021 à 00:58, Michael S. Tsirkin a écrit : > > On Tue, Nov 09, 2021 at 10:21:44PM +0100, Christophe JAILLET wrote: > > > Le 09/11/2021 à 03:54, Jason Wang a écrit : > > > > On Tue, Nov 9, 2021 at 3:32 AM Christophe JAILLET > > > > <christophe.jaillet@wanadoo.fr> wrote: > > > > > > > > > > Le 08/11/2021 à 06:55, Jason Wang a écrit : > > > > > > On Mon, Nov 8, 2021 at 12:15 AM Christophe JAILLET > > > > > > <christophe.jaillet@wanadoo.fr> wrote: > > > > > > > > > > > > > > In the error handling path, a successful 'vp_legacy_probe()' should be > > > > > > > balanced by a corresponding 'vp_legacy_remove()' call, as already done in > > > > > > > the remove function. > > > > > > > > > > > > > > Add the missing call and update gotos accordingly. > > > > > > > > > > > > > > Fixes: e85087beedca ("eni_vdpa: add vDPA driver for Alibaba ENI") > > > > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > > > > > --- > > > > > > > drivers/vdpa/alibaba/eni_vdpa.c | 6 ++++-- > > > > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c > > > > > > > index 3f788794571a..12b3db6b4517 100644 > > > > > > > --- a/drivers/vdpa/alibaba/eni_vdpa.c > > > > > > > +++ b/drivers/vdpa/alibaba/eni_vdpa.c > > > > > > > @@ -501,7 +501,7 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > > > > > if (!eni_vdpa->vring) { > > > > > > > ret = -ENOMEM; > > > > > > > ENI_ERR(pdev, "failed to allocate virtqueues\n"); > > > > > > > - goto err; > > > > > > > + goto err_remove_vp_legacy; > > > > > > > } > > > > > > > > > > > > > > for (i = 0; i < eni_vdpa->queues; i++) { > > > > > > > @@ -513,11 +513,13 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > > > > > ret = vdpa_register_device(&eni_vdpa->vdpa, eni_vdpa->queues); > > > > > > > if (ret) { > > > > > > > ENI_ERR(pdev, "failed to register to vdpa bus\n"); > > > > > > > - goto err; > > > > > > > + goto err_remove_vp_legacy; > > > > > > > } > > > > > > > > > > > > > > return 0; > > > > > > > > > > > > > > +err_remove_vp_legacy: > > > > > > > + vp_legacy_remove(&eni_vdpa->ldev); > > > > > > > > > > > > Won't vp_legacy_remove() be triggered by the put_devic() below? > > > > > > > > > > Hi, I'm sorry but i don't see how. > > > > > > > > > > My understanding is that: > > > > > - on "put_device(&eni_vdpa->vdpa.dev);", the corresponding release > > > > > function will be called. > > > > > > > > > > - This release function is the one recorded in the > > > > > '__vdpa_alloc_device()' function. > > > > > > > > > > - So it should be 'vdpa_release_dev()'. > > > > > > > > > > - This function, AFAIU, has no knowledge of 'vp_legacy_remove()' or > > > > > anything that could call it. > > > > > > > > > > > > > > > Unless I misread something or miss something obvious, I don't see how > > > > > 'vp_legacy_remove() would be called. > > > > > > > > > > > > > > > Could you elaborate? > > > > > > > > I think the device should release the driver (see > > > > device_release_driver()) during during its deleting. > > > > > > Hi, I still don't follow the logic and I don't understand how > > > 'vp_legacy_remove()' will finely be called, but it is not that important. > > > > > > If it's fine for you, it's fine for me :) > > > > > > CJ > > > > So pls post just patch 2? > > Patch 2/2 should apply cleanly with or without patch 1/2. > Do I really need to send a v2 just for dropping the first patch? :/ > > CJ It is preferable, yes. A patch in isolation is not the same as a patch in the series. > > > > > > > > > > Thanks > > > > > > > > > > > > > > CJ > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > err: > > > > > > > put_device(&eni_vdpa->vdpa.dev); > > > > > > > return ret; > > > > > > > -- > > > > > > > 2.30.2 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > Virtualization mailing list > > > > Virtualization@lists.linux-foundation.org > > > > https://lists.linuxfoundation.org/mailman/listinfo/virtualization > > > > > > > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <02045bdcbbb25f79bae4827f66029cfcddc90381.1636301587.git.christophe.jaillet@wanadoo.fr>]
* Re: [PATCH 2/2] eni_vdpa: Simplify 'eni_vdpa_probe()' [not found] ` <02045bdcbbb25f79bae4827f66029cfcddc90381.1636301587.git.christophe.jaillet@wanadoo.fr> @ 2021-11-09 2:56 ` Jason Wang 0 siblings, 0 replies; 5+ messages in thread From: Jason Wang @ 2021-11-09 2:56 UTC (permalink / raw) To: Christophe JAILLET Cc: Arnd Bergmann, mst, kernel-janitors, linux-kernel, virtualization, Wu Zongyong On Mon, Nov 8, 2021 at 12:15 AM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > When 'pcim_enable_device()' is used, some resources become automagically > managed. > There is no need to call 'pci_free_irq_vectors()' when the driver is > removed. The same will already be done by 'pcim_release()'. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Acked-by: Jason Wang <jasowang@redhat.com> > --- > Leaving the 'pci_free_irq_vectors()' call is harmless. > --- > drivers/vdpa/alibaba/eni_vdpa.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c > index 12b3db6b4517..3db686436628 100644 > --- a/drivers/vdpa/alibaba/eni_vdpa.c > +++ b/drivers/vdpa/alibaba/eni_vdpa.c > @@ -450,11 +450,6 @@ static u16 eni_vdpa_get_num_queues(struct eni_vdpa *eni_vdpa) > return num; > } > > -static void eni_vdpa_free_irq_vectors(void *data) > -{ > - pci_free_irq_vectors(data); > -} > - > static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { > struct device *dev = &pdev->dev; > @@ -488,13 +483,6 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id) > eni_vdpa->vdpa.dma_dev = &pdev->dev; > eni_vdpa->queues = eni_vdpa_get_num_queues(eni_vdpa); > > - ret = devm_add_action_or_reset(dev, eni_vdpa_free_irq_vectors, pdev); > - if (ret) { > - ENI_ERR(pdev, > - "failed for adding devres for freeing irq vectors\n"); > - goto err; > - } > - > eni_vdpa->vring = devm_kcalloc(&pdev->dev, eni_vdpa->queues, > sizeof(*eni_vdpa->vring), > GFP_KERNEL); > -- > 2.30.2 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-25 7:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <f6b2d087ca3840604b4e711a208d35b5d6285cb4.1636301587.git.christophe.jaillet@wanadoo.fr>
2021-11-08 5:55 ` [PATCH 1/2] eni_vdpa: Fix an error handling path in 'eni_vdpa_probe()' Jason Wang
[not found] ` <393fb7b7-653b-eae6-16bd-5ffc7d600619@wanadoo.fr>
2021-11-09 2:54 ` Jason Wang
[not found] ` <069ed3fb-ef99-ff36-136d-a0221ba85a96@wanadoo.fr>
2021-11-24 23:58 ` Michael S. Tsirkin
[not found] ` <131b9250-f338-c931-39e6-ca3c1e3a9b66@wanadoo.fr>
2021-11-25 7:02 ` Michael S. Tsirkin
[not found] ` <02045bdcbbb25f79bae4827f66029cfcddc90381.1636301587.git.christophe.jaillet@wanadoo.fr>
2021-11-09 2:56 ` [PATCH 2/2] eni_vdpa: Simplify 'eni_vdpa_probe()' Jason Wang
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).