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 smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 063DDC28B20 for ; Wed, 2 Apr 2025 10:33:18 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id A2DE1612AC; Wed, 2 Apr 2025 10:33:18 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id Pdu0L0evT9x6; Wed, 2 Apr 2025 10:33:18 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.166.142; helo=lists1.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 1DF8B608D0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1743589998; bh=eUvCGED7xu+Is2PLjm8dDuOWzRFSd+JT+RSSDf85Q6c=; h=Date:From:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=TDKMwejuA4OrNLO0W+6ysuEXw9TxXVm2tWEwJf3nFhXSTkWTHYbOsPnJHV/ODMiES HxlJFV5iQcCOWjHHsTBFO0tFnoN3q3t3GgLIelJ+9/x9nojOqKGlkEozwd3TFtVU6j jziCkTs1CpDD42E8VsaIq9N1tO02wVAj5YGyzZ7Io8Guwcv/pLuYGZPIVsv+X75ElD HCq98chk/ecKmDLrEK6dsbBk5lmrj4JvS+wcW8kRUvRhVaV2M8HmDieffNESsjwXT+ WlFxcmBMSreP41ito+DFdxisu45ozqeeZmcEcgPeli2bTFtBNpa9QoNVh3jMAcX7JF N61K/kq8Dtdhg== Received: from lists1.osuosl.org (lists1.osuosl.org [140.211.166.142]) by smtp3.osuosl.org (Postfix) with ESMTP id 1DF8B608D0; Wed, 2 Apr 2025 10:33:18 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists1.osuosl.org (Postfix) with ESMTP id 3D1F411F for ; Wed, 2 Apr 2025 10:33:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 2271340C54 for ; Wed, 2 Apr 2025 10:33:17 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id xeZ6goE3hEEt for ; Wed, 2 Apr 2025 10:33:16 +0000 (UTC) Received-SPF: None (mailfrom) identity=mailfrom; client-ip=198.175.65.10; helo=mgamail.intel.com; envelope-from=andriy.shevchenko@linux.intel.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp4.osuosl.org 4977941276 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 4977941276 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by smtp4.osuosl.org (Postfix) with ESMTPS id 4977941276 for ; Wed, 2 Apr 2025 10:33:16 +0000 (UTC) X-CSE-ConnectionGUID: bogLm6yfRVKXWaVjh2oi1Q== X-CSE-MsgGUID: RMFFgB12RiSunYIKuou/2Q== X-IronPort-AV: E=McAfee;i="6700,10204,11391"; a="62346685" X-IronPort-AV: E=Sophos;i="6.14,182,1736841600"; d="scan'208";a="62346685" Received: from orviesa009.jf.intel.com ([10.64.159.149]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2025 03:32:58 -0700 X-CSE-ConnectionGUID: jnKqawtdRCC7ytoiJ+6FvA== X-CSE-MsgGUID: 9Y43AS5zQbiCJvtQWGGu+A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,182,1736841600"; d="scan'208";a="126417413" Received: from smile.fi.intel.com ([10.237.72.58]) by orviesa009.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2025 03:32:55 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.98.2) (envelope-from ) id 1tzvOu-00000008RE3-0xzR; Wed, 02 Apr 2025 13:32:52 +0300 Date: Wed, 2 Apr 2025 13:32:51 +0300 From: Andy Shevchenko To: Przemek Kitszel Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, vbabka@suse.cz, torvalds@linux-foundation.org, peterz@infradead.org, intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org Message-ID: References: <20250401134408.37312-1-przemyslaw.kitszel@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250401134408.37312-1-przemyslaw.kitszel@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1743589997; x=1775125997; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=uzgVRHkC0/6oVwMh4YG7sDY5ZqY1Wc1ZMefrtNkrfxw=; b=ZNR0e/ifFoCRm1YcC0C88pk/IH+Dcn6W0RkU9/gTTAMazpwH9jGK/Y13 wg6swhSoOW0cgtHgTKG/QcIIjBFpD2EWpoN020W1DrKMh1ghAWMeT+Wjb MMYK+CZta4aywY8/Fbizu8kDhAeryyaD4GxWzkOfWpew43RL7t5FxkYFo B0sTQGMTfx+1hEBvbJQCZmxKZK9D7JgoifMQAFb0MM/JHzPMxizko/dON bxwSa9KsmscELSLPtmSEBm4ie5PS706Fl+rmYBANnFDYdzWlPnRi6d8+m RzQcQRHDDIegGuPeh63sROJwWsIaCuEzhuR52dpPlKBlvVvp6K+l/YnWD w==; X-Mailman-Original-Authentication-Results: smtp4.osuosl.org; dmarc=none (p=none dis=none) header.from=linux.intel.com X-Mailman-Original-Authentication-Results: smtp4.osuosl.org; dkim=pass (2048-bit key, unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=ZNR0e/if Subject: Re: [Intel-wired-lan] [RFC] slab: introduce auto_kfree macro X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" On Tue, Apr 01, 2025 at 03:44:08PM +0200, Przemek Kitszel wrote: > Add auto_kfree macro that acts as a higher level wrapper for manual > __free(kfree) invocation, and sets the pointer to NULL - to have both > well defined behavior also for the case code would lack other assignement. > > Consider the following code: > int my_foo(int arg) > { > struct my_dev_foo *foo __free(kfree); /* no assignement */ > > foo = kzalloc(sizeof(*foo), GFP_KERNEL); > /* ... */ > } > > So far it is fine and even optimal in terms of not assigning when > not needed. But it is typical to don't touch (and sadly to don't > think about) code that is not related to the change, so let's consider > an extension to the above, namely an "early return" style to check > arg prior to allocation: > int my_foo(int arg) > { > struct my_dev_foo *foo __free(kfree); /* no assignement */ > + > + if (!arg) > + return -EINVAL; > foo = kzalloc(sizeof(*foo), GFP_KERNEL); > /* ... */ > } > Now we have uninitialized foo passed to kfree, what likely will crash. > One could argue that `= NULL` should be added to this patch, but it is > easy to forgot, especially when the foo declaration is outside of the > default git context. > > With new auto_kfree, we simply will start with > struct my_dev_foo *foo auto_kfree; > and be safe against future extensions. > > I believe this will open up way for broader adoption of Scope Based > Resource Management, say in networking. > I also believe that my proposed name is special enough that it will > be easy to know/spot that the assignement is hidden. I understand the issue and the problem it solves, but... > +#define auto_kfree __free(kfree) = NULL ...I do not like this syntax at all (note, you forgot to show the result in the code how it will look like). What would be better in my opinion is to have it something like DEFINE_*() type, which will look more naturally in the current kernel codebase (as we have tons of DEFINE_FOO(). DEFINE_AUTO_KFREE_VAR(name, struct foo); with equivalent to struct foo *name __free(kfree) = NULL -- With Best Regards, Andy Shevchenko From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3C7511953A1; Wed, 2 Apr 2025 10:33:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.10 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1743589997; cv=none; b=I1u0xVROqYphdZCMOfd0tO/0gqGJi6pKVyCim6JKqnwK+YS/UfahWnryK2MTaEtd0OZt4+b3ow2hZ0Kd7DNy7DUdkKJlBtNdvr1xBVG+Bf8Wo1gg9FvlYeZ86rOjmeQMsw6ZY4aeXcHcYcu0pn9QvXjEFCL0oRTuqVV3wxMxj5g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1743589997; c=relaxed/simple; bh=uzgVRHkC0/6oVwMh4YG7sDY5ZqY1Wc1ZMefrtNkrfxw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tRpKoe3nHwP2V3+327UX466dxRyfir+pR30k/WC4qWgeR9/1+y7wA8pr68A5Nd6TRnZ60L1klF5YvjX84nGagZT0XrjcsUbQxy7w2JNHG79/N+tG+BLuFdRbe2sFRkS+AnXtGAHX3dsaRikM9a5uNg0JT8sedubmOmn8dAYkj+I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=ZNR0e/if; arc=none smtp.client-ip=198.175.65.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="ZNR0e/if" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1743589997; x=1775125997; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=uzgVRHkC0/6oVwMh4YG7sDY5ZqY1Wc1ZMefrtNkrfxw=; b=ZNR0e/ifFoCRm1YcC0C88pk/IH+Dcn6W0RkU9/gTTAMazpwH9jGK/Y13 wg6swhSoOW0cgtHgTKG/QcIIjBFpD2EWpoN020W1DrKMh1ghAWMeT+Wjb MMYK+CZta4aywY8/Fbizu8kDhAeryyaD4GxWzkOfWpew43RL7t5FxkYFo B0sTQGMTfx+1hEBvbJQCZmxKZK9D7JgoifMQAFb0MM/JHzPMxizko/dON bxwSa9KsmscELSLPtmSEBm4ie5PS706Fl+rmYBANnFDYdzWlPnRi6d8+m RzQcQRHDDIegGuPeh63sROJwWsIaCuEzhuR52dpPlKBlvVvp6K+l/YnWD w==; X-CSE-ConnectionGUID: PN7LF4jiRB+P/HYH9m1x0g== X-CSE-MsgGUID: FvGcDMdmQp+UOCz8CnonVQ== X-IronPort-AV: E=McAfee;i="6700,10204,11391"; a="62346681" X-IronPort-AV: E=Sophos;i="6.14,182,1736841600"; d="scan'208";a="62346681" Received: from orviesa009.jf.intel.com ([10.64.159.149]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2025 03:32:58 -0700 X-CSE-ConnectionGUID: jnKqawtdRCC7ytoiJ+6FvA== X-CSE-MsgGUID: 9Y43AS5zQbiCJvtQWGGu+A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,182,1736841600"; d="scan'208";a="126417413" Received: from smile.fi.intel.com ([10.237.72.58]) by orviesa009.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Apr 2025 03:32:55 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.98.2) (envelope-from ) id 1tzvOu-00000008RE3-0xzR; Wed, 02 Apr 2025 13:32:52 +0300 Date: Wed, 2 Apr 2025 13:32:51 +0300 From: Andy Shevchenko To: Przemek Kitszel Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, vbabka@suse.cz, torvalds@linux-foundation.org, peterz@infradead.org, intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org Subject: Re: [RFC] slab: introduce auto_kfree macro Message-ID: References: <20250401134408.37312-1-przemyslaw.kitszel@intel.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250401134408.37312-1-przemyslaw.kitszel@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo On Tue, Apr 01, 2025 at 03:44:08PM +0200, Przemek Kitszel wrote: > Add auto_kfree macro that acts as a higher level wrapper for manual > __free(kfree) invocation, and sets the pointer to NULL - to have both > well defined behavior also for the case code would lack other assignement. > > Consider the following code: > int my_foo(int arg) > { > struct my_dev_foo *foo __free(kfree); /* no assignement */ > > foo = kzalloc(sizeof(*foo), GFP_KERNEL); > /* ... */ > } > > So far it is fine and even optimal in terms of not assigning when > not needed. But it is typical to don't touch (and sadly to don't > think about) code that is not related to the change, so let's consider > an extension to the above, namely an "early return" style to check > arg prior to allocation: > int my_foo(int arg) > { > struct my_dev_foo *foo __free(kfree); /* no assignement */ > + > + if (!arg) > + return -EINVAL; > foo = kzalloc(sizeof(*foo), GFP_KERNEL); > /* ... */ > } > Now we have uninitialized foo passed to kfree, what likely will crash. > One could argue that `= NULL` should be added to this patch, but it is > easy to forgot, especially when the foo declaration is outside of the > default git context. > > With new auto_kfree, we simply will start with > struct my_dev_foo *foo auto_kfree; > and be safe against future extensions. > > I believe this will open up way for broader adoption of Scope Based > Resource Management, say in networking. > I also believe that my proposed name is special enough that it will > be easy to know/spot that the assignement is hidden. I understand the issue and the problem it solves, but... > +#define auto_kfree __free(kfree) = NULL ...I do not like this syntax at all (note, you forgot to show the result in the code how it will look like). What would be better in my opinion is to have it something like DEFINE_*() type, which will look more naturally in the current kernel codebase (as we have tons of DEFINE_FOO(). DEFINE_AUTO_KFREE_VAR(name, struct foo); with equivalent to struct foo *name __free(kfree) = NULL -- With Best Regards, Andy Shevchenko