Received-SPF: pass (google.com: domain of linux-kernel+bounces-117758-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 25 Mar 2024 12:00:49 -0700
From: Dan Williams <dan.j.williams@intel.com>
To: Przemek Kitszel <przemyslaw.kitszel@intel.com>, <jiri@resnulli.us>
CC: <42.hyeyoo@gmail.com>, <akpm@linux-foundation.org>, <cl@linux.com>,
	<danielj@nvidia.com>, <iamjoonsoo.kim@lge.com>,
	<linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>, <mingo@kernel.org>,
	<penberg@kernel.org>, <peterz@infradead.org>, <rientjes@google.com>,
	<roman.gushchin@linux.dev>, <vbabka@suse.cz>
Subject: Re: Re [patch RFC] mm/slab: introduce KZALLOC_FREE() cleanup-ed
 allocation macro
Message-ID: <6601c9e161df3_2690d294e5@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240315132249.2515468-1-jiri@resnulli.us>
 <20240321162648.23693-1-przemyslaw.kitszel@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240321162648.23693-1-przemyslaw.kitszel@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?k32hxxinQV9zeRTir7+hGmmbdQB4VOyC4ZByMl8q9fEz9RUEQwOT+/NrA6s0?=
 =?us-ascii?Q?ZUaOzQ1HEQIodRegXeo9P6/AMKk/iZMnvCJRic65mTnHceCewMVNAkLaWSt+?=
 =?us-ascii?Q?oO0pB8vqvvcS6YU9SRso10+AIJCHX7HqBpHwXme2ge0x1xGDEMw2XrPLflyn?=
 =?us-ascii?Q?cDVkWQIb2rj+4IJ3dbm4zFFbDS8dpPQJuJ5Xs10/GwGVXJNW8MKyFNDeSNxi?=
 =?us-ascii?Q?QCptwPtV9MDg3do4oFaEQNK3tmTOyyNRsO0nTufusAhSd31E44rhrXajl7pY?=
 =?us-ascii?Q?MgB9wzz3GF4MKfwrNbYpL7VbUQPt8vPYDJnKHu9FQibIFpCBDZpFokwVgKV6?=
 =?us-ascii?Q?vS5X2SER0nYMzw767OFy/GZhDjvtNfGW3X+Uvx27hj5DLpFsnwzHEzyD4u8n?=
 =?us-ascii?Q?GTtQN07cTdiCfMZnchbmEUe/Yu0iPN0FGpD0s+zftW81xJE4yv3qFW+2TFMj?=
 =?us-ascii?Q?mHOIpHoRqAfDiDIVeYWM1pZayMr4iq/dEUaXbP0fwONlhlnKxKdYNlGTOoC8?=
 =?us-ascii?Q?FTFdkLeOurBcMrR7/sgQXyZJPkYHR1MrTzwqhWSUXNAMenG7d4d27vgBLWRO?=
 =?us-ascii?Q?Qe8H9WGMpoAbRhXdr+TIp6mDnjt0OVozjmZ59xpysnN+Rq4JVIugHSncP0a8?=
 =?us-ascii?Q?kyhC/8cT3CBsDOR5oGgalnyzHfhYlmRyG7o0ExgcLWWhgeoxpmp/N/fuzK8g?=
 =?us-ascii?Q?B4laat3xWPZ+a3q6aTAnyTFe2gWd4bDLft6DsqgvZ6/L2pd4m9ezhHRjKuR0?=
 =?us-ascii?Q?RBxRpY3G9ztc5vY774ltPkgucX01sOgbV9EbNC9VMbkLo7650qse14IjIVxJ?=
 =?us-ascii?Q?p3s264Bl09BiPJIju0iZZWs22DGI1sXDKNZMBHgbPljHrgAfIXngCrFsXbRU?=
 =?us-ascii?Q?q+P/UaUoXAsczJe5QlkD+46OGEIJITJQTGMODnU1M4EyrepwB/+o/WqZeOKz?=
 =?us-ascii?Q?3D994P9myVwuIjZArI8Qox1hvNjB0oA77bDby4SrwhmTfg88/UQ7XXd956DN?=
 =?us-ascii?Q?ViXUFwzf54gZXxP+sy5NrTgMsKEE/U5c+Vv/0fnrHizcC0gfFiSncKUcUhb7?=
 =?us-ascii?Q?ZDV5du5fSxKgY7tP0iZhCFUtc2bDkULY4nMnqvdBGpa9brsxXEcXx8Um4RO4?=
 =?us-ascii?Q?52T4iLct6R9ODT8lQvhYnl0Q0q1f9X1l/75Ws4te66i7z0iKpJ6309xebHHF?=
 =?us-ascii?Q?/8s9UbtysDpavz22P4Jb/yJYspAxOw6OsOdBGThPKVylUAHPSMjBi0Juu1Hq?=
 =?us-ascii?Q?Pb7MQtyAbh5lgd/HcCxQnRJsWMDLDzA/OsshQRgTqNLz7ZVZDTD4Jai/8vaD?=
 =?us-ascii?Q?PhWYJo7xy24c2BhoKcz2pz3yZKjPT6GJpv7NLOy5nxCwG3F2SlDv+1VmQYPe?=
 =?us-ascii?Q?pCly3mtmxo3PnXEGFQeRV64BeZZ5Oe7fq7fKwKhaIFiBHowqmMaRHD25yS61?=
 =?us-ascii?Q?oIkNlcmvM+JSbHdDLBrfkHz+ZqfRxPqLJuUasGMzb5FKVcNyhLSzq5xzSSS6?=
 =?us-ascii?Q?Igczb9GikyKbjbRlWvSY5yVsbt9kfP/7lWefDmrMvAYmKlMh+M8Aq27YOgqJ?=
 =?us-ascii?Q?jNLMqc/KW4iQWR/w8oQ/f+UsqIT3deRkdf2Kh6FFTjFf6mSS4GTu00a6Buqt?=
 =?us-ascii?Q?ew=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5620f0be-a4ba-4e62-d08a-08dc4cfde451
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Mar 2024 19:00:52.2263
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lBhSXWuYEp75K2vNchKgyu9y9nKndrtD5xqHfYyig+LU+IIvkXkq7F8/5fqAACLARkkyCLKcum8V8kEOLgmkJMtMkSdwW9vRfLb5Q1VX5m8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4855
X-OriginatorOrg: intel.com

Przemek Kitszel wrote:
> > From: Jiri Pirko <jiri@nvidia.com>
> > 
> > With introduction of __free() macro using cleanup infrastructure, it
> > will very likely become quite common to see following pattern:
> > 	type *var __free(kfree) = kzalloc(sizeof(*var), GFP_KERNEL);
> > 
> > To follow the CLASS() flow from cleanup.h, introduce a simple macro
> > KZALLOC_FREE() to wrap this over and allow the same flow.
> > 
> > Show an example usage in gpio-sim driver.
> > 
> > Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> > ---
> >  drivers/gpio/gpio-sim.c | 3 +--
> >  include/linux/slab.h    | 3 +++
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
> > index c4106e37e6db..997237b3d80c 100644
> > --- a/drivers/gpio/gpio-sim.c
> > +++ b/drivers/gpio/gpio-sim.c
> > @@ -1496,8 +1496,7 @@ gpio_sim_config_make_device_group(struct config_group *group, const char *name)
> >  {
> >  	int id;
> >  
> > -	struct gpio_sim_device *dev __free(kfree) = kzalloc(sizeof(*dev),
> > -							    GFP_KERNEL);
> > +	KZALLOC_FREE(struct gpio_sim_device *, dev, GFP_KERNEL);
> >  	if (!dev)
> >  		return ERR_PTR(-ENOMEM);
> >  
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index b5f5ee8308d0..baee6acd58d3 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -711,6 +711,9 @@ static inline __alloc_size(1) void *kzalloc(size_t size, gfp_t flags)
> >  	return kmalloc(size, flags | __GFP_ZERO);
> >  }
> >  
> > +#define KZALLOC_FREE(_type, var, _gfp_t)				\
> > +	_type var __free(kfree) = kzalloc(sizeof(*var), _gfp_t)
> > +
> 
> Nice, but I would rather see this wrapper in the cleanup.h file, that have all
> of the rest of related stuff.
> 
> On top of that, I want to propose also a wrapper that is simpler in that it
> does not allocate but just assigns null, with that in mind `_FREE` part of your
> proposed name does not sound right.

No, do not hide assignments within macros

http://lore.kernel.org/r/CAHk-=whYxkfLVtBW_B-PgNqhKOAThTbfoH5CxtOTkwOB6VOt6w@mail.gmail.com

I.e. the amount of incremenal cleverness that include/linux/cleanup.h
will tolerate is low. Any helper should look like typical C.