From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YybEK-0004iR-5M for qemu-devel@nongnu.org; Sat, 30 May 2015 03:30:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YybEI-0007WQ-R0 for qemu-devel@nongnu.org; Sat, 30 May 2015 03:30:52 -0400 Message-ID: <5569664B.5030403@huawei.com> Date: Sat, 30 May 2015 15:27:07 +0800 From: Shannon Zhao MIME-Version: 1.0 References: <1432814932-12608-1-git-send-email-zhaoshenglong@huawei.com> <1432814932-12608-30-git-send-email-zhaoshenglong@huawei.com> <20150528151118.78bed6a5.cornelia.huck@de.ibm.com> In-Reply-To: <20150528151118.78bed6a5.cornelia.huck@de.ibm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 29/29] hw/s390x/sclpcpu.c: Fix memory leak spotted by valgrind List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: peter.maydell@linaro.org, qemu-trivial@nongnu.org, mjt@tls.msk.ru, qemu-devel@nongnu.org, shannon.zhao@linaro.org, pbonzini@redhat.com On 2015/5/28 21:11, Cornelia Huck wrote: > On Thu, 28 May 2015 20:08:52 +0800 > Shannon Zhao wrote: > >> > From: Shannon Zhao >> > >> > valgrind complains about: >> > ==1413== 188 (8 direct, 180 indirect) bytes in 1 blocks are definitely lost in loss record 951 of 1,199 >> > ==1413== at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) >> > ==1413== by 0x262D8B: malloc_and_trace (vl.c:2556) >> > ==1413== by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) >> > ==1413== by 0x2C7ACF: qemu_extend_irqs (irq.c:55) >> > ==1413== by 0x2C7B5B: qemu_allocate_irqs (irq.c:64) >> > ==1413== by 0x2168F4: irq_cpu_hotplug_init (sclpcpu.c:84) >> > ==1413== by 0x21623F: event_realize (event-facility.c:385) >> > ==1413== by 0x2C4610: device_set_realized (qdev.c:1058) >> > ==1413== by 0x317DDA: property_set_bool (object.c:1514) >> > ==1413== by 0x3166D4: object_property_set (object.c:837) >> > ==1413== by 0x3189F6: object_property_set_qobject (qom-qobject.c:24) >> > ==1413== by 0x316943: object_property_set_bool (object.c:905) >> > >> > Signed-off-by: Shannon Zhao >> > Signed-off-by: Shannon Zhao >> > --- >> > hw/s390x/sclpcpu.c | 9 ++++++--- >> > 1 file changed, 6 insertions(+), 3 deletions(-) >> > >> > diff --git a/hw/s390x/sclpcpu.c b/hw/s390x/sclpcpu.c >> > index 2fe8b5a..1090e2f 100644 >> > --- a/hw/s390x/sclpcpu.c >> > +++ b/hw/s390x/sclpcpu.c >> > @@ -25,13 +25,13 @@ typedef struct ConfigMgtData { >> > uint8_t event_qualifier; >> > } QEMU_PACKED ConfigMgtData; >> > >> > -static qemu_irq *irq_cpu_hotplug; /* Only used in this file */ >> > +static qemu_irq irq_cpu_hotplug; /* Only used in this file */ >> > >> > #define EVENT_QUAL_CPU_CHANGE 1 >> > >> > void raise_irq_cpu_hotplug(void) >> > { >> > - qemu_irq_raise(*irq_cpu_hotplug); >> > + qemu_irq_raise(irq_cpu_hotplug); >> > } >> > >> > static unsigned int send_mask(void) >> > @@ -81,7 +81,10 @@ static void trigger_signal(void *opaque, int n, int level) >> > >> > static int irq_cpu_hotplug_init(SCLPEvent *event) >> > { >> > - irq_cpu_hotplug = qemu_allocate_irqs(trigger_signal, event, 1); >> > + qemu_irq irq = qemu_allocate_irq(trigger_signal, event, 0); >> > + >> > + irq_cpu_hotplug = irq; >> > + qemu_free_irq(irq); > This looks... odd. But then, the whole code structure here with its > global variable and its roundabout way of triggering the notification > looks weird. We'd probably be better off looking up the sclp event for > cpu hotplug and triggering the interrupt directly. > Yeah, it's odd and　I don't find who calls raise_irq_cpu_hotplug(). -- Shannon