All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ethtool: reduce stack usage with clang
@ 2019-03-07  9:33 Arnd Bergmann
  2019-03-07 10:06 ` Michal Kubecek
  0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2019-03-07  9:33 UTC (permalink / raw)
  To: David S. Miller, Florian Fainelli, Jakub Kicinski, Jiri Pirko
  Cc: Nick Desaulniers, Arnd Bergmann, Kees Cook, Wenwen Wang,
	Ilya Lesokhin, Pablo Neira Ayuso, Edward Cree, Michal Kubecek,
	netdev, linux-kernel

clang inlines the dev_ethtool() more aggressively than gcc does, leading
to a larger amount of used stack space:

net/core/ethtool.c:2536:24: error: stack frame size of 1216 bytes in function 'dev_ethtool' [-Werror,-Wframe-larger-than=]

Marking the sub-functions that require the most stack space as
noinline_for_stack gives us reasonable behavior on all compilers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/core/ethtool.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index d4918ffddda8..fcbed78172a0 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2319,9 +2319,10 @@ static int ethtool_set_tunable(struct net_device *dev, void __user *useraddr)
 	return ret;
 }
 
-static int ethtool_get_per_queue_coalesce(struct net_device *dev,
-					  void __user *useraddr,
-					  struct ethtool_per_queue_op *per_queue_opt)
+static noinline_for_stack int
+ethtool_get_per_queue_coalesce(struct net_device *dev,
+			       void __user *useraddr,
+			       struct ethtool_per_queue_op *per_queue_opt)
 {
 	u32 bit;
 	int ret;
@@ -2349,9 +2350,10 @@ static int ethtool_get_per_queue_coalesce(struct net_device *dev,
 	return 0;
 }
 
-static int ethtool_set_per_queue_coalesce(struct net_device *dev,
-					  void __user *useraddr,
-					  struct ethtool_per_queue_op *per_queue_opt)
+static noinline_for_stack int
+ethtool_set_per_queue_coalesce(struct net_device *dev,
+			       void __user *useraddr,
+			       struct ethtool_per_queue_op *per_queue_opt)
 {
 	u32 bit;
 	int i, ret = 0;
@@ -2405,7 +2407,7 @@ static int ethtool_set_per_queue_coalesce(struct net_device *dev,
 	return ret;
 }
 
-static int ethtool_set_per_queue(struct net_device *dev,
+static int noinline_for_stack ethtool_set_per_queue(struct net_device *dev,
 				 void __user *useraddr, u32 sub_cmd)
 {
 	struct ethtool_per_queue_op per_queue_opt;
@@ -2533,7 +2535,7 @@ static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr)
 
 /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
 
-int dev_ethtool(struct net *net, struct ifreq *ifr)
+int noinline_for_stack dev_ethtool(struct net *net, struct ifreq *ifr)
 {
 	struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
 	void __user *useraddr = ifr->ifr_data;
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ethtool: reduce stack usage with clang
  2019-03-07  9:33 [PATCH] ethtool: reduce stack usage with clang Arnd Bergmann
@ 2019-03-07 10:06 ` Michal Kubecek
  2019-03-07 15:59   ` Arnd Bergmann
  0 siblings, 1 reply; 3+ messages in thread
From: Michal Kubecek @ 2019-03-07 10:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Florian Fainelli, Jakub Kicinski, Jiri Pirko,
	Nick Desaulniers, Kees Cook, Wenwen Wang, Ilya Lesokhin,
	Pablo Neira Ayuso, Edward Cree, netdev, linux-kernel

On Thu, Mar 07, 2019 at 10:33:35AM +0100, Arnd Bergmann wrote:
> clang inlines the dev_ethtool() more aggressively than gcc does, leading
> to a larger amount of used stack space:
> 
> net/core/ethtool.c:2536:24: error: stack frame size of 1216 bytes in function 'dev_ethtool' [-Werror,-Wframe-larger-than=]
> 
> Marking the sub-functions that require the most stack space as
> noinline_for_stack gives us reasonable behavior on all compilers.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  net/core/ethtool.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index d4918ffddda8..fcbed78172a0 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
...
> @@ -2533,7 +2535,7 @@ static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr)
>  
>  /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
>  
> -int dev_ethtool(struct net *net, struct ifreq *ifr)
> +int noinline_for_stack dev_ethtool(struct net *net, struct ifreq *ifr)
>  {
>  	struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
>  	void __user *useraddr = ifr->ifr_data;

Is this part really needed? AFAICS dev_ethtool() is only called from
dev_ioctl() which is in a different compilation unit so that
dev_ethtool() won't be inlined anyway.

Michal Kubecek

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ethtool: reduce stack usage with clang
  2019-03-07 10:06 ` Michal Kubecek
@ 2019-03-07 15:59   ` Arnd Bergmann
  0 siblings, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2019-03-07 15:59 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: David S. Miller, Florian Fainelli, Jakub Kicinski, Jiri Pirko,
	Nick Desaulniers, Kees Cook, Wenwen Wang, Ilya Lesokhin,
	Pablo Neira Ayuso, Edward Cree, Networking,
	Linux Kernel Mailing List

On Thu, Mar 7, 2019 at 11:06 AM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Thu, Mar 07, 2019 at 10:33:35AM +0100, Arnd Bergmann wrote:

> > @@ -2533,7 +2535,7 @@ static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr)
> >
> >  /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
> >
> > -int dev_ethtool(struct net *net, struct ifreq *ifr)
> > +int noinline_for_stack dev_ethtool(struct net *net, struct ifreq *ifr)
> >  {
> >       struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
> >       void __user *useraddr = ifr->ifr_data;
>
> Is this part really needed? AFAICS dev_ethtool() is only called from
> dev_ioctl() which is in a different compilation unit so that
> dev_ethtool() won't be inlined anyway.

No, you are right. I had accidentally left this in place from an earlier
version. Sending a v2 now.

      Arnd

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-03-07 15:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07  9:33 [PATCH] ethtool: reduce stack usage with clang Arnd Bergmann
2019-03-07 10:06 ` Michal Kubecek
2019-03-07 15:59   ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.