On 23/02/17 06:46 AM, Chen, Xiaogang wrote: >> From: Michel Dänzer [mailto:mic...@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.daen...@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) valgrind's vgdb support is more useful for analyzing this. Example session without my patch: Run Xorg in valgrind, e.g. something like sudo -b valgrind Xorg :1 Run a client in the background, to prevent the server from resetting after each invocation of xrandr: DISPLAY=:1 xterm& Determine the --pid argument to use for vgdb: sudo vgdb -l use --pid=15930 for /usr/bin/valgrind.bin Xorg Get the baseline memory consumption: sudo vgdb --pid=15930 leak_check summary sending command leak_check summary to pid 15930 ==15930== LEAK SUMMARY: ==15930== definitely lost: 20,983 (+20,983) bytes in 87 (+87) blocks ==15930== indirectly lost: 5,930 (+5,930) bytes in 92 (+92) blocks ==15930== possibly lost: 5,802,594 (+5,802,594) bytes in 37,436 (+37,436) blocks ==15930== still reachable: 10,608,319 (+10,608,391) bytes in 20,966 (+20,966) blocks ==15930== suppressed: 0 (+0) bytes in 0 (+0) blocks ==15930== Reachable blocks (those to which a pointer was found) are not shown. ==15930== To see them, add 'reachable any' args to leak_check ==15930== Run xrandr and get the updated memory consumption: DISPLAY=:1 xrandr >/dev/null sudo vgdb --pid=15930 leak_check sending command leak_check to pid 15930 ==15930== LEAK SUMMARY: ==15930== definitely lost: 20,983 (+0) bytes in 87 (+0) blocks ==15930== indirectly lost: 5,930 (+0) bytes in 92 (+0) blocks ==15930== possibly lost: 5,802,594 (+0) bytes in 37,436 (+0) blocks ==15930== still reachable: 10,610,170 (+1,851) bytes in 20,986 (+20) blocks ==15930== suppressed: 0 (+0) bytes in 0 (+0) blocks ==15930== Reachable blocks (those to which a pointer was found) are not shown. ==15930== To see them, add 'reachable any' args to leak_check ==15930== The xrandr invocation caused reachable memory to grow by 1851 bytes in 20 blocks. With my patch applied, this doesn't report any increase in reachable memory[0], no matter how many times I run xrandr in the meantime. [0] except for the first time, due to https://bugs.freedesktop.org/show_bug.cgi?id=99521#c26 Run "sudo vgdb --pid=<PID> help" to get some help about possible commands, or see http://www.valgrind.org/docs/manual/mc-manual.html#mc manual.monitor-commands for more information about valgrind's vgdb support. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel