netdev - Re: [PATCH net-next v2 07/12] net: remove the xps possible

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: <CAKgT0Ue1mYiuP1-qAovV4WwUrJ_k2Ug0tB+syzzHRtHeMiz7ww@mail.gmail.com>
Date:   Mon, 8 Feb 2021 13:43:39 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Antoine Tenart <atenart@...nel.org>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 07/12] net: remove the xps possible_mask

On Mon, Feb 8, 2021 at 9:19 AM Antoine Tenart <atenart@...nel.org> wrote:
>
> Remove the xps possible_mask. It was an optimization but we can just
> loop from 0 to nr_ids now that it is embedded in the xps dev_maps. That
> simplifies the code a bit.
>
> Suggested-by: Alexander Duyck <alexander.duyck@...il.com>
> Signed-off-by: Antoine Tenart <atenart@...nel.org>
> ---
>  net/core/dev.c       | 43 ++++++++++++++-----------------------------
>  net/core/net-sysfs.c |  4 ++--
>  2 files changed, 16 insertions(+), 31 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index abbb2ae6b3ed..d0c07ccea2e5 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2505,33 +2505,27 @@ static void reset_xps_maps(struct net_device *dev,
>         kfree_rcu(dev_maps, rcu);
>  }
>
> -static void clean_xps_maps(struct net_device *dev, const unsigned long *mask,
> +static void clean_xps_maps(struct net_device *dev,
>                            struct xps_dev_maps *dev_maps, u16 offset, u16 count,
>                            bool is_rxqs_map)
>  {
> -       unsigned int nr_ids = dev_maps->nr_ids;
>         bool active = false;
>         int i, j;
>
> -       for (j = -1; j = netif_attrmask_next(j, mask, nr_ids), j < nr_ids;)
> -               active |= remove_xps_queue_cpu(dev, dev_maps, j, offset,
> -                                              count);
> +       for (j = 0; j < dev_maps->nr_ids; j++)
> +               active |= remove_xps_queue_cpu(dev, dev_maps, j, offset, count);
>         if (!active)
>                 reset_xps_maps(dev, dev_maps, is_rxqs_map);
>
> -       if (!is_rxqs_map) {
> -               for (i = offset + (count - 1); count--; i--) {
> +       if (!is_rxqs_map)
> +               for (i = offset + (count - 1); count--; i--)
>                         netdev_queue_numa_node_write(
> -                               netdev_get_tx_queue(dev, i),
> -                               NUMA_NO_NODE);
> -               }
> -       }
> +                               netdev_get_tx_queue(dev, i), NUMA_NO_NODE);
>  }
>

This violates the coding-style guide for the kernel. The if statement
should still have braces as the for loop and
netdev_queue_numa_node_write are more than a single statement. I'd be
curious to see if checkpatch also complains about this because it
probably should.

For reference see the end of section 3.0 in
Documentation/process/coding-style.rst.

Other than that the rest of the patch seemed to be fine.