[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<AS8PR04MB8849FBA73A50F0D553F1AF1B96DE2@AS8PR04MB8849.eurprd04.prod.outlook.com>
Date: Tue, 18 Mar 2025 08:08:24 +0000
From: Claudiu Manoil <claudiu.manoil@....com>
To: Vladimir Oltean <vladimir.oltean@....com>, Wei Fang <wei.fang@....com>
CC: Clark Wang <xiaoning.wang@....com>, "andrew+netdev@...n.ch"
<andrew+netdev@...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>,
"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "linuxppc-dev@...ts.ozlabs.org"
<linuxppc-dev@...ts.ozlabs.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v4 net-next 04/14] net: enetc: add MAC filter for i.MX95
ENETC PF
> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@....com>
> Sent: Monday, March 17, 2025 4:18 PM
[...]
> Subject: Re: [PATCH v4 net-next 04/14] net: enetc: add MAC filter for i.MX95
> ENETC PF
>
> On Tue, Mar 11, 2025 at 01:38:20PM +0800, Wei Fang wrote:
[...]
> > +static void enetc4_pf_destroy_mac_list(struct enetc_pf *pf)
> > +{
> > + struct enetc_mac_list_entry *entry;
> > + struct hlist_node *tmp;
> > +
> > + mutex_lock(&pf->mac_list_lock);
>
> The mutex_lock() usage here should raise serious questions. This is
> running right before mutex_destroy(). So if there were any concurrent
> attempt to acquire this lock, that concurrent code would have been broken
> any time it would have lost arbitration, by the fact that it would
> attempt to acquire a destroyed mutex.
>
> But there's no such concurrent thread, because we run after destroy_workqueue()
> which flushes those concurrent calls and prevents new ones. So the mutex
> usage here is not necessary.
>
> [ same thing with mutex_init() immediately followed by mutex_lock().
> It is an incorrect pattern most of the time. ]
>
This is not as bad as it seems. In the final version of the code, mutex 'mac_list_lock'
serializes the access btw the thread programming the filter for the PF instance,
and the threads programming the filter on behalf of underlying VFs (triggered by async
request from VF). But since VF support is not included in this patch set (as Wei
already mentioned) the lock can/should be added later, with the VF patches.
Powered by blists - more mailing lists