From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA10FC47077 for ; Tue, 16 Jan 2024 14:35:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 635AE6B007B; Tue, 16 Jan 2024 09:35:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5E6016B007D; Tue, 16 Jan 2024 09:35:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4D50B6B0080; Tue, 16 Jan 2024 09:35:17 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 3D8066B007B for ; Tue, 16 Jan 2024 09:35:17 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 16F6C160514 for ; Tue, 16 Jan 2024 14:35:17 +0000 (UTC) X-FDA: 81685421874.13.2DC6672 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf16.hostedemail.com (Postfix) with ESMTP id 44ECE180007 for ; Tue, 16 Jan 2024 14:35:14 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=D4jEQCA1; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf16.hostedemail.com: domain of robh@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=robh@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705415714; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=8e7f81VvOS1e+8fclE1Gnm0C1TOtW+rhXA9Mg8y90kE=; b=6Es82axTAcfLFzqaj+t400O0nDQEyNqtVo8crqE019HmG54G1rxR3R57959KQpdUEmwsYF NZkDjmYE8C3+mfFoTAHPglNjKsSuzjyy1WBmjdfnM3W5iF2ecHkuz84vsrilr9D5uGGbvi VZFKOPiJDp/Xq9wTa7zKplKQGHTSoSk= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=D4jEQCA1; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf16.hostedemail.com: domain of robh@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=robh@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705415714; a=rsa-sha256; cv=none; b=yuCHK9e3Xbys5lXefxozJ3I7r+hw0kIy2/f/xyFNCLCOrxl1LjVRNDWplEPFPQrfSLs9ni +zgbjxbAQNX2jp8MYJQGMZdqH75tzVREJ5rCT3MKEUTnE9F6dEEggRzf+uRiGTa1V0emVe hxhAX6wbt7D1+9sx5TuRyxF45AHC4eA= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 977CC6125C; Tue, 16 Jan 2024 14:35:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 17A51C43390; Tue, 16 Jan 2024 14:35:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705415711; bh=YxRAEs1+500SQYSJEteA7haAvPYo8UdPYN9Xe9/mIio=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=D4jEQCA1oX2gdAuwVRAq1lU9gfTshP5xoyXgnUw5fC28RHQN1BTC/y12Wp2JP8d2Q ZifSys3WVBpWG10QhMIvdm8TRX/BQ0MIIOVa0vPaMbzKxrkQyAxIwkhptUb+9RQa+f 2M7l3yDoJJsO9wq/DESwzXCATOm/bGMCzQRPnnghsL+qwphJBAtxUsSX4L8w4CMAZ9 wZ8WgqQkQf65IsRckBsyg91ZU4ILHZUBhRLHY1FDOzfJ0oXwNtXeB8vhNgvgtlIiQp Ccgf4p10qpBOo8A0I4sbezUfWKcyK3F41Xj7hRikxXHJbQsxywodU7C0SZWNeW/S5k MsPjEWUqjaYfg== Date: Tue, 16 Jan 2024 08:35:09 -0600 From: Rob Herring To: Pintu Agarwal Cc: vichy.kuo@gmail.com, Pintu Kumar , linux-kernel@vger.kernel.org, akpm@linux-foundation.org, linux-mm@kvack.org, frowand.list@gmail.com, devicetree@vger.kernel.org Subject: Re: [PATCH] of: reserved_mem: fix error log for reserved mem init failure Message-ID: <20240116143509.GA3845101-robh@kernel.org> References: <20231206151600.26833-1-quic_pintu@quicinc.com> <20231208203128.GA2646347-robh@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 44ECE180007 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 18r6n39uk9dkks53pdzofi3mrtdz4o61 X-HE-Tag: 1705415714-197366 X-HE-Meta: U2FsdGVkX19YNMZbNJNPAvEPX5C4IjQORJD2KIAeun2pxfoDN2L0Ho+BUkVCRWgSTTsNBZkmeOioiZmJlgCtTtYM2IrTh4INTcgPqGKzN9hymxpodkpXq/RUlxxovTzzAfE0Ze8n7Ouk7zVGB3vDBBfyxaLAuh/dB3TgQCT8yeyfWsrwRQfjVEt1zRGzWDa21ogcTtP+sXEAz8uAHndglyMCR5b8LTB6jhaCuwj/BJ8ghx+48rwsy3aDxN5y1W45Mel219aNWxN7+qpisZ4mOx4mYfFd3e1Czv6TL9M2cDk8ItsTZ1oYuqR5Pmy5eORz4pY6vKPRPUfTR23Z8RoQXwIeHcOb0DGZ0ksvKPgpy5lfN/vVsA+3zLaSvJBtMoYIUCzwHKMzFsXdhj/qfRqS/KVyypKIAC6aJ+jD9i1g+F2UQ28cpb5bkgto0ORHARYYtK4bo3rLefMxjAh+Zuu2/T/yzTPXyEKargtdscSoiU9ua/KCiYzpZ47mldrHv7Q4uVg4HKHBZvIrUdi7NfRHkJ50sxIJrxg1Pxyi3iTe87nLqTHdjaNF8ekyqKOH8Ziuh5Vvzy7B9DFfxXI+nXssNMEfy3G96uUWGIx4+l9zFrXsTYbXId9krpq6CmvSv2LC1J6A2GWHdlGAk83UHSRZggmiVs8nCf1lXnlSxUzFFvp6AKjmmdsCiHprr9kSzjvWonXOoXrkBB2dlCtwzvwsOlC0uC+0Y7obP86TNepg652wrivr7IZyAZs0GohgMaJtNXma4Jk5c6VORx6d15PCqBXUdAcpLdeuQ/eBF1gYOZNrDoW1OKBaO//KpnVZwuyC0EdKQThbV6eYG8iRIffFWTNEdxXq27r0HyBUvHRRbHSiDA8eEL/Vh/Ux1E60hgv6W4dJxV17zyrTqGr+hBxWtNHXkqRowbrj7OSiowADXK/MaoXgcc+9pkbRsnq95jqSGyuH/4+xAT9Bwx/A9X8 5IQu2KAK hcyQgF7fNTOJTyx6lZC+/L9cWkD9VaKQECRvCF9XxtXlkte3oqGQAhi4R/fZAhLS/OEiPCI3D9wFqnbEEWNLVc7ndYvPvjTFlIAn+93C0aQ4Bso6UC2AydiPF/vt/LOSW7WDp/Cbpz2aHCQsl6ArkQWbkbXSbSsQEq3QiGWSY1QvpvgHulUinIQ2vwtqqVuFDO477sGzpLvITJPE6UbXnt4FOGIjSmK0sL7KNx12qrA3hjz1bdq1aGffY+WBZPjeJN75+vDmtPSOjMIMIlDatkyVVL/WZDM2pdq44OXCXjCiundlv7rfah1p2yaKUPdXz6w6pU4ibS2M/LlqpcYcYcnjx/fcecZZ1aLaQPMQuM6I+QDJie3720PxiVit9Q1UYBxEL8XTe+o/aOL8KCdDdAO5ogRx/oIxAi2e2sGPBQcFgx3n7HHD2KbvEyyox5WYXubcr X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sat, Jan 06, 2024 at 11:31:12PM +0530, Pintu Agarwal wrote: > Hi, > > On Thu, 14 Dec 2023 at 22:47, Pintu Agarwal wrote: > > > > On Mon, 11 Dec 2023 at 20:13, Pintu Agarwal wrote: > > > > > > Hi, > > > > > > On Sat, 9 Dec 2023 at 02:01, Rob Herring wrote: > > > > > > > > On Wed, Dec 06, 2023 at 08:46:00PM +0530, Pintu Kumar wrote: > > > > > During fdt_init_reserved_mem() when __reserved_mem_init_node() > > > > > fail we are using pr_info to print error. > > > > > > > > > > So, if we change the loglevel to 4 (or below), this error > > > > > message will be missed. > > > > > > > > > > Thus, change the pr_info to pr_err for fail case. > > > > > > > > > > Signed-off-by: Pintu Kumar > > > > > --- > > > > > drivers/of/of_reserved_mem.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > > > > > index 7ec94cfcbddb..473665e76b6f 100644 > > > > > --- a/drivers/of/of_reserved_mem.c > > > > > +++ b/drivers/of/of_reserved_mem.c > > > > > @@ -334,7 +334,7 @@ void __init fdt_init_reserved_mem(void) > > > > > if (err == 0) { > > > > > err = __reserved_mem_init_node(rmem); > > > > > if (err != 0 && err != -ENOENT) { > > > > > - pr_info("node %s compatible matching fail\n", > > > > > + pr_err("node %s compatible matching fail\n", > > > > > > > > Isn't the message just wrong. If compatible match fails, we return > > > > ENOENT. The failure here would be from the init function. > > > > > > > Okay. > > > You mean to say, if __reserved_mem_init_node fails with default err > > > (ENOENT) then it may not hit this condition. > > > Instead it will hit the 'else' case which is wrong ? > > > Also, the "initfn" inside "__reserved_mem_init_node" may fail in which > > > case also it may return default err. > > > > > > Maybe, the initial author's intention was to free the memory only if > > > the failure type is not the default ENOENT type. > > > > > > This seems to be a different issue. > > > Can we address this separately in a different patch ? > > > > > > And how do we fix this ? > > > One option is to add another "if" condition with just ENOENT error check ? > > > if (err == -ENOENT) { > > > pr_err("node %s compatible matching fail\n", rmem->name); > > > return; > > > } > > > Then, correct the existing log with a different message: > > > pr_err("node %s matching reserved mem not found.\n", rmem->name); > > > Or, add one more "if else" condition ? > > > Or, fix the calling function itself : __reserved_mem_init_node ? > > > > > > > Any further comments on this ? > > Any further comments or suggestions on the above ? > Shall we just fix the log message, or correct the if/else case as well ? It looked to me like the original author's intent was this is not an error. Either convince me otherwise or wait for me to study this further. This code gets a lot of drive-by patches and what is "correct" isn't always clear. Rob