<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:#954F72;
        text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
        {mso-style-priority:99;
        mso-style-link:"Plain Text Char";
        margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
span.PlainTextChar
        {mso-style-name:"Plain Text Char";
        mso-style-priority:99;
        mso-style-link:"Plain Text";
        font-family:"Calibri",sans-serif;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-family:"Calibri",sans-serif;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoPlainText">Hi Michel, Peter:<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Thank you for your review. I understand your concerns. I used memleax to monitor memory alloc/dealloc(<a href="https://github.com/WuBingzheng/memleax">https://github.com/WuBingzheng/memleax</a>) by attaching it to Xorg process. This
 tool can record call stacks that lead to memory allocated but the memory blocks have not been release yet. Its stack monitoring result is attached here.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Inside function <b>xf86DDCGetModes</b> some DisplayMode are allocated(such as by DDCModesFromEstablished(), etc). To be honest, I do not know why it does that, but I do not find these DisplayMode(s) got used afterword, and they are not
 release either afterword. It is the reason I dealloc these DisplayMode at <span lang="EN">
xf86EdidMonitorSet().<o:p></o:p></span></p>
<p class="MsoPlainText"><span lang="EN"><o:p> </o:p></span></p>
<p class="MsoPlainText">To verify that, I ran the script<b>: while true; do xrandr --verbose; done</b><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">With original code, by using top or htop, I sought the virtual memory consumption of Xorg process or system memory consumption keep going up(1MB every 1-2 second) when running the script above. The memleax pointed to
<b>xf86DDCGetModes </b>has allocated some memory(DisplayMode) and these memory blocks are not released so far.<b>
<o:p></o:p></b></p>
<p class="MsoPlainText"><b><o:p> </o:p></b></p>
<p class="MsoPlainText">After delete these<b> </b>DisplayMode the memory consumption of Xorg(and system used mem) keeps very stable when running same script above. And I did not see any regression so far.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Please correct me if I did not understand why inside xf86DDCGetMode some<b>
</b>DisplayMode are allocated. Thanks.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Xiaogang<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">-----Original Message-----<br>
From: Michel Dänzer [mailto:michel@daenzer.net] <br>
Sent: Monday, February 20, 2017 8:05 PM<br>
To: Xiaogang Chen <chenxiaogang888@gmail.com><br>
Cc: xorg-devel@lists.x.org; Chen, Xiaogang <Xiaogang.Chen@amd.com><br>
Subject: Re: [xserver] xfree86: fix Xorg memory leak when run xrandr</p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">On 21/02/17 03:55 AM, Xiaogang Chen wrote:<o:p></o:p></p>
<p class="MsoPlainText">> xf86DDCGetModes generates some DisplayMods that are not used and not released after.<o:p></o:p></p>
<p class="MsoPlainText">> Fixes: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=99521">
<span style="color:windowtext;text-decoration:none">https://bugs.freedesktop.org/show_bug.cgi?id=99521</span></a><o:p></o:p></p>
<p class="MsoPlainText">> <o:p></o:p></p>
<p class="MsoPlainText">> Signed-off-by: Xiaogang Chen <<a href="mailto:Xiaogang.Chen@amd.com"><span style="color:windowtext;text-decoration:none">Xiaogang.Chen@amd.com</span></a>><o:p></o:p></p>
<p class="MsoPlainText">> ---<o:p></o:p></p>
<p class="MsoPlainText">>  hw/xfree86/modes/xf86EdidModes.c | 17 +++++------------<o:p></o:p></p>
<p class="MsoPlainText">>  1 file changed, 5 insertions(+), 12 deletions(-)<o:p></o:p></p>
<p class="MsoPlainText">> <o:p></o:p></p>
<p class="MsoPlainText">> diff --git a/hw/xfree86/modes/xf86EdidModes.c <o:p></o:p></p>
<p class="MsoPlainText">> b/hw/xfree86/modes/xf86EdidModes.c<o:p></o:p></p>
<p class="MsoPlainText">> index f0e1e97..915b291 100644<o:p></o:p></p>
<p class="MsoPlainText">> --- a/hw/xfree86/modes/xf86EdidModes.c<o:p></o:p></p>
<p class="MsoPlainText">> +++ b/hw/xfree86/modes/xf86EdidModes.c<o:p></o:p></p>
<p class="MsoPlainText">> @@ -1201,18 +1201,11 @@ xf86EdidMonitorSet(int scrnIndex, MonPtr Monitor, xf86MonPtr DDC)<o:p></o:p></p>
<p class="MsoPlainText">>          /* look for last Mode */<o:p></o:p></p>
<p class="MsoPlainText">>          Mode = Modes;<o:p></o:p></p>
<p class="MsoPlainText">>  <o:p></o:p></p>
<p class="MsoPlainText">> -        while (Mode->next)<o:p></o:p></p>
<p class="MsoPlainText">> -            Mode = Mode->next;<o:p></o:p></p>
<p class="MsoPlainText">> +        /*release these video modes that are not used after*/<o:p></o:p></p>
<p class="MsoPlainText">> +        while (Mode) {<o:p></o:p></p>
<p class="MsoPlainText">> +           xf86DeleteMode(&Modes, Mode);<o:p></o:p></p>
<p class="MsoPlainText">> +           Mode = Modes;<o:p></o:p></p>
<p class="MsoPlainText">> +       }<o:p></o:p></p>
<p class="MsoPlainText">>  <o:p></o:p></p>
<p class="MsoPlainText">> -        /* add to MonPtr */<o:p></o:p></p>
<p class="MsoPlainText">> -        if (Monitor->Modes) {<o:p></o:p></p>
<p class="MsoPlainText">> -            Monitor->Last->next = Modes;<o:p></o:p></p>
<p class="MsoPlainText">> -            Modes->prev = Monitor->Last;<o:p></o:p></p>
<p class="MsoPlainText">> -            Monitor->Last = Mode;<o:p></o:p></p>
<p class="MsoPlainText">> -        }<o:p></o:p></p>
<p class="MsoPlainText">> -        else {<o:p></o:p></p>
<p class="MsoPlainText">> -            Monitor->Modes = Modes;<o:p></o:p></p>
<p class="MsoPlainText">> -            Monitor->Last = Mode;<o:p></o:p></p>
<p class="MsoPlainText">> -        }<o:p></o:p></p>
<p class="MsoPlainText">>      }<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">This doesn't make sense to me I'm afraid. The intention of the existing code is to append all modes in the Modes list to the Monitor->Modes list. You're changing it to just delete all modes in the Modes list instead.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">I'm not sure how this function could be responsible for the memory leaks you're seeing. Can you provide valgrind output about those leaks?<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">-- <o:p></o:p></p>
<p class="MsoPlainText">Earthling Michel Dänzer               |               <a href="http://www.amd.com">
<span style="color:windowtext;text-decoration:none">http://www.amd.com</span></a><o:p></o:p></p>
<p class="MsoPlainText">Libre software enthusiast             |             Mesa and X developer<o:p></o:p></p>
</div>
</body>
</html>