Message ID | 20230804164932.40582-1-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6358:1a9a:b0:139:fa0d:b2d with SMTP id gm26csp96263rwb; Fri, 4 Aug 2023 10:38:11 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEM4uFDg0vYnDvw5FvZa4PG+avVpYiPBUCHYh7qou4392fJpB+SB0l/W/qJ7Y0kREW60Z4S X-Received: by 2002:a17:902:ce91:b0:1bb:e74b:39ff with SMTP id f17-20020a170902ce9100b001bbe74b39ffmr2653635plg.0.1691170690768; Fri, 04 Aug 2023 10:38:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691170690; cv=none; d=google.com; s=arc-20160816; b=yup9w8Usk2KeAT/ECrtD7qtwaFE+Qa9KPcqwwcoQE9nWyy3TGrNLHRyFjBOiijiuEQ t4k9mvtBDPrRqKW7eTx5eDyj3eAYyd5Zko6i+BBSM7LERFmiRiGEaGMbqlOVCWWqeE5s d+f2aE/72RaI9ucAtViVqinXAnDt5UWdnMghRfC+d3A0UdJphS92dHjERlSr33RYgJ/y 4PMk9LRpn5E9jV2jEzzJ2LWCq/xPbOrUGj0+B/JFvBJRuII6MdEclTOiSDxYrTIH8tCj uiZrkIe6m1KpyEvtaBuymzvvPjnv4H6i4/R6VnErbeFIMM71lXkFjoxCSW86tT8qD8gt miug== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=g98+d9zBbQZpws/oA60Sq1MrsETovu7XVq9BVM/S+t4=; fh=sfuNpiqpmJ923uOdgBV6mUfT1tNLeE/0kVq5Y7UOacM=; b=p9PK4u044+X45YGSE1AYVbhVf4kIjJKJsU21J0Xnfjo3ZxMmLJGFqDnmWNLtYfiA+V qRLyE87zU4J7HGZbVEwrpjW1aXNa+qBlK+PPuzdMlQNCviS3F67nRRg5xaetwsHl+rtH EXpqW2LfQ7+57yDq46TfQglswu0G/ehqbIEyz1Kxj8tNNAS8NBI4CZerYobmtXOsXD8Q ubuHSLEI+Pi0TJnHedCoLCF/mYTzKNeywr1wiyC5RdjLd5L6oun2GYEhfXJkMtfUGqk1 sDPsGlzQMTX4ljekZxiGSVsh96V5HMoxIJ8EtcUzLJeDy8zuFOn/mX8SHz+KXdYISd86 YJ/A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=ceU8loo2; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id j15-20020a170903024f00b001b5395382a0si2132465plh.212.2023.08.04.10.37.31; Fri, 04 Aug 2023 10:38:10 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=ceU8loo2; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229848AbjHDQtZ (ORCPT <rfc822;sukrut.bellary@gmail.com> + 99 others); Fri, 4 Aug 2023 12:49:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55608 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230495AbjHDQtY (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 4 Aug 2023 12:49:24 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4C98749E1 for <linux-kernel@vger.kernel.org>; Fri, 4 Aug 2023 09:49:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1691167761; x=1722703761; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=aahh5Swgdsj9tjDJPMypieJ+4SsMPQB76IH8IvwJU4w=; b=ceU8loo2zlPBQYTloD0nKwXLEsXjR0+Vrviw9/hZ0XDULg2qptG7JdEY iwMQSSybxB3UEllcbqgZEW+WRD9+QlGP7/K/kw7wH61u+480rIhmYA9O6 cJFsKFtiPcB/8xsPOx58IelQk/lnuBht82SM6mVk/aH+vk0N3QENe0xYe zrz74ybIqsNQ3S7lAdb/yehyWzAafT12iH2AEHCGVYSN8JmUMDkUHjUMv f7JY/BFU4cw6TtpgxTxnlcQAPVHn5jcxocHTQfHvrhZartD5JybPRbrnX S5HfrnS5ZXBfrd/i0hUw++rY0tV2zSKDYdjaE/v0uMyT63WYDHQNliiiJ w==; X-IronPort-AV: E=McAfee;i="6600,9927,10792"; a="350505882" X-IronPort-AV: E=Sophos;i="6.01,255,1684825200"; d="scan'208";a="350505882" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Aug 2023 09:49:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10792"; a="903943911" X-IronPort-AV: E=Sophos;i="6.01,255,1684825200"; d="scan'208";a="903943911" Received: from black.fi.intel.com ([10.237.72.28]) by orsmga005.jf.intel.com with ESMTP; 04 Aug 2023 09:49:18 -0700 Received: by black.fi.intel.com (Postfix, from userid 1003) id 104BE45B; Fri, 4 Aug 2023 19:49:33 +0300 (EEST) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Marc Zyngier <maz@kernel.org>, Johan Hovold <johan+linaro@kernel.org>, linux-kernel@vger.kernel.org Cc: Thomas Gleixner <tglx@linutronix.de>, Andy Shevchenko <andriy.shevchenko@linux.intel.com> Subject: [PATCH v1 1/1] irqdomain: Refactor error path in __irq_domain_alloc_fwnode() Date: Fri, 4 Aug 2023 19:49:32 +0300 Message-Id: <20230804164932.40582-1-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.40.0.1.gaa8946217a0b MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773320998469592244 X-GMAIL-MSGID: 1773320998469592244 |
Series |
[v1,1/1] irqdomain: Refactor error path in __irq_domain_alloc_fwnode()
|
|
Commit Message
Andy Shevchenko
Aug. 4, 2023, 4:49 p.m. UTC
First of all, there is no need to call kasprintf() if the previous
allocation failed. Second, there is no need to call for kfree()
when we know that its parameter is NULL. Refactor the code accordingly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
kernel/irq/irqdomain.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Comments
On 2023-08-04 17:49, Andy Shevchenko wrote: > First of all, there is no need to call kasprintf() if the previous > allocation failed. Second, there is no need to call for kfree() > when we know that its parameter is NULL. Refactor the code accordingly. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > kernel/irq/irqdomain.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index 0bdef4fe925b..d2bbba46c808 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -81,6 +81,8 @@ struct fwnode_handle > *__irq_domain_alloc_fwnode(unsigned int type, int id, > char *n; > > fwid = kzalloc(sizeof(*fwid), GFP_KERNEL); > + if (!fwid) > + return NULL; > > switch (type) { > case IRQCHIP_FWNODE_NAMED: > @@ -93,10 +95,8 @@ struct fwnode_handle > *__irq_domain_alloc_fwnode(unsigned int type, int id, > n = kasprintf(GFP_KERNEL, "irqchip@%pa", pa); > break; > } > - > - if (!fwid || !n) { > + if (!n) { > kfree(fwid); > - kfree(n); > return NULL; > } What are you trying to fix? We have a common error handling path, which makes it easy to track the memory management. I don't think this sort of bike shedding adds much to the maintainability of this code. Now if you have spotted an actual bug, I'm all ears. M.
On Fri, Aug 04, 2023 at 06:33:07PM +0100, Marc Zyngier wrote: > On 2023-08-04 17:49, Andy Shevchenko wrote: > > First of all, there is no need to call kasprintf() if the previous > > allocation failed. Second, there is no need to call for kfree() > > when we know that its parameter is NULL. Refactor the code accordingly. ... > > n = kasprintf(GFP_KERNEL, "irqchip@%pa", pa); > > break; > > } > > - > > - if (!fwid || !n) { > > + if (!n) { > > kfree(fwid); > > - kfree(n); > > return NULL; > > } > > What are you trying to fix? I'm not trying to fix anything (there is no such statement from me), but I would think of some micro-optimization (speedup boot for unnoticeable time? Dunno.). > We have a common error handling path, which makes it easy to > track the memory management. I don't think this sort of bike > shedding adds much to the maintainability of this code. Your call, of course, but I not often see in the kernel two or three attempts to allocate some memory and have grouped check for the failure. > Now if you have spotted an actual bug, I'm all ears.
On Fri, 04 Aug 2023 21:12:11 +0100, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Aug 04, 2023 at 06:33:07PM +0100, Marc Zyngier wrote: > > On 2023-08-04 17:49, Andy Shevchenko wrote: > > > First of all, there is no need to call kasprintf() if the previous > > > allocation failed. Second, there is no need to call for kfree() > > > when we know that its parameter is NULL. Refactor the code accordingly. > > ... > > > > n = kasprintf(GFP_KERNEL, "irqchip@%pa", pa); > > > break; > > > } > > > - > > > - if (!fwid || !n) { > > > + if (!n) { > > > kfree(fwid); > > > - kfree(n); > > > return NULL; > > > } > > > > What are you trying to fix? > > I'm not trying to fix anything (there is no such statement from me), > but I would think of some micro-optimization (speedup boot for > unnoticeable time? Dunno.). Error handling paths rarely qualify as an optimisation. > > > We have a common error handling path, which makes it easy to > > track the memory management. I don't think this sort of bike > > shedding adds much to the maintainability of this code. > > Your call, of course, but I not often see in the kernel two or three attempts > to allocate some memory and have grouped check for the failure. Things like this[1]? Well, this is a pattern I use often enough. Maybe it isn't everybody's taste, but it suits me. M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-gic-v3-its.c#n3438
On Fri, Aug 04, 2023 at 11:24:02PM +0100, Marc Zyngier wrote: > On Fri, 04 Aug 2023 21:12:11 +0100, > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Aug 04, 2023 at 06:33:07PM +0100, Marc Zyngier wrote: > > > On 2023-08-04 17:49, Andy Shevchenko wrote: ... > > > > n = kasprintf(GFP_KERNEL, "irqchip@%pa", pa); > > > > break; > > > > } > > > > - > > > > - if (!fwid || !n) { > > > > + if (!n) { > > > > kfree(fwid); > > > > - kfree(n); > > > > return NULL; > > > > } > > > > > > What are you trying to fix? > > > > I'm not trying to fix anything (there is no such statement from me), > > but I would think of some micro-optimization (speedup boot for > > unnoticeable time? Dunno.). > > Error handling paths rarely qualify as an optimisation. OK. ... > > > We have a common error handling path, which makes it easy to > > > track the memory management. I don't think this sort of bike > > > shedding adds much to the maintainability of this code. > > > > Your call, of course, but I not often see in the kernel two or three attempts > > to allocate some memory and have grouped check for the failure. > > Things like this[1]? Yes. > Well, this is a pattern I use often enough. Maybe > it isn't everybody's taste, but it suits me. Understand. Thanks for review! > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-gic-v3-its.c#n3438
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index 0bdef4fe925b..d2bbba46c808 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -81,6 +81,8 @@ struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id, char *n; fwid = kzalloc(sizeof(*fwid), GFP_KERNEL); + if (!fwid) + return NULL; switch (type) { case IRQCHIP_FWNODE_NAMED: @@ -93,10 +95,8 @@ struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id, n = kasprintf(GFP_KERNEL, "irqchip@%pa", pa); break; } - - if (!fwid || !n) { + if (!n) { kfree(fwid); - kfree(n); return NULL; }