Re: [PATCH v3 05/20] RDMA/irdma: Add driver framework definitions

[Leon Romanovsky <leon@kernel.org>]

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