* [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.