netdev - Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE

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: <Zow5FUmOADrqUpM9@gmail.com>
Date: Mon, 8 Jul 2024 12:08:05 -0700
From: Breno Leitao <leitao@...ian.org>
To: Vladimir Oltean <olteanv@...il.com>
Cc: kernel test robot <lkp@...el.com>, linuxppc-dev@...ts.ozlabs.org,
	linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
	llvm@...ts.linux.dev, oe-kbuild-all@...ts.linux.dev,
	kuba@...nel.org, horms@...nel.org, Roy.Pledge@....com,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

Hello Vladimir,

On Mon, Jul 08, 2024 at 04:37:46PM +0300, Vladimir Oltean wrote:
> On Thu, Jun 27, 2024 at 11:40:24AM -0700, Breno Leitao wrote:

> > > >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> > > >          |            ^
> > > >    1 warning generated.
> > > 
> > > Arrays of NR_CPUS elements are what it probably doesn't like?

> > Can it use the number of online CPUs instead of NR_CPUS?

> I don't see how, given that variable length arrays are something which
> should be avoided in the kernel?

I thought about a patch like the following (compile tested only). What
do you think?

	Author: Breno Leitao <leitao@...ian.org>
	Date:   Mon Jul 8 11:57:33 2024 -0700

	    net: dpaa: Allocate only for online CPUs in dpaa_set_coalesce
	    
	    Currently, dpaa_set_coalesce allocates a boolean for every possible CPU
	    (NR_CPUS). This approach is suboptimal and causes failures in COMPILE_TEST.
	    For reference, see:
	    https://lore.kernel.org/all/202406261920.l5pzM1rj-lkp@intel.com/
	    
	    Modify the allocation to consider only online CPUs instead of
	    NR_CPUs. This change reduces the function's memory footprint and resolves
	    the COMPILE_TEST issues.
	    
	    Signed-off-by: Breno Leitao <leitao@...ian.org>

	diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	index 5bd0b36d1feb..7202a5310045 100644
	--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	@@ -457,7 +457,7 @@ static int dpaa_set_coalesce(struct net_device *dev,
				     struct netlink_ext_ack *extack)
	 {
		const cpumask_t *cpus = qman_affine_cpus();
	-	bool needs_revert[NR_CPUS] = {false};
	+	bool *needs_revert;
		struct qman_portal *portal;
		u32 period, prev_period;
		u8 thresh, prev_thresh;
	@@ -466,6 +466,11 @@ static int dpaa_set_coalesce(struct net_device *dev,
		period = c->rx_coalesce_usecs;
		thresh = c->rx_max_coalesced_frames;
	 
	+	needs_revert = kmalloc_array(num_possible_cpus(), sizeof(bool), GFP_KERNEL);
	+	if (!needs_revert)
	+		return -ENOMEM;
	+	memset(needs_revert, 0, num_online_cpus() * sizeof(bool));
	+
		/* save previous values */
		portal = qman_get_affine_portal(smp_processor_id());
		qman_portal_get_iperiod(portal, &prev_period);
	@@ -498,6 +503,7 @@ static int dpaa_set_coalesce(struct net_device *dev,
			qman_dqrr_set_ithresh(portal, prev_thresh);
		}
	 
	+	kfree(needs_revert);
		return res;
	 }