mbox series

[v2,00/15] Fix check-qtest-ppc64 sanitizer errors

Message ID 20240627-san-v2-0-750bb0946dbd@daynix.com
Headers show
Series Fix check-qtest-ppc64 sanitizer errors | expand

Message

Akihiko Odaki June 27, 2024, 1:37 p.m. UTC
Based-on: <3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathbern@quicinc.com>
("[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'")

I saw various sanitizer errors when running check-qtest-ppc64. While
I could just turn off sanitizers, I decided to tackle them this time.

Unfortunately, GLib does not free test data in some cases so some
sanitizer errors remain. All sanitizer errors will be gone with this
patch series combined with the following change for GLib:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v2:
- Rebased to "[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'".
  (Philippe Mathieu-Daudé)
- Converted IRQs into GPIO lines and removed one qemu_irq usage.
  (Peter Maydell)
- s/suppresses/fixes/ (Michael S. Tsirkin)
- Corrected title of patch "hw/virtio: Free vqs after vhost_dev_cleanup()"
  (was "hw/virtio: Free vqs before vhost_dev_cleanup()")
- Link to v1: https://lore.kernel.org/r/20240626-san-v1-0-f3cc42302189@daynix.com

---
Akihiko Odaki (15):
      cpu: Free cpu_ases
      hw/ide: Convert macio ide_irq into GPIO line
      hw/ide: Remove internal DMA qemu_irq
      hw/isa/vt82c686: Define a GPIO line between vt82c686 and i8259
      spapr: Free stdout path
      ppc/vof: Fix unaligned FDT property access
      hw/virtio: Free vqs after vhost_dev_cleanup()
      migration: Free removed SaveStateEntry
      memory: Do not create circular reference with subregion
      tests/qtest: Use qtest_add_data_func_full()
      tests/qtest: Free unused QMP response
      tests/qtest: Free old machine variable name
      tests/qtest: Delete previous boot file
      tests/qtest: Free paths
      tests/qtest: Free GThread

 include/hw/ppc/mac_dbdma.h           |  5 +++--
 hw/core/cpu-common.c                 |  1 +
 hw/ide/macio.c                       | 18 +++++++++++++-----
 hw/isa/vt82c686.c                    |  7 ++++---
 hw/misc/macio/mac_dbdma.c            | 10 +++++-----
 hw/ppc/spapr_vof.c                   |  2 +-
 hw/ppc/vof.c                         |  2 +-
 hw/virtio/vhost-user-base.c          |  2 ++
 migration/savevm.c                   |  2 ++
 system/memory.c                      | 11 +++++++++--
 tests/qtest/device-introspect-test.c |  7 +++----
 tests/qtest/libqtest.c               |  3 +++
 tests/qtest/migration-test.c         | 18 +++++++++++-------
 tests/qtest/qos-test.c               | 16 ++++++++++++----
 tests/qtest/vhost-user-test.c        |  6 +++---
 15 files changed, 73 insertions(+), 37 deletions(-)
---
base-commit: af799a2337c3e39994411f90631905d809a41da4
change-id: 20240625-san-097afaf4f1c2

Best regards,

Comments

Michael S. Tsirkin July 1, 2024, 8:11 p.m. UTC | #1
On Thu, Jun 27, 2024 at 10:37:43PM +0900, Akihiko Odaki wrote:
> Based-on: <3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathbern@quicinc.com>
> ("[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'")
> 
> I saw various sanitizer errors when running check-qtest-ppc64. While
> I could just turn off sanitizers, I decided to tackle them this time.
> 
> Unfortunately, GLib does not free test data in some cases so some
> sanitizer errors remain. All sanitizer errors will be gone with this
> patch series combined with the following change for GLib:
> https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

who's merging all this?

> ---
> Changes in v2:
> - Rebased to "[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'".
>   (Philippe Mathieu-Daudé)
> - Converted IRQs into GPIO lines and removed one qemu_irq usage.
>   (Peter Maydell)
> - s/suppresses/fixes/ (Michael S. Tsirkin)
> - Corrected title of patch "hw/virtio: Free vqs after vhost_dev_cleanup()"
>   (was "hw/virtio: Free vqs before vhost_dev_cleanup()")
> - Link to v1: https://lore.kernel.org/r/20240626-san-v1-0-f3cc42302189@daynix.com
> 
> ---
> Akihiko Odaki (15):
>       cpu: Free cpu_ases
>       hw/ide: Convert macio ide_irq into GPIO line
>       hw/ide: Remove internal DMA qemu_irq
>       hw/isa/vt82c686: Define a GPIO line between vt82c686 and i8259
>       spapr: Free stdout path
>       ppc/vof: Fix unaligned FDT property access
>       hw/virtio: Free vqs after vhost_dev_cleanup()
>       migration: Free removed SaveStateEntry
>       memory: Do not create circular reference with subregion
>       tests/qtest: Use qtest_add_data_func_full()
>       tests/qtest: Free unused QMP response
>       tests/qtest: Free old machine variable name
>       tests/qtest: Delete previous boot file
>       tests/qtest: Free paths
>       tests/qtest: Free GThread
> 
>  include/hw/ppc/mac_dbdma.h           |  5 +++--
>  hw/core/cpu-common.c                 |  1 +
>  hw/ide/macio.c                       | 18 +++++++++++++-----
>  hw/isa/vt82c686.c                    |  7 ++++---
>  hw/misc/macio/mac_dbdma.c            | 10 +++++-----
>  hw/ppc/spapr_vof.c                   |  2 +-
>  hw/ppc/vof.c                         |  2 +-
>  hw/virtio/vhost-user-base.c          |  2 ++
>  migration/savevm.c                   |  2 ++
>  system/memory.c                      | 11 +++++++++--
>  tests/qtest/device-introspect-test.c |  7 +++----
>  tests/qtest/libqtest.c               |  3 +++
>  tests/qtest/migration-test.c         | 18 +++++++++++-------
>  tests/qtest/qos-test.c               | 16 ++++++++++++----
>  tests/qtest/vhost-user-test.c        |  6 +++---
>  15 files changed, 73 insertions(+), 37 deletions(-)
> ---
> base-commit: af799a2337c3e39994411f90631905d809a41da4
> change-id: 20240625-san-097afaf4f1c2
> 
> Best regards,
> -- 
> Akihiko Odaki <akihiko.odaki@daynix.com>
BALATON Zoltan July 1, 2024, 10:23 p.m. UTC | #2
On Mon, 1 Jul 2024, Michael S. Tsirkin wrote:
> On Thu, Jun 27, 2024 at 10:37:43PM +0900, Akihiko Odaki wrote:
>> Based-on: <3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathbern@quicinc.com>
>> ("[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'")
>>
>> I saw various sanitizer errors when running check-qtest-ppc64. While
>> I could just turn off sanitizers, I decided to tackle them this time.
>>
>> Unfortunately, GLib does not free test data in some cases so some
>> sanitizer errors remain. All sanitizer errors will be gone with this
>> patch series combined with the following change for GLib:
>> https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> who's merging all this?

Maybe needs to be split. Mark had an alternative for macio which was 
picked up by Philippe if I'm not mistaken. I've sent an alternative for 
vt82c686 which is still discussed but could belong to Philippe as well. 
PPC parts could be taken by the PPC maintainers if there were any active 
at the moment and I don't know who maintains tests normally or other misc 
areas.

Regards,
BALATON Zoltan

>> ---
>> Changes in v2:
>> - Rebased to "[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'".
>>   (Philippe Mathieu-Daudé)
>> - Converted IRQs into GPIO lines and removed one qemu_irq usage.
>>   (Peter Maydell)
>> - s/suppresses/fixes/ (Michael S. Tsirkin)
>> - Corrected title of patch "hw/virtio: Free vqs after vhost_dev_cleanup()"
>>   (was "hw/virtio: Free vqs before vhost_dev_cleanup()")
>> - Link to v1: https://lore.kernel.org/r/20240626-san-v1-0-f3cc42302189@daynix.com
>>
>> ---
>> Akihiko Odaki (15):
>>       cpu: Free cpu_ases
>>       hw/ide: Convert macio ide_irq into GPIO line
>>       hw/ide: Remove internal DMA qemu_irq
>>       hw/isa/vt82c686: Define a GPIO line between vt82c686 and i8259
>>       spapr: Free stdout path
>>       ppc/vof: Fix unaligned FDT property access
>>       hw/virtio: Free vqs after vhost_dev_cleanup()
>>       migration: Free removed SaveStateEntry
>>       memory: Do not create circular reference with subregion
>>       tests/qtest: Use qtest_add_data_func_full()
>>       tests/qtest: Free unused QMP response
>>       tests/qtest: Free old machine variable name
>>       tests/qtest: Delete previous boot file
>>       tests/qtest: Free paths
>>       tests/qtest: Free GThread
>>
>>  include/hw/ppc/mac_dbdma.h           |  5 +++--
>>  hw/core/cpu-common.c                 |  1 +
>>  hw/ide/macio.c                       | 18 +++++++++++++-----
>>  hw/isa/vt82c686.c                    |  7 ++++---
>>  hw/misc/macio/mac_dbdma.c            | 10 +++++-----
>>  hw/ppc/spapr_vof.c                   |  2 +-
>>  hw/ppc/vof.c                         |  2 +-
>>  hw/virtio/vhost-user-base.c          |  2 ++
>>  migration/savevm.c                   |  2 ++
>>  system/memory.c                      | 11 +++++++++--
>>  tests/qtest/device-introspect-test.c |  7 +++----
>>  tests/qtest/libqtest.c               |  3 +++
>>  tests/qtest/migration-test.c         | 18 +++++++++++-------
>>  tests/qtest/qos-test.c               | 16 ++++++++++++----
>>  tests/qtest/vhost-user-test.c        |  6 +++---
>>  15 files changed, 73 insertions(+), 37 deletions(-)
>> ---
>> base-commit: af799a2337c3e39994411f90631905d809a41da4
>> change-id: 20240625-san-097afaf4f1c2
>>
>> Best regards,
>> --
>> Akihiko Odaki <akihiko.odaki@daynix.com>
>
>
>
Thomas Huth July 2, 2024, 6:23 a.m. UTC | #3
On 02/07/2024 00.23, BALATON Zoltan wrote:
> On Mon, 1 Jul 2024, Michael S. Tsirkin wrote:
>> On Thu, Jun 27, 2024 at 10:37:43PM +0900, Akihiko Odaki wrote:
>>> Based-on: 
>>> <3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathbern@quicinc.com>
>>> ("[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'")
>>>
>>> I saw various sanitizer errors when running check-qtest-ppc64. While
>>> I could just turn off sanitizers, I decided to tackle them this time.
>>>
>>> Unfortunately, GLib does not free test data in some cases so some
>>> sanitizer errors remain. All sanitizer errors will be gone with this
>>> patch series combined with the following change for GLib:
>>> https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>
>>
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> who's merging all this?
> 
> Maybe needs to be split. Mark had an alternative for macio which was picked 
> up by Philippe if I'm not mistaken. I've sent an alternative for vt82c686 
> which is still discussed but could belong to Philippe as well. PPC parts 
> could be taken by the PPC maintainers if there were any active at the moment 
> and I don't know who maintains tests normally or other misc areas.

I can take the qtest patches through my tree.

  Thomas
Nicholas Piggin July 4, 2024, 11:37 a.m. UTC | #4
On Tue Jul 2, 2024 at 4:23 PM AEST, Thomas Huth wrote:
> On 02/07/2024 00.23, BALATON Zoltan wrote:
> > On Mon, 1 Jul 2024, Michael S. Tsirkin wrote:
> >> On Thu, Jun 27, 2024 at 10:37:43PM +0900, Akihiko Odaki wrote:
> >>> Based-on: 
> >>> <3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathbern@quicinc.com>
> >>> ("[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'")
> >>>
> >>> I saw various sanitizer errors when running check-qtest-ppc64. While
> >>> I could just turn off sanitizers, I decided to tackle them this time.
> >>>
> >>> Unfortunately, GLib does not free test data in some cases so some
> >>> sanitizer errors remain. All sanitizer errors will be gone with this
> >>> patch series combined with the following change for GLib:
> >>> https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120
> >>>
> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >>
> >>
> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >>
> >> who's merging all this?
> > 
> > Maybe needs to be split. Mark had an alternative for macio which was picked 
> > up by Philippe if I'm not mistaken. I've sent an alternative for vt82c686 
> > which is still discussed but could belong to Philippe as well. PPC parts 
> > could be taken by the PPC maintainers if there were any active at the moment 
> > and I don't know who maintains tests normally or other misc areas.
>
> I can take the qtest patches through my tree.
>       cpu: Free cpu_ases
>       migration: Free removed SaveStateEntry
>       memory: Do not create circular reference with subregion
>       hw/virtio: Free vqs after vhost_dev_cleanup()

These are all core code, maybe Philippe, Peter, or Richard,
Fabiano for migration perhaps?

>       hw/ide: Convert macio ide_irq into GPIO line
>       hw/ide: Remove internal DMA qemu_irq
>       hw/isa/vt82c686: Define a GPIO line between vt82c686 and i8259

I guess I'll let you wrangle these and go through Philippe then?

>       spapr: Free stdout path
>       ppc/vof: Fix unaligned FDT property access

These ppc ones I can take, they seem okay. Sorry for my recent
inactivity.

Thanks,
Nick

>       tests/qtest: Use qtest_add_data_func_full()
>       tests/qtest: Free unused QMP response
>       tests/qtest: Free old machine variable name
>       tests/qtest: Delete previous boot file
>       tests/qtest: Free paths
>       tests/qtest: Free GThread