On Tue, Sep 04, 2018 at 02:47:43PM -0600, Jason Gunthorpe wrote:
> On Mon, Sep 03, 2018 at 10:15:16AM +0300, Leon Romanovsky wrote:
> > On Sun, Sep 02, 2018 at 02:42:38PM -0600, Jason Gunthorpe wrote:
> > > On Tue, Aug 28, 2018 at 02:12:44PM +0300, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > >
> > > > Change on stack allocation to be dynamically allocated structure to fix
> > > > the following compilation warning.
> > > >
> > > > drivers/infiniband/hw/mlx5/cq.c: In function _mlx5_ib_create_cq_:
> > > > drivers/infiniband/hw/mlx5/cq.c:1091:1: warning: the frame size of 1064
> > > > bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > > }
> > > > ^
> > > >
> > > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > > drivers/infiniband/hw/mlx5/cq.c | 49 ++++++++++++++++++++++++-----------------
> > > > 1 file changed, 29 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> > > > index 088205d7f1a1..ada77c9e4b6b 100644
> > > > +++ b/drivers/infiniband/hw/mlx5/cq.c
> > > > @@ -778,7 +778,7 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
> > > > int entries, u32 **cqb,
> > > > int *cqe_size, int *index, int *inlen)
> > > > {
> > > > - struct mlx5_ib_create_cq ucmd = {};
> > > > + struct mlx5_ib_create_cq *ucmd;
> > >
> > > This is a 24 byte structure, it seems silly to allocate that alone on
> > > the heap.
> > >
> > > How did the stack frame get to 1064 bytes big? I can't get anywhere
> > > near that by eyeballing the stack usage in create_cq_user and
> > > mlx5_ib_create_cq..
> >
> > 1.
> > Before this patch
> > _ kernel git:(rdma-next) objdump -d drivers/infiniband/hw/mlx5/mlx5_ib.ko | scripts/checkstack.pl | grep mlx5_ib_create_cq
> > 0x00014da2 mlx5_ib_create_cq [mlx5_ib]: 1064
> >
> > After this patch
> > _ kernel git:(rdma-next) objdump -d drivers/infiniband/hw/mlx5/mlx5_ib.ko | scripts/checkstack.pl | grep mlx5_ib_create_cq
> > 0x00014da2 mlx5_ib_create_cq [mlx5_ib]: 992
> >
> > 2.
> > Main consumer of stack in create_cq_user() is mlx5_ib_cont_pages() with
> > reverted patch in question and empty mlx5_ib_cont_pages():
> > 0x00014da2 mlx5_ib_create_cq [mlx5_ib]: 848
>
> Wow, that is still another 800 bytes scattered all around.. That is
> alot of inlining for this function..
>
> > However I prefer do not touch mlx5_ib_cont_pages() without real bug.
>
> Well, none of this is a real bug, other places in the kernel require >
> 1024 in this warning today, so let's not make anything more complicated?
I have mixed feelings about it. From one side I agree with you that
there are other places in kernel which violate frame size warning and
this patch doesn't fix the worst case among them. From another, it is
exactly the reason why we ended here, no one is fixing those warnings.
I'm fine to drop it.
Thanks
>
> Jason
Attachment:
signature.asc
Description: PGP signature
