Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751202AbdCQOTT (ORCPT ); Fri, 17 Mar 2017 10:19:19 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:41412 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751096AbdCQOSi (ORCPT ); Fri, 17 Mar 2017 10:18:38 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org A3CBE600EC Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=shankerd@codeaurora.org Reply-To: shankerd@codeaurora.org Subject: Re: [PATCH v3] irqchip/gicv3-its: Avoid memory over allocation for ITEs References: <1488896720-6223-1-git-send-email-shankerd@codeaurora.org> To: Marc Zyngier Cc: Vikram Sethi , Thomas Gleixner , Jason Cooper , linux-arm-kernel , linux-kernel From: Shanker Donthineni Message-ID: Date: Fri, 17 Mar 2017 09:18:33 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2434 Lines: 56 Hi Marc, On 03/17/2017 08:50 AM, Marc Zyngier wrote: > On 07/03/17 14:25, Shanker Donthineni wrote: >> We are always allocating extra 255Bytes of memory to handle ITE >> physical address alignment requirement. The kmalloc() satisfies >> the ITE alignment since the ITS driver is requesting a minimum >> size of ITS_ITT_ALIGN bytes. >> >> Let's try to allocate the exact amount of memory that is required >> for ITEs to avoid wastage. >> >> Signed-off-by: Shanker Donthineni >> ---Hi >> v2: removed 'Change-Id: Ia8084189833f2081ff13c392deb5070c46a64038' from commit. >> v3: changed from IITE to ITE. >> >> drivers/irqchip/irq-gic-v3-its.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index 86bd428..5aeca78 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -1329,8 +1329,13 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id, >> */ >> nr_ites = max(2UL, roundup_pow_of_two(nvecs)); >> sz = nr_ites * its->ite_size; >> - sz = max(sz, ITS_ITT_ALIGN) + ITS_ITT_ALIGN - 1; >> + sz = max(sz, ITS_ITT_ALIGN); >> itt = kzalloc(sz, GFP_KERNEL); >> + if (itt && !IS_ALIGNED(virt_to_phys(itt), ITS_ITT_ALIGN)) { >> + kfree(itt); >> + itt = kzalloc(sz + ITS_ITT_ALIGN - 1, GFP_KERNEL); >> + } >> + > Is this really worth the complexity? Are you aware of a system where the > accumulation of overallocation actually shows up as being an issue? As such there is no issue with over allocation. Actually this change masked QDF2400 bug 'iirqchip/gicv3-its: Add workaround for QDF2400 ITS erratum 0065' till now, found and fixed recently while looking at the code for possible memory optimizations. > If you want to be absolutely exact in your allocation, then I'd suggest > doing it all the time, and have a proper dedicated allocator that always > do the right thing, without a wasteful fallback like you still have here. We don't need to fallbak, and it can be removed safely. Looking for your suggestion. should I implement a dedicated allocator or remove fallbak for simpler code? > Thanks, > > M. -- Shanker Donthineni Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.