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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Dec 2019 08:07:32 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net,
        gregkh@...uxfoundation.org,
        Mustafa Ismail <mustafa.ismail@...el.com>,
        netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
        nhorman@...hat.com, sassmann@...hat.com, parav@...lanox.com,
        Shiraz Saleem <shiraz.saleem@...el.com>
Subject: Re: [PATCH v3 05/20] RDMA/irdma: Add driver framework definitions

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