On Tue, Dec 10, 2019 at 03:04:38PM -0400, Jason Gunthorpe wrote:
> On Mon, Dec 09, 2019 at 02:49:20PM -0800, Jeff Kirsher wrote:
> > +{
> > + struct i40e_info *ldev = (struct i40e_info *)rf->ldev.if_ldev;
>
> Why are there so many casts in this file? Is this really container of?
>
> > + hdl = kzalloc((sizeof(*hdl) + sizeof(*iwdev)), GFP_KERNEL);
> > + if (!hdl)
> > + return -ENOMEM;
> > +
> > + iwdev = (struct irdma_device *)((u8 *)hdl + sizeof(*hdl));
>
> Yikes, use structs and container of for things like this please.
>
> > + iwdev->param_wq = alloc_ordered_workqueue("l2params", WQ_MEM_RECLAIM);
> > + if (!iwdev->param_wq)
> > + goto error;
>
> Leon usually asks why another work queue at this point, at least have
> a comment justifying why. Shouldn't it have a better name?
Yeah, combination of WQ_MEM_RECLAIM flag and "params" in the name raises
eyebrows immediately.
Thanks
