From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753836AbcA3BNS (ORCPT ); Fri, 29 Jan 2016 20:13:18 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:37965 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751642AbcA3BNQ (ORCPT ); Fri, 29 Jan 2016 20:13:16 -0500 Subject: Re: [PATCH v2] irqchip: gicv3-its: Fix memory leak in its_free_tables() To: Thomas Gleixner References: <1454042578-13933-1-git-send-email-shankerd@codeaurora.org> Cc: Marc Zyngier , Vikram Sethi , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Jason Cooper From: Shanker Donthineni Message-ID: <56AC0E2A.1090402@codeaurora.org> Date: Fri, 29 Jan 2016 19:13:14 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/29/2016 02:30 AM, Thomas Gleixner wrote: > On Thu, 28 Jan 2016, Shanker Donthineni wrote: >> @@ -807,9 +810,10 @@ static void its_free_tables(struct its_node *its) >> int i; >> >> for (i = 0; i < GITS_BASER_NR_REGS; i++) { >> - if (its->tables[i]) { >> - free_page((unsigned long)its->tables[i]); >> - its->tables[i] = NULL; >> + if (its->tables[i].base) { >> + free_pages((unsigned long)its->tables[i].base, >> + get_order(its->tables[i].size)); >> + its->tables[i].base = NULL; >> } >> } >> } >> @@ -880,6 +884,7 @@ retry_alloc_baser: >> if (alloc_pages > GITS_BASER_PAGES_MAX) { >> alloc_pages = GITS_BASER_PAGES_MAX; >> order = get_order(GITS_BASER_PAGES_MAX * psz); >> + alloc_size = (1 << order) * PAGE_SIZE; > Why don't you record the order instead of converting back and forth ? I can use page order information to fix memory leak and I will post v3 patch with your suggestion. We have another coding BUG which is related to not refreshing alloc_size whenever order changes. Because we are not updating alloc_size variable here, later part of the code logic uses incorrect alloc_size value in two places as shown below. Code snippet-1: if (!shr) { cache = GITS_BASER_nC; __flush_dcache_area(base, alloc_size); } Code snippet-2: pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n", (int)(alloc_size / entry_size), its_base_type_string[type], (unsigned long)virt_to_phys(base), psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT); How do you suggest I fix the second problem? Should I create another patch or include in v3 patch? > Thanks, > > tglx > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel