From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BE2862095DE4C for ; Thu, 10 Aug 2017 03:17:16 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7C3E34B98; Thu, 10 Aug 2017 10:19:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7C3E34B98 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-96.phx2.redhat.com [10.3.116.96]) by smtp.corp.redhat.com (Postfix) with ESMTP id DFDD892B92; Thu, 10 Aug 2017 10:19:34 +0000 (UTC) To: Andrew Fish , edk2-devel References: <41A7B048-4140-4889-AE7C-36A0ED3B8C1C@apple.com> From: Laszlo Ersek Message-ID: <57fe7358-6acd-0b47-ee00-0dd333d4ddeb@redhat.com> Date: Thu, 10 Aug 2017 12:19:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <41A7B048-4140-4889-AE7C-36A0ED3B8C1C@apple.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 10 Aug 2017 10:19:35 +0000 (UTC) Subject: Re: Does a double Page free report "ConvertPages: Incompatible memory types", maybe we could do better. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Aug 2017 10:17:16 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/10/17 03:03, Andrew Fish wrote: > It looks to me like if you Free pages, after you free pages you hit this DEBUG message. > > > https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Mem/Page.c#L790 > > if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == EfiConventionalMemory ? 1 : 0)) { > DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory types\n")); > return EFI_NOT_FOUND; > } > > I'm not sure I've thought out all the paths, but would it make more sense if you are trying to convert EfiConventionalMemory to EfiConventionalMemory that you are trying to free pages that are already freed. That is not very obvious from the above DEBUG print. Could there be an if in the error path to print a better DEBUG message for a free pages bug? > > Also to be pedantic the function change names to: CoreConvertPagesEx(). (Reacting only to this side question:) I generally prefer: DEBUG (( DebugMask, "%a: ...", __FUNCTION__, ... )); This way when the function is renamed, or the DEBUG is moved to another function, the log will update itself. Taking it one step further: if the DEBUG is in a widely used library instance, then I like DEBUG (( DebugMask, "%a:%a: ...", gEfiCallerBaseName, __FUNCTION__, ... )); Because this identifies the calling driver module as well (using the BASE_NAME from the module's INF file). ("%g" with &gEfiCallerIdGuid would be more robust than "%a" with gEfiCallerBaseName, but the log should also be readable...) Thanks Laszlo