linux-kernel - Re: [PATCH 1/2] device: Stop requiring that struct device is embedded in struct pci

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:   Tue, 7 Mar 2017 18:14:03 +0100
From:   "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To:     Bart Van Assche <Bart.VanAssche@...disk.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "parav@...lanox.com" <parav@...lanox.com>,
        "sebott@...ux.vnet.ibm.com" <sebott@...ux.vnet.ibm.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "benh@...nel.crashing.org" <benh@...nel.crashing.org>
Subject: Re: [PATCH 1/2] device: Stop requiring that struct device is
 embedded in struct pci_dev

On Tue, Mar 07, 2017 at 04:54:58PM +0000, Bart Van Assche wrote:
> On Tue, 2017-03-07 at 05:52 +0100, Greg Kroah-Hartman wrote:
> > Somehow all other subsystems work just fine, don't instantly think that
> > the driver core needs to bend to the will of the IB code, because you
> > are somehow "special".  Hint, you aren't :)
> 
> Hi Greg,
> 
> In another e-mail Parav compared IB drivers with networking drivers.

Great, then notice that networking drivers don't need to do this type of
crud :)

> But I think that's a bad comparison: in the networking stack it's the
> network driver itself that sets up and triggers DMA while in the IB
> stack it's the upper layer protocol (ULP) driver that calls the
> functions defined in struct dma_ops. For some IB HW drivers (hfi1, qib
> and rdma_rxe) the ULP driver must
> use the DMA mapping operations from lib/dma-virt.c while for all other IB HW
> drivers the ULP driver must use the PCI DMA mapping functions. The ib_dma_*()
> functions select the right DMA mapping operations - either the PCI DMA
> mapping operations or those from lib/dma-virt.c. My question to you is how we
> should organize struct ib_device such that we can get rid of the ib_dma_*()
> helper functions. How to make sure that the to_pci_dev() translation works
> correctly for the device structure that is embedded in struct ib_device?
> Should a pointer to struct pci_dev be embedded in struct device (as done in
> patch 1/2 in this series)

I already said no to this, why do you think that it is still ok?

> or should the struct device in ib_device be changed
> into a struct pci_dev

Ick, no.

> and should the pci_dev information from /sys/devices/pci*/*/* be
> duplicated into the pci_dev information in struct ib_device
> (/sys/devices/pci*/*/*/infiniband/*)?

I don't think you really thought that one through :)

> For the latter approach, would
> there be a risk that the duplicated information becomes inconsistent?

No, it just wouldn't work :)

Why not just save off a pointer to your pci_dev in your ib_device
structure?  That way you know what the type is, and you have access to
everything you need.

But hey, I know nothing about IB and I really want to keep it that way.
You do what you want to, as long as you don't abuse the driver model,
like your patch 1/2 did.  Remember, not all the world is IB.

thanks,

greg k-h