From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59212) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4SLE-0007bS-Fy for qemu-devel@nongnu.org; Mon, 05 Mar 2012 02:28:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4SLC-0002QI-DJ for qemu-devel@nongnu.org; Mon, 05 Mar 2012 02:28:19 -0500 Received: from plane.gmane.org ([80.91.229.3]:45573) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4SLC-0002QD-6N for qemu-devel@nongnu.org; Mon, 05 Mar 2012 02:28:18 -0500 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1S4SL9-0004jf-WA for qemu-devel@nongnu.org; Mon, 05 Mar 2012 08:28:16 +0100 Received: from 213.33.220.118 ([213.33.220.118]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 05 Mar 2012 08:28:15 +0100 Received: from e.voevodin by 213.33.220.118 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 05 Mar 2012 08:28:15 +0100 From: Evgeny Voevodin Date: Mon, 05 Mar 2012 11:28:05 +0400 Message-ID: <4F546B05.9050200@samsung.com> References: <1330680137-6601-1-git-send-email-batuzovk@ispras.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit In-Reply-To: <1330680137-6601-1-git-send-email-batuzovk@ispras.ru> Subject: Re: [Qemu-devel] [PATCH 0/1] Fix large memory chunks allocation with tcg_malloc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: zhur@ispras.ru, Dmitry Solodkiy On 02.03.2012 13:22, Kirill Batuzov wrote: > Currently large memory chunk allocation with tcg_malloc is broken. An attempt > to allocate such chunk when pool_current field of TCGContext is not NULL will > result in circular links in list of memory pools: > > p = new pool; > s->pool_current->next = p; > p->next = s->pool_current; > (in tcg_malloc_internal) > > Later p became a current pool, and current pool became next pool. Next > tcg_malloc will switch current pool to next pool ('previous' current pool) > and will start allocating memory from it's beginning. But some memory in > the beginning of this pool was already allocated and will be used twice > for different arrays. > > At the end of this cover letter there is a patch that demonstrates the > problem. It breaks current trunk on the first translation block containing > labels. > > Large memory pools can not be reused by memory allocator for big allocations > and an attempt to reuse them for small allocations may result in an infinite > increase of memory consumption during run time. Memory consumption would > increase every time a new large chunk of memory is allocated. If code > allocates such chunk on every translation block (like patch at the end of this > letter do) then memory consumption would increase with every new block > translated. > > My fix for the problems mentioned above is in the second e-mail. I moved large > memory pools to a separate list and free them on pool_reset. > As I understand, your approach removes linking back to the previous allocated chunk to avoid usage of already allocated and used memory again. Also you added g_free() to tcg_pool_reset(). Wouldn't it slow down emulation? Maybe such linkage to previous chunk was made with assumption that big allocations will not happen twice one after another? For example, while loading kernel on realview and exynos, the code never reaches this string in tcg_malloc_internal(): p->next = s->pool_current; If this assumption is correct, maybe it's better to just insert a verification that allocated chunk has enough space to hold requested block? + if (s->pool_cur - p->data + size > p->size) { + goto new_pool; + } s->pool_current = p; s->pool_cur = p->data + size; s->pool_end = p->data + p->size; return p->data; -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow R&D center, Samsung Electronics e-mail: e.voevodin@samsung.com