Re: [linux-next:master 4744/6155] net/dcb/dcbnl.c:1230:1: warning: the frame size of 1244 bytes is larger than 1024 bytes

[<Daniel.Machon@microchip.com>]

> >> kernel test robot <lkp@intel.com> writes:
> >>
> >> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> >> > head:   d8e87774068af213ab5b058b1b114dc397b577aa
> >> > commit: 6182d5875c330a5a611687caa05f47752455720c [4744/6155] net: dcb: add new apptrust attribute
> >> > config: i386-buildonly-randconfig-r006-20221107
> >> > compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> >> > reproduce (this is a W=1 build):
> >> >         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=6182d5875c330a5a611687caa05f47752455720c
> >> >         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >> >         git fetch --no-tags linux-next master
> >> >         git checkout 6182d5875c330a5a611687caa05f47752455720c
> >> >         # save the config file
> >> >         mkdir build_dir && cp config build_dir/.config
> >> >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/dcb/
> >> >
> >> > If you fix the issue, kindly add following tag where applicable
> >> > | Reported-by: kernel test robot <lkp@intel.com>
> >> >
> >> > All warnings (new ones prefixed by >>):
> >> >
> >> >    net/dcb/dcbnl.c: In function 'dcbnl_ieee_fill':
> >> >>> net/dcb/dcbnl.c:1230:1: warning: the frame size of 1244 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> >>
> >> The on-stack array that was added is pushing things:
> >>
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1170       if (ops->dcbnl_getapptrust) {
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1171               u8 selectors[IEEE_8021QAZ_APP_SEL_MAX + 1] = {0};
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1172               int nselectors, i;
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1173
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1174               apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1175               if (!apptrust)
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1176                       return -EMSGSIZE;
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1177
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1178               err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1179               if (!err) {
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1180                       for (i = 0; i < nselectors; i++) {
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1181                               enum ieee_attrs_app type =
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1182                                       dcbnl_app_attr_type_get(selectors[i]);
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1183                               err = nla_put_u8(skb, type, selectors[i]);
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1184                               if (err) {
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1185                                       nla_nest_cancel(skb, apptrust);
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1186                                       return err;
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1187                               }
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1188                       }
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1189               }
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1190
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1191               nla_nest_end(skb, apptrust);
> >> > 6182d5875c330a Daniel Machon   2022-11-01  1192       }
> >>
> >> I think this should either be converted to dynamic allocation, or the
> >> array size should be reduced to number of unique selectors, instead of
> >> the value of the highest selector.
> >
> > Yep. Dynamic allocation sounds good to me. Alternatively, could we move
> > the getapptrust part to a separate noinline function?
> 
> I think the reasoning behind the warning is that functions with large
> frames consume too much stack space, which is scarce in kernel. Moving
> the logic to a separate function might silence the warning, but wouldn't
> make the issue go away.
> 
> Extracting the logic to a separate function might still make sense,
> because it's going to make it easier to manage the dynamic allocation,
> but is not enough on its own.

Alright. Will submit a patch to net-next soon :-)

/ Daniel