* [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() @ 2019-01-13 14:36 Fei Li 2019-01-13 16:54 ` Marcel Apfelbaum 2019-01-14 3:31 ` Peter Xu 0 siblings, 2 replies; 7+ messages in thread From: Fei Li @ 2019-01-13 14:36 UTC (permalink / raw) To: qemu-devel, shirley17fei Cc: Markus Armbruster, Peter Xu, Michael S . Tsirkin, Marcel Apfelbaum From: Fei Li <shirley17fei@gmail.com> Let's supplement the msi_uninit() when failing to realize the pci edu device. Cc: Markus Armbruster <armbru@redhat.com> Cc: Peter Xu <peterx@redhat.com> Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Signed-off-by: Fei Li <shirley17fei@gmail.com> --- hw/misc/edu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/misc/edu.c b/hw/misc/edu.c index cdcf550dd7..4feb7503de 100644 --- a/hw/misc/edu.c +++ b/hw/misc/edu.c @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) { EduState *edu = EDU(pdev); + msi_uninit(pdev); qemu_mutex_lock(&edu->thr_mutex); edu->stopping = true; qemu_mutex_unlock(&edu->thr_mutex); -- 2.17.2 (Apple Git-113) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() 2019-01-13 14:36 [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() Fei Li @ 2019-01-13 16:54 ` Marcel Apfelbaum 2019-01-14 3:31 ` Peter Xu 1 sibling, 0 replies; 7+ messages in thread From: Marcel Apfelbaum @ 2019-01-13 16:54 UTC (permalink / raw) To: Fei Li, qemu-devel, shirley17fei Cc: Markus Armbruster, Peter Xu, Michael S . Tsirkin On 1/13/19 4:36 PM, Fei Li wrote: > From: Fei Li <shirley17fei@gmail.com> > > Let's supplement the msi_uninit() when failing to realize > the pci edu device. > > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Peter Xu <peterx@redhat.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Signed-off-by: Fei Li <shirley17fei@gmail.com> > --- > hw/misc/edu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/misc/edu.c b/hw/misc/edu.c > index cdcf550dd7..4feb7503de 100644 > --- a/hw/misc/edu.c > +++ b/hw/misc/edu.c > @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) > { > EduState *edu = EDU(pdev); > > + msi_uninit(pdev); > qemu_mutex_lock(&edu->thr_mutex); > edu->stopping = true; > qemu_mutex_unlock(&edu->thr_mutex); Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com> Thanks, Marcel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() 2019-01-13 14:36 [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() Fei Li 2019-01-13 16:54 ` Marcel Apfelbaum @ 2019-01-14 3:31 ` Peter Xu 2019-01-14 7:02 ` Markus Armbruster 1 sibling, 1 reply; 7+ messages in thread From: Peter Xu @ 2019-01-14 3:31 UTC (permalink / raw) To: Fei Li Cc: qemu-devel, shirley17fei, Markus Armbruster, Michael S . Tsirkin, Marcel Apfelbaum On Sun, Jan 13, 2019 at 10:36:41PM +0800, Fei Li wrote: > From: Fei Li <shirley17fei@gmail.com> > > Let's supplement the msi_uninit() when failing to realize > the pci edu device. > > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Peter Xu <peterx@redhat.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Signed-off-by: Fei Li <shirley17fei@gmail.com> > --- > hw/misc/edu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/misc/edu.c b/hw/misc/edu.c > index cdcf550dd7..4feb7503de 100644 > --- a/hw/misc/edu.c > +++ b/hw/misc/edu.c > @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) > { > EduState *edu = EDU(pdev); > > + msi_uninit(pdev); It would be cleaner to me to call this after the join() since edu_fact_thread() could potentially use msi_*() helpers then the destructions follow the reverse order of init. Reviewed-by: Peter Xu <peterx@redhat.com> > qemu_mutex_lock(&edu->thr_mutex); > edu->stopping = true; > qemu_mutex_unlock(&edu->thr_mutex); > -- > 2.17.2 (Apple Git-113) > Regards, -- Peter Xu ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() 2019-01-14 3:31 ` Peter Xu @ 2019-01-14 7:02 ` Markus Armbruster 2019-01-14 7:18 ` Peter Xu 0 siblings, 1 reply; 7+ messages in thread From: Markus Armbruster @ 2019-01-14 7:02 UTC (permalink / raw) To: Peter Xu; +Cc: Fei Li, Michael S . Tsirkin, qemu-devel, shirley17fei Peter Xu <peterx@redhat.com> writes: > On Sun, Jan 13, 2019 at 10:36:41PM +0800, Fei Li wrote: >> From: Fei Li <shirley17fei@gmail.com> >> >> Let's supplement the msi_uninit() when failing to realize >> the pci edu device. >> >> Cc: Markus Armbruster <armbru@redhat.com> >> Cc: Peter Xu <peterx@redhat.com> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >> Signed-off-by: Fei Li <shirley17fei@gmail.com> >> --- >> hw/misc/edu.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/misc/edu.c b/hw/misc/edu.c >> index cdcf550dd7..4feb7503de 100644 >> --- a/hw/misc/edu.c >> +++ b/hw/misc/edu.c >> @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) >> { >> EduState *edu = EDU(pdev); >> >> + msi_uninit(pdev); > > It would be cleaner to me to call this after the join() since > edu_fact_thread() could potentially use msi_*() helpers then the > destructions follow the reverse order of init. Destruction in reverse creation order is good practice. This being the "QEMU educational PCI device", good practice is even more desirable. > Reviewed-by: Peter Xu <peterx@redhat.com> > >> qemu_mutex_lock(&edu->thr_mutex); >> edu->stopping = true; >> qemu_mutex_unlock(&edu->thr_mutex); >> -- >> 2.17.2 (Apple Git-113) >> > > Regards, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() 2019-01-14 7:02 ` Markus Armbruster @ 2019-01-14 7:18 ` Peter Xu 2019-01-14 10:40 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 7+ messages in thread From: Peter Xu @ 2019-01-14 7:18 UTC (permalink / raw) To: Markus Armbruster; +Cc: Fei Li, Michael S . Tsirkin, qemu-devel, shirley17fei On Mon, Jan 14, 2019 at 08:02:23AM +0100, Markus Armbruster wrote: > Peter Xu <peterx@redhat.com> writes: > > > On Sun, Jan 13, 2019 at 10:36:41PM +0800, Fei Li wrote: > >> From: Fei Li <shirley17fei@gmail.com> > >> > >> Let's supplement the msi_uninit() when failing to realize > >> the pci edu device. > >> > >> Cc: Markus Armbruster <armbru@redhat.com> > >> Cc: Peter Xu <peterx@redhat.com> > >> Cc: Michael S. Tsirkin <mst@redhat.com> > >> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > >> Signed-off-by: Fei Li <shirley17fei@gmail.com> > >> --- > >> hw/misc/edu.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/hw/misc/edu.c b/hw/misc/edu.c > >> index cdcf550dd7..4feb7503de 100644 > >> --- a/hw/misc/edu.c > >> +++ b/hw/misc/edu.c > >> @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) > >> { > >> EduState *edu = EDU(pdev); > >> > >> + msi_uninit(pdev); > > > > It would be cleaner to me to call this after the join() since > > edu_fact_thread() could potentially use msi_*() helpers then the > > destructions follow the reverse order of init. > > Destruction in reverse creation order is good practice. > > This being the "QEMU educational PCI device", good practice is even more > desirable. Very persuasive. With that, I'd like to withdraw my excuse of "msi_uninit() is optional" too. :) Fei, please feel free to pick my r-b if you want to repost, and IMHO you can also add: Reported-by: Markus Armbruster <armbru@redhat.com> Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() 2019-01-14 7:18 ` Peter Xu @ 2019-01-14 10:40 ` Philippe Mathieu-Daudé 2019-01-14 12:45 ` Fei Li 0 siblings, 1 reply; 7+ messages in thread From: Philippe Mathieu-Daudé @ 2019-01-14 10:40 UTC (permalink / raw) To: Peter Xu, Markus Armbruster Cc: Fei Li, shirley17fei, qemu-devel, Michael S . Tsirkin On 1/14/19 8:18 AM, Peter Xu wrote: > On Mon, Jan 14, 2019 at 08:02:23AM +0100, Markus Armbruster wrote: >> Peter Xu <peterx@redhat.com> writes: >> >>> On Sun, Jan 13, 2019 at 10:36:41PM +0800, Fei Li wrote: >>>> From: Fei Li <shirley17fei@gmail.com> >>>> >>>> Let's supplement the msi_uninit() when failing to realize >>>> the pci edu device. >>>> >>>> Cc: Markus Armbruster <armbru@redhat.com> >>>> Cc: Peter Xu <peterx@redhat.com> >>>> Cc: Michael S. Tsirkin <mst@redhat.com> >>>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >>>> Signed-off-by: Fei Li <shirley17fei@gmail.com> >>>> --- >>>> hw/misc/edu.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/hw/misc/edu.c b/hw/misc/edu.c >>>> index cdcf550dd7..4feb7503de 100644 >>>> --- a/hw/misc/edu.c >>>> +++ b/hw/misc/edu.c >>>> @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) >>>> { >>>> EduState *edu = EDU(pdev); >>>> >>>> + msi_uninit(pdev); >>> >>> It would be cleaner to me to call this after the join() since >>> edu_fact_thread() could potentially use msi_*() helpers then the >>> destructions follow the reverse order of init. >> >> Destruction in reverse creation order is good practice. >> >> This being the "QEMU educational PCI device", good practice is even more >> desirable. > > Very persuasive. With that, I'd like to withdraw my excuse of > "msi_uninit() is optional" too. :) > > Fei, please feel free to pick my r-b if you want to repost, and IMHO > you can also add: > > Reported-by: Markus Armbruster <armbru@redhat.com> > > Thanks, > Moving msi_uninit() after timer_del(): Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() 2019-01-14 10:40 ` Philippe Mathieu-Daudé @ 2019-01-14 12:45 ` Fei Li 0 siblings, 0 replies; 7+ messages in thread From: Fei Li @ 2019-01-14 12:45 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Peter Xu, Markus Armbruster Cc: shirley17fei, qemu-devel, Michael S . Tsirkin 在 2019/1/14 下午6:40, Philippe Mathieu-Daudé 写道: > On 1/14/19 8:18 AM, Peter Xu wrote: >> On Mon, Jan 14, 2019 at 08:02:23AM +0100, Markus Armbruster wrote: >>> Peter Xu <peterx@redhat.com> writes: >>> >>>> On Sun, Jan 13, 2019 at 10:36:41PM +0800, Fei Li wrote: >>>>> From: Fei Li <shirley17fei@gmail.com> >>>>> >>>>> Let's supplement the msi_uninit() when failing to realize >>>>> the pci edu device. >>>>> >>>>> Cc: Markus Armbruster <armbru@redhat.com> >>>>> Cc: Peter Xu <peterx@redhat.com> >>>>> Cc: Michael S. Tsirkin <mst@redhat.com> >>>>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >>>>> Signed-off-by: Fei Li <shirley17fei@gmail.com> >>>>> --- >>>>> hw/misc/edu.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/hw/misc/edu.c b/hw/misc/edu.c >>>>> index cdcf550dd7..4feb7503de 100644 >>>>> --- a/hw/misc/edu.c >>>>> +++ b/hw/misc/edu.c >>>>> @@ -367,6 +367,7 @@ static void pci_edu_uninit(PCIDevice *pdev) >>>>> { >>>>> EduState *edu = EDU(pdev); >>>>> >>>>> + msi_uninit(pdev); >>>> It would be cleaner to me to call this after the join() since >>>> edu_fact_thread() could potentially use msi_*() helpers then the >>>> destructions follow the reverse order of init. >>> Destruction in reverse creation order is good practice. >>> >>> This being the "QEMU educational PCI device", good practice is even more >>> desirable. >> Very persuasive. With that, I'd like to withdraw my excuse of >> "msi_uninit() is optional" too. :) >> >> Fei, please feel free to pick my r-b if you want to repost, and IMHO >> you can also add: >> >> Reported-by: Markus Armbruster <armbru@redhat.com> >> >> Thanks, >> > Moving msi_uninit() after timer_del(): > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Ok, thanks all! Will do the update and send a new version soon. Have a nice day Fei ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-14 12:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-13 14:36 [Qemu-devel] [PATCH] hw/misc/edu: add msi_uninit() for pci_edu_uninit() Fei Li 2019-01-13 16:54 ` Marcel Apfelbaum 2019-01-14 3:31 ` Peter Xu 2019-01-14 7:02 ` Markus Armbruster 2019-01-14 7:18 ` Peter Xu 2019-01-14 10:40 ` Philippe Mathieu-Daudé 2019-01-14 12:45 ` Fei Li
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.