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 <lei...@debian.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-...@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 <lei...@debian.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;
         }
         

Reply via email to