[PATCH xserver] edid: Prune duplicates after adding modes from DDC

[Michel Dänzer]

On 20/03/17 02:32 PM, Chen, Xiaogang wrote:
>> From: Michel Dänzer [mailto:michel at daenzer.net]
>> On 23/02/17 06:46 AM, Chen, Xiaogang wrote:
>>>> From: Michel Dänzer [mailto:michel at daenzer.net]
>>>>
>>>> Multiple calls to xf86EdidMonitorSet (which can be triggered e.g. by
>>>> running
>>>> xrandr) would potentially keep adding the same modes, causing the
>>>> Monitor-
>>>>> Modes list to keep growing larger and using up more memory.
>>>>
>>>> Fix this by calling xf86PruneDuplicateModes after adding the modes
>>>> returned by xf86DDCGetModes. This removes any newly added modes
>> which
>>>> were already in the Monitor->Modes list before, but keeps new modes
>>>> which weren't yet.
>>>>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99521
>>>> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
>>>> ---
>>>>
>>>> Xiaogang / Jorge, does this fix the memory leak you're seeing?
>>>>
>>> [XCHEN] Hi Michel: I tested your patch with same script I used before
>>> (while true; do xrandr --verbose; done), the memory leak is still
>>> there.
>>
>> I stepped through the execution of the xf86PruneDuplicateModes call added by
>> my patch in gdb and verified that it works as intended, eliminating the
>> duplicate modes.
>>
>> I suspect you're being mislead my the memleax tool: It reports any memory
>> which is allocated and not freed within a certain period of time (10 seconds by
>> default). However, that's expected to happen here, not necessarily a sign of a
>> leak, since we're keeping the list of modes in
>> Monitor->Modes. (xf86PruneDuplicateModes may delete the old modes from
>> the previous xf86DDCGetModes call, not the new ones allocated in the current
>> call)
> [XCHEN] I have same understanding on memleax tool: it reports memory
> allocated and not freed in a certain time. It just gives clues where
> the memory leaks may happen.
> We claim there is memory leak by using top or htop cmd to see Xorg
> virtual memory space size or system memory consumption. They keep
> increasing with or without your patch.

I confirmed that my patch fixes the leak (technically not a leak but
unbounded growth in memory allocation) in xf86EdidMonitorSet.

You may be running into other similar issues. You can try isolating
those e.g. with the valgrind vgdb technique I described.

(Note that in theory it's even possible for the virtual memory usage to
keep growing without any leaks or similar issues, due to heap
fragmentation. But at this point it seems more likely that there are
other similar issues in the xserver code)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer