netdev - Re: [PATCH v2 net-next] net: fec: Convert fec driver to use lock guards

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <657b4098-60b8-4522-8ea0-f10aa338e1b6@web.de>
Date: Sun, 23 Jun 2024 13:01:08 +0200
From: Markus Elfring <Markus.Elfring@....de>
To: Wei Fang <wei.fang@....com>, netdev@...r.kernel.org,
 kernel-janitors@...r.kernel.org, imx@...ts.linux.dev,
 Andrew Lunn <andrew@...n.ch>, Clark Wang <xiaoning.wang@....com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Richard Cochran <richardcochran@...il.com>,
 Shenwei Wang <shenwei.wang@....com>
Cc: Julia Lawall <julia.lawall@...ia.fr>,
 Peter Zijlstra <peterz@...radead.org>, Simon Horman <horms@...nel.org>,
 Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v2 net-next] net: fec: Convert fec driver to use lock
 guards

> The Scope-based resource management mechanism has been introduced into
…
      scope?                                    was?

…
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -99,18 +99,17 @@
>   */
>  static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
>  {
> -	unsigned long flags;
>  	u32 val, tempval;
>  	struct timespec64 ts;
>  	u64 ns;
>
> -	if (fep->pps_enable == enable)
> -		return 0;
> -
>  	fep->pps_channel = DEFAULT_PPS_CHANNEL;
>  	fep->reload_period = PPS_OUPUT_RELOAD_PERIOD;
>
> -	spin_lock_irqsave(&fep->tmreg_lock, flags);
> +	guard(spinlock_irqsave)(&fep->tmreg_lock);
> +
> +	if (fep->pps_enable == enable)
> +		return 0;
>
>  	if (enable) {
>  		/* clear capture or output compare interrupt status if have.
…

Was this source code adjustment influenced also by a hint about “LOCK EVASION”
from the analysis tool “Coverity”?
https://lore.kernel.org/linux-kernel/AM0PR0402MB38910DB23A6DABF1C074EF1D88E52@AM0PR0402MB3891.eurprd04.prod.outlook.com/
https://lkml.org/lkml/2024/5/8/77

Will any tags (like “Fixes” and “Cc”) become relevant here?

How do you think about to take the known advice “Solve only one problem per patch”
better into account?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc4#n81

Under which circumstances will development interests grow for further approaches
according to the presentation of similar change combinations?

Regards,
Markus