[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<AS8PR04MB8849B58CD8CC440E7A9F6EAB96722@AS8PR04MB8849.eurprd04.prod.outlook.com>
Date: Fri, 4 Oct 2024 06:31:03 +0000
From: Claudiu Manoil <claudiu.manoil@....com>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>, Rosen Penev
<rosenp@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "andrew@...n.ch"
<andrew@...n.ch>, "davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next 3/6] net: gianfar: allocate queues with devm
> -----Original Message-----
> From: Maxime Chevallier <maxime.chevallier@...tlin.com>
> Sent: Wednesday, October 2, 2024 10:25 AM
[...]
>
> On Tue, 1 Oct 2024 14:22:01 -0700
> Rosen Penev <rosenp@...il.com> wrote:
>
> > There seems to be a mistake here where free_tx_queue is called on
> > failure. Just let devm deal with it.
>
> Good catch, this looks good to me.
>
I like your enthusiasm, but there's nothing to catch here.
kfree() does nothing to NULL objects, however the 'constructor' allocates an
array of objects so free_tx_queues() has to iterate over all objects, to free those
allocated before failure.
I don't have a strong opinion regarding the usage of devm_*() API to allocate resources
at device probing time, it saves some lines of code. However I see this as bringing limited
benefits for simple cases like device probe()/remove(), especially when converting old drivers
like this one. And there's also the risk of falling into the trap of thinking that devm_*() takes
care of everything.
-Claudiu
Powered by blists - more mailing lists