From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B5BC2192E6 for ; Mon, 24 Feb 2025 11:54:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740398099; cv=none; b=E0on+X3CDfLxLF8l0X82opiVE+ihz590rDhU5osKgfsALrlLeQ878AMIrcpV5at+vAbmTjKbwga5KsoKOmDMely7hc6quzSjP653O3vwZ2Wg7Wpb8aQPmIIbLaR3Tzz7KvdTFuGzxh542Pr/LBiwImtHPsnvTUWRaAI66YXp5bQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740398099; c=relaxed/simple; bh=rOprXm82okdY/KlMUZ3w74nT4emJ5jwW/UJLnTKC3ts=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MyWWvSauQm0RNb8Tsm8YMTxB0uHiR55tBdicuohd+KYJIDzgZp2gqARHOH4OYtgf4l34CRd5LvhA/6oN7XVD5WrwZopY8ufZhz8Vuk0LMdhTJYwG7Fu2Fa9Kc8R4rnLK3e7899BiOM6WpDeiFUfU7LqKdcKKNWsU6KUEhZXpmbk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=gsIpx2Cq; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="gsIpx2Cq" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-220e83d65e5so77670435ad.1 for ; Mon, 24 Feb 2025 03:54:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1740398097; x=1741002897; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=v+iFNVpGUmMFzQSXzDgeeu0QHpe9S71wTLihb0U3yO4=; b=gsIpx2CqJUuyYo/A4970OBGoh6pV0GYfwBQafF7Cd610+2okUfjmCVHOPgqzEfIdz7 cV2PgxIEdWbhxHEq8q7fZDf8JYcN4eytEF0g1DKkb53PdjDoXNkGE5Pk7DNuvKquiyNF YRIfLxoyj9A4GWdB73M6EkGIXKHjDHDli24OUjaDUqEHt8T8n3lu9NXBju3U9X5mw1YP S7Au6MjLr8Kav2vlci+CRC00L780DC5+vo9EUn5VFUZxp9sUljIVMh/4/8NKKqoP3pnw 5w9JS/S939rwPEF2HVTqtK+LYIlD/PP1VE480xp1q7/KBPS3s/1Fzvw16GRyoHbnQAXU HwIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740398097; x=1741002897; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=v+iFNVpGUmMFzQSXzDgeeu0QHpe9S71wTLihb0U3yO4=; b=nIzsPrZVQm15ZrPonFmMCC/jPino7hmlPmbMVortGdCmC93scSABOnqnSWix7B+Gf/ ZTE/u3QZMU1hALkVsQt0CDTfnnQ1hSTS7RD43a84H6V4apAnzGKCMk6o1mdyLlbOWzsU LQKkI20EswnDdzgIud/ytj4s69qsEVycOAEuxw7pslld1Omva0wjxz+7p8h3ECTqNG2v U+/6qj6mmoOyR7XyrrWAUBaxH2fMYlaa8XxgDLB+GkAlncJDOQV4LwTXrNbghjB5IaAX wyRFv5pQHYCFZ1P+3LfZ2Pv4Y/N6CSWabs+amhxGTT2QDnHPZttX07atTJulXxrPwbQE FU/Q== X-Forwarded-Encrypted: i=1; AJvYcCVoj/YnQ2Yfp1z2iIWTBsFgC0Qu2chuByZfpxRLAvgE86MHd2meG7SLJX9ONQ/KTnEVR5YMcTDUAx5vumw=@vger.kernel.org X-Gm-Message-State: AOJu0YwCAsePvi5SqPo4MpCMgMS8kR/qt+MLkQJXoPJq4XwzWpDrRoCf solfZ9RvvM07Ohkpk1NUEapGIDibsu7NkqDtfvmHWElqt2X1OGWmv1Rv2Lu86Q== X-Gm-Gg: ASbGncsEclW1dCaxcUhGPNOu6wl5ZnN3gDRBlh3RpXhkmFtv3dVWv4pmXrNnMwi+5jx AgNQUXwPQwCf6O7Ve77g0YlxzKwBIgoHyf+H6y2+3mX2OvMD9h03z6hgtV8Joi8E1B7MuvYaetQ 1lht/JkoPxejcdu8LRWLI7WCw7J8B+XN8yEIGpfxNQWWbbovchOoWOO0wvoMIbp37TqondWhlcW 28NkmyL7onfjn6Of/zfrn0hdrZXlm0MAZcE0LTb++SEGXOpisISta1KsHl3S1J8Q9qRmgv2MZI2 kftCuUvc7DkXF63FYvHD+RW33Nl1BZfDnUfzyWg= X-Google-Smtp-Source: AGHT+IHNJW28+htkmToyI+q9OTvpWx6Kawty/jaa2u3yXSLrfbqqTtiiK6tDw4N1s5eKYCkye4wF/A== X-Received: by 2002:a05:6a21:501:b0:1d9:c615:d1e6 with SMTP id adf61e73a8af0-1eef3b1aa6dmr23697828637.0.1740398097482; Mon, 24 Feb 2025 03:54:57 -0800 (PST) Received: from thinkpad ([220.158.156.158]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7326d348076sm16024354b3a.77.2025.02.24.03.54.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Feb 2025 03:54:57 -0800 (PST) Date: Mon, 24 Feb 2025 17:24:52 +0530 From: Manivannan Sadhasivam To: Anand Moon Cc: Kevin Xie , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Rob Herring , Bjorn Helgaas , Conor Dooley , Minda Chen , "open list:PCIE DRIVER FOR STARFIVE JH71x0" , open list Subject: Re: [PATCH v1] PCI: starfive: Fix kmemleak in StarFive PCIe driver's IRQ handling Message-ID: <20250224115452.micfqctwjkt6gwrs@thinkpad> References: <20250208140110.2389-1-linux.amoon@gmail.com> <20250210174400.b63bhmtkuqhktb57@thinkpad> <20250214060935.cgnc436upawnfzn6@thinkpad> <20250224080129.zm7fvxermgeyycav@thinkpad> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Mon, Feb 24, 2025 at 03:38:29PM +0530, Anand Moon wrote: > Hi Manivannan > > On Mon, 24 Feb 2025 at 13:31, Manivannan Sadhasivam > wrote: > > > > On Thu, Feb 20, 2025 at 03:53:31PM +0530, Anand Moon wrote: > > > > [...] > > > > > Following the change fix this warning in a kernel memory leak. > > > Would you happen to have any comments on these changes? > > > > > > diff --git a/drivers/pci/controller/plda/pcie-plda-host.c > > > b/drivers/pci/controller/plda/pcie-plda-host.c > > > index 4153214ca410..5a72a5a33074 100644 > > > --- a/drivers/pci/controller/plda/pcie-plda-host.c > > > +++ b/drivers/pci/controller/plda/pcie-plda-host.c > > > @@ -280,11 +280,6 @@ static u32 plda_get_events(struct plda_pcie_rp *port) > > > return events; > > > } > > > > > > -static irqreturn_t plda_event_handler(int irq, void *dev_id) > > > -{ > > > - return IRQ_HANDLED; > > > -} > > > - > > > static void plda_handle_event(struct irq_desc *desc) > > > { > > > struct plda_pcie_rp *port = irq_desc_get_handler_data(desc); > > > @@ -454,13 +449,10 @@ int plda_init_interrupts(struct platform_device *pdev, > > > > > > if (event->request_event_irq) > > > ret = event->request_event_irq(port, event_irq, i); > > > - else > > > - ret = devm_request_irq(dev, event_irq, > > > - plda_event_handler, > > > - 0, NULL, port); > > > > This change is not related to the memleak. But I'd like to have it in a separate > > patch since this code is absolutely not required, rather pointless. > > > Yes, remove these changes to fix the memory leak issue I observed. > Sorry, I don't get you. This specific code change of removing 'devm_request_irq' is not supposed to fix memleak. If you are seeing the memleak getting fixed because of it, then something is wrong with the irq implementation. You need to figure it out. > > > > > > if (ret) { > > > dev_err(dev, "failed to request IRQ %d\n", event_irq); > > > + irq_dispose_mapping(event_irq); > > > > So the issue overall is that the 'devm_request_irq' fails on your platform > > because the interrupts are not defined in DT and then the IRQ is not disposed in > > the error path? > > > On microchip PCIe driver utilizes interrupt resources from its > "bridge" and "cntrl" > components, accessed via ioremap, to handle hardware events. > These resources are absent in the StarFive PCIe controller. > > While the StarFive JH-7110 datasheet [1] indicates support for PME, MSI, and INT > messages and specific implementation details for leveraging these interrupts are > unavailable. > > As per StarFive JH-7110 Datasheet PCI support [1] > 1 Power Management Event (PME message) ◦ > 2 MSI (up to 32) and INT message support > > [1] https://doc-en.rvspace.org/JH7110/PDF/JH7110I_DS.pdf > Fine. > > In that case, none of the error paths below for_each_set_bit() loop is disposing > > the IRQs. Also, calling 'irq_dispose_mapping()' only once is not going to > > dispose all mappings that were created before in the loop. > > I was looking at how the IRQ error path will clean up IRQ in case of failure > hence, I added this in case of failure to unmap IRQ events, > I will drop this if not required. Absolutely not. These are fixing the irq leaks. But if it is not related to the 'memleak' you observed, then these should be part of a separate patch. - Mani -- மணிவண்ணன் சதாசிவம்