On Fri, Dec 10, 2021 at 11:39 AM Nathan Chancellor <[email protected]> wrote: > > Hi Dan, > > On Tue, Sep 21, 2021 at 12:22:16PM -0700, Dan Williams wrote: > > The kbuild robot reports: > > > > drivers/cxl/core/bus.c:516:1: warning: stack frame size (1032) exceeds > > limit (1024) in function 'devm_cxl_add_decoder' > > > > It is also the case the devm_cxl_add_decoder() is unwieldy to use for > > all the different decoder types. Fix the stack usage by splitting the > > creation into alloc and add steps. This also allows for context > > specific construction before adding. > > > > With the split the caller is responsible for registering a devm callback > > to trigger device_unregister() for the decoder rather than it being > > implicit in the decoder registration. I.e. the routine that calls alloc > > is responsible for calling put_device() if the "add" operation fails. > > > > Reported-by: kernel test robot <[email protected]> > > Reported-by: Nathan Chancellor <[email protected]> > > Reported-by: Dan Carpenter <[email protected]> > > Reviewed-by: Ben Widawsky <[email protected]> > > Signed-off-by: Dan Williams <[email protected]> > > Apologies for not noticing this sooner, given that I was on the thread. > > This patch as commit 48667f676189 ("cxl/core: Split decoder setup into > alloc + add") in mainline does not fully resolve the stack frame > warning. I still see an error with both GCC 11 and LLVM 12 with > allmodconfig minus CONFIG_KASAN. > > GCC 11: > > drivers/cxl/core/bus.c: In function ‘cxl_decoder_alloc’: > drivers/cxl/core/bus.c:523:1: error: the frame size of 1032 bytes is larger > than 1024 bytes [-Werror=frame-larger-than=] > 523 | } > | ^ > cc1: all warnings being treated as errors > > LLVM 12: > > drivers/cxl/core/bus.c:486:21: error: stack frame size of 1056 bytes in > function 'cxl_decoder_alloc' [-Werror,-Wframe-larger-than=] > struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port, int nr_targets) > ^ > 1 error generated. > > This is due to the cxld_const_init structure, which is allocated on the > stack, presumably due to the "const" change requested in v5 that was > applied to v6. Undoing that resolves the warning for me with both > compilers. I am not sure if you have a better idea for how to resolve > that. > > diff --git a/drivers/cxl/core/bus.c b/drivers/cxl/core/bus.c > index ebd061d03950..46ce58376580 100644 > --- a/drivers/cxl/core/bus.c > +++ b/drivers/cxl/core/bus.c > @@ -485,9 +485,7 @@ static int decoder_populate_targets(struct cxl_decoder > *cxld, > > struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port, int nr_targets) > { > - struct cxl_decoder *cxld, cxld_const_init = { > - .nr_targets = nr_targets, > - }; > + struct cxl_decoder *cxld; > struct device *dev; > int rc = 0; > > @@ -497,13 +495,13 @@ struct cxl_decoder *cxl_decoder_alloc(struct cxl_port > *port, int nr_targets) > cxld = kzalloc(struct_size(cxld, target, nr_targets), GFP_KERNEL); > if (!cxld) > return ERR_PTR(-ENOMEM); > - memcpy(cxld, &cxld_const_init, sizeof(cxld_const_init)); > > rc = ida_alloc(&port->decoder_ida, GFP_KERNEL); > if (rc < 0) > goto err; > > cxld->id = rc; > + cxld->nr_targets = nr_targets; > dev = &cxld->dev; > device_initialize(dev); > device_set_pm_not_required(dev); > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 3af704e9b448..7c2b51746e31 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -191,7 +191,7 @@ struct cxl_decoder { > int interleave_granularity; > enum cxl_decoder_type target_type; > unsigned long flags; > - const int nr_targets; > + int nr_targets; > struct cxl_dport *target[];
That change looks like the right way to go to me. Thanks for the follow-up!
