From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by sourceware.org (Postfix) with ESMTPS id 9F3F13858D3C for ; Thu, 1 Sep 2022 19:04:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9F3F13858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=foss.st.com Received: from pps.filterd (m0288072.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 281GX867010934; Thu, 1 Sep 2022 21:04:20 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=fhcjN1zq7IIWunkdbPyXMRE7jE40Wt8vhcQraZmTieU=; b=QQqWHLkLVaH5idqQ/Zi9nTalYoS00Xykp6LA+UwYedf+kvSpESmHGEf8GhqvAPm4UBkt n2KrnieYg8id1A+4k+oaPL3UUKZzmgduB6a4xAu1WNkTTcdPaRCXAf7NuCH87ttKnZIw x9iz2pjg8wZV956+SKKTijLKdjVWuDrIQBtkc4UVLrNaDJVh9KeTn7Pyn/muwTR+GxfZ FRr2mC2cACW1GcuQ4O+kXX3eu6UzF9zkA8Yd3D8MjOlTlROl5ipBiAGXqZrST07meOnG j5VuBladisWj2YqopfdSPvGwL+pAxUbyjy2LKDC0mLxNiLGibhT4Z6Z8lnnduo3kA2WS 9g== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3j78pkq3k8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 01 Sep 2022 21:04:20 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 364AC10002A; Thu, 1 Sep 2022 21:04:19 +0200 (CEST) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 29E3726DDC7; Thu, 1 Sep 2022 21:04:19 +0200 (CEST) Received: from [10.252.7.99] (10.75.127.122) by SHFDAG1NODE3.st.com (10.75.129.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2375.7; Thu, 1 Sep 2022 21:04:16 +0200 Message-ID: Date: Thu, 1 Sep 2022 21:04:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.1.2 Subject: Re: [PATCH 2/2] Don't allocate another header when merging chunks Content-Language: en-US To: Jeff Johnston CC: Newlib References: <20220830135625.2247198-1-torbjorn.svensson@foss.st.com> <20220830135625.2247198-2-torbjorn.svensson@foss.st.com> From: Torbjorn SVENSSON In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.75.127.122] X-ClientProxiedBy: GPXDAG2NODE4.st.com (10.75.127.68) To SHFDAG1NODE3.st.com (10.75.129.71) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-09-01_12,2022-08-31_03,2022-06-22_01 X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Sep 2022 19:04:27 -0000 Hi Jeff, Thanks for the review. No, I don't think the MALLOC_MINCHUNK should apply as I think is to ensure that there is enough room for the chunk header. /* size of smallest possible chunk. A memory piece smaller than this size * won't be able to create a chunk */ #define MALLOC_MINCHUNK (CHUNK_OFFSET + MALLOC_PADDING + MALLOC_MINSIZE) In this particular case, there is already a chunk, but it's a bit too small to fit the requested size to malloc(). As a result, it should be the requested size minus the size for the last chunk minus the size of the chunk header. The remaining number of bytes is the require memory to create a chunk (user data + chunk header) that is just the right size. Think of it like this: If there is enough free heap, then malloc(100) would just call _sbrk(108) and return that space. If that area is then free'ed, and a following malloc(200) is called, then there are 3 scenarios: a) there is enough free heap, so it will be like returning the space returned by _sbrk(208). b) these is not enough free heap, and the last chunk is still in use. In this case, malloc would return 0 and errno should be ENOMEM. c) there is not enough free heap, but the last chunk is no used, thus it could be extended. If the the chunk was 108 bytes including the header, then _sbrk(200-108) would be needed to create a single chunk that is 200 bytes + 8 bytes for the header. If you look for the extreme, then the last chunk size could be 1 byte less than what is requested of malloc, like the scenario above. The last chunk size is 100 bytes (plus header) and the application calls malloc(101). Then it would not make sense to apply MALLOC_MINCHUNK, as that would call _sbrk(MALLOC_MINCHUNK) rather than _sbrk(1). In this case, wasting MALLOC_MINCHUNK-1 bytes as they will never be access by the application. Does this make sense or am I overthinking this? Kind regards, Torbjörn Ps. Header size for a chunk on arm-none-eabi is 8 bytes, for other targets, the chunk header can be of different size, but I assumed arm-none-eabi in my comment above to make it more clear. Ps2. _sbrk and malloc is obviously more complicated than my example above, but I tried to reduce the complexity to the involved logic for extending a chunk size. On 2022-09-01 20:44, Jeff Johnston wrote: > I think that the check for MALLOC_MINCHUNK should still apply.  Do you > agree? > > -- Jeff J. > > On Tue, Aug 30, 2022 at 9:57 AM Torbjörn SVENSSON > > > wrote: > > In the nano version of malloc, when the last chunk is to be extended, > there is no need to acount for the header again as it's already taken > into account in the overall "alloc_size" at the beginning of the > function. > > Contributed by STMicroelectronics > > Signed-off-by: Torbjörn SVENSSON > > --- >  newlib/libc/stdlib/nano-mallocr.c | 4 ---- >  1 file changed, 4 deletions(-) > > diff --git a/newlib/libc/stdlib/nano-mallocr.c > b/newlib/libc/stdlib/nano-mallocr.c > index 43eb20e07..b2273ba60 100644 > --- a/newlib/libc/stdlib/nano-mallocr.c > +++ b/newlib/libc/stdlib/nano-mallocr.c > @@ -328,10 +328,6 @@ void * nano_malloc(RARG malloc_size_t s) >                 /* The last free item has the heap end as neighbour. >                  * Let's ask for a smaller amount and merge */ >                 alloc_size -= p->size; > -               alloc_size = ALIGN_SIZE(alloc_size, CHUNK_ALIGN); /* > size of aligned data load */ > -               alloc_size += MALLOC_PADDING; /* padding */ > -               alloc_size += CHUNK_OFFSET; /* size of chunk head */ > -               alloc_size = MAX(alloc_size, MALLOC_MINCHUNK); > >                 if (sbrk_aligned(RCALL alloc_size) != (void *)-1) >                 { > -- > 2.25.1 >