From: Saeed Mahameed <saeedm@mellanox.com>
To: "davem@davemloft.net" <davem@davemloft.net>,
"arnd@arndb.de" <arnd@arndb.de>,
"leon@kernel.org" <leon@kernel.org>
Cc: "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
Or Gerlitz <ogerlitz@mellanox.com>, Oz Shlomo <ozsh@mellanox.com>,
Paul Blakey <paulb@mellanox.com>, Mark Bloch <markb@mellanox.com>,
Maor Gottlieb <maorg@mellanox.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Eli Britstein <elibr@mellanox.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH] net/mlx5e: reduce stack usage in mlx5_eswitch_termtbl_create
Date: Mon, 17 Jun 2019 18:02:39 +0000 [thread overview]
Message-ID: <9efb76f79369b8577ef425c7f6e694132719353e.camel@mellanox.com> (raw)
In-Reply-To: <20190617110855.2085326-1-arnd@arndb.de>
On Mon, 2019-06-17 at 13:08 +0200, Arnd Bergmann wrote:
> Putting an empty 'mlx5_flow_spec' structure on the stack is a bit
> wasteful and causes a warning on 32-bit architectures when building
> with clang -fsanitize-coverage:
>
> drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c:
> In function 'mlx5_eswitch_termtbl_create':
> drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c:90
> :1: error: the frame size of 1032 bytes is larger than 1024 bytes [-
> Werror=frame-larger-than=]
>
> Since the structure is never written to, we can statically allocate
> it to avoid the stack usage. To be on the safe side, mark all
> subsequent function arguments that we pass it into as 'const'
> as well.
>
> Fixes: 10caabdaad5a ("net/mlx5e: Use termination table for VLAN push
> actions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> .../mlx5/core/eswitch_offloads_termtbl.c | 2 +-
> .../net/ethernet/mellanox/mlx5/core/fs_core.c | 20 +++++++++------
> ----
> include/linux/mlx5/fs.h | 2 +-
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git
> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
> index cb7d8ebe2c95..171f3d4ef9ac 100644
> ---
> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
> +++
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
> @@ -50,7 +50,7 @@ mlx5_eswitch_termtbl_create(struct mlx5_core_dev
> *dev,
> struct mlx5_flow_act *flow_act)
> {
> struct mlx5_flow_namespace *root_ns;
> - struct mlx5_flow_spec spec = {};
> + static const struct mlx5_flow_spec spec = {};
LGTM, just make sure please to have a reverse xmas tree here.
Mark, please let me know if you are ok with such API constrain to flow
steering (spec must be const).
Thanks,
Saeed.
> int prio, flags;
> int err;
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index fe76c6fd6d80..739123e1363b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@ -584,7 +584,7 @@ static int insert_fte(struct mlx5_flow_group *fg,
> struct fs_fte *fte)
> }
>
> static struct fs_fte *alloc_fte(struct mlx5_flow_table *ft,
> - u32 *match_value,
> + const u32 *match_value,
> struct mlx5_flow_act *flow_act)
> {
> struct mlx5_flow_steering *steering = get_steering(&ft->node);
> @@ -612,7 +612,7 @@ static void dealloc_flow_group(struct
> mlx5_flow_steering *steering,
>
> static struct mlx5_flow_group *alloc_flow_group(struct
> mlx5_flow_steering *steering,
> u8
> match_criteria_enable,
> - void *match_criteria,
> + const void
> *match_criteria,
> int start_index,
> int end_index)
> {
> @@ -642,7 +642,7 @@ static struct mlx5_flow_group
> *alloc_flow_group(struct mlx5_flow_steering *steer
>
> static struct mlx5_flow_group *alloc_insert_flow_group(struct
> mlx5_flow_table *ft,
> u8
> match_criteria_enable,
> - void
> *match_criteria,
> + const void
> *match_criteria,
> int start_index,
> int end_index,
> struct list_head
> *prev)
> @@ -1285,7 +1285,7 @@ add_rule_fte(struct fs_fte *fte,
> }
>
> static struct mlx5_flow_group *alloc_auto_flow_group(struct
> mlx5_flow_table *ft,
> - struct
> mlx5_flow_spec *spec)
> + const struct
> mlx5_flow_spec *spec)
> {
> struct list_head *prev = &ft->node.children;
> struct mlx5_flow_group *fg;
> @@ -1451,7 +1451,7 @@ static int check_conflicting_ftes(struct fs_fte
> *fte, const struct mlx5_flow_act
> }
>
> static struct mlx5_flow_handle *add_rule_fg(struct mlx5_flow_group
> *fg,
> - u32 *match_value,
> + const u32 *match_value,
> struct mlx5_flow_act
> *flow_act,
> struct
> mlx5_flow_destination *dest,
> int dest_num,
> @@ -1536,7 +1536,7 @@ static void free_match_list(struct
> match_list_head *head)
>
> static int build_match_list(struct match_list_head *match_head,
> struct mlx5_flow_table *ft,
> - struct mlx5_flow_spec *spec)
> + const struct mlx5_flow_spec *spec)
> {
> struct rhlist_head *tmp, *list;
> struct mlx5_flow_group *g;
> @@ -1589,7 +1589,7 @@ static u64 matched_fgs_get_version(struct
> list_head *match_head)
>
> static struct fs_fte *
> lookup_fte_locked(struct mlx5_flow_group *g,
> - u32 *match_value,
> + const u32 *match_value,
> bool take_write)
> {
> struct fs_fte *fte_tmp;
> @@ -1622,7 +1622,7 @@ lookup_fte_locked(struct mlx5_flow_group *g,
> static struct mlx5_flow_handle *
> try_add_to_existing_fg(struct mlx5_flow_table *ft,
> struct list_head *match_head,
> - struct mlx5_flow_spec *spec,
> + const struct mlx5_flow_spec *spec,
> struct mlx5_flow_act *flow_act,
> struct mlx5_flow_destination *dest,
> int dest_num,
> @@ -1715,7 +1715,7 @@ try_add_to_existing_fg(struct mlx5_flow_table
> *ft,
>
> static struct mlx5_flow_handle *
> _mlx5_add_flow_rules(struct mlx5_flow_table *ft,
> - struct mlx5_flow_spec *spec,
> + const struct mlx5_flow_spec *spec,
> struct mlx5_flow_act *flow_act,
> struct mlx5_flow_destination *dest,
> int dest_num)
> @@ -1823,7 +1823,7 @@ static bool fwd_next_prio_supported(struct
> mlx5_flow_table *ft)
>
> struct mlx5_flow_handle *
> mlx5_add_flow_rules(struct mlx5_flow_table *ft,
> - struct mlx5_flow_spec *spec,
> + const struct mlx5_flow_spec *spec,
> struct mlx5_flow_act *flow_act,
> struct mlx5_flow_destination *dest,
> int num_dest)
> diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
> index 2ddaa97f2179..c0c029664527 100644
> --- a/include/linux/mlx5/fs.h
> +++ b/include/linux/mlx5/fs.h
> @@ -200,7 +200,7 @@ struct mlx5_flow_act {
> */
> struct mlx5_flow_handle *
> mlx5_add_flow_rules(struct mlx5_flow_table *ft,
> - struct mlx5_flow_spec *spec,
> + const struct mlx5_flow_spec *spec,
> struct mlx5_flow_act *flow_act,
> struct mlx5_flow_destination *dest,
> int num_dest);
next prev parent reply other threads:[~2019-06-17 18:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-17 11:08 [PATCH] net/mlx5e: reduce stack usage in mlx5_eswitch_termtbl_create Arnd Bergmann
2019-06-17 18:02 ` Saeed Mahameed [this message]
2019-06-17 18:24 ` Mark Bloch
2019-06-17 21:02 ` David Miller
2019-06-17 21:30 ` Saeed Mahameed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9efb76f79369b8577ef425c7f6e694132719353e.camel@mellanox.com \
--to=saeedm@mellanox.com \
--cc=arnd@arndb.de \
--cc=davem@davemloft.net \
--cc=elibr@mellanox.com \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=maorg@mellanox.com \
--cc=markb@mellanox.com \
--cc=netdev@vger.kernel.org \
--cc=ogerlitz@mellanox.com \
--cc=ozsh@mellanox.com \
--cc=paulb@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).