RE: [PATCH 1/2] device: Stop requiring that struct device is embedded in struct pci_dev

[Parav Pandit]

Hi Bart,

> -----Original Message-----
> From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Bart Van Assche
> Sent: Monday, March 6, 2017 6:36 PM
> To: Doug Ledford <dledford@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Sebastian Ott
> <sebott@xxxxxxxxxxxxxxxxxx>; Parav Pandit <parav@xxxxxxxxxxxx>; linux-
> rdma@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Bart Van Assche
> <bart.vanassche@xxxxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>;
> Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>; David Woodhouse
> <dwmw2@xxxxxxxxxxxxx>; H . Peter Anvin <hpa@xxxxxxxxx>; Ingo Molnar
> <mingo@xxxxxxxxxx>; Russell King <linux@xxxxxxxxxxxxxxx>
> Subject: [PATCH 1/2] device: Stop requiring that struct device is embedded in
> struct pci_dev
> 
> The dma mapping operations of several architectures and also of several I/O
> MMU implementations need to translate a struct device pointer into a struct
> pci_dev pointer. This translation is performed by to_pci_dev(). That macro
> assumes that struct device is embedded in struct pci_dev. However, that is
> not the case for the device structure in struct ib_device. Since that device

Why can't ib subsystem pass device structure that is embedded in pci_dev when it makes calls to dma_map APIs?
The whole point of clean up was to avoid an if() condition in hot datapath.
If we invoke dma_map_ and friend functions with right device, shouldn't it work?
That avoids the if() condition as well and avoids changing core of Linux like done in this bug fix.
I think ib_device should store the right struct device pointer that needs to go to dma_apis, rather than including pci_dev structure pointer in device core 
layer.

Pseudo example code:
struct ib_device {
	struct device *dma_device;
};

ib_dma_unmap_single() which had if(), that got removed with dma_unmap_single() with cleanup patch.

Instead of,
static inline void ib_dma_unmap_single(struct ib_device *dev,
                                       u64 addr, size_t size,
                                       enum dma_data_direction direction)
{
	dma_unmap_single(&dev->dev, addr, size, direction);
}

Why can't we do this?

static inline void ib_dma_unmap_single(struct ib_device *dev,
                                       u64 addr, size_t size,
                                       enum dma_data_direction direction)
{
	dma_unmap_single(dev->dma_device, addr, size, direction);
}

This avoids increasing all device size by 8 bytes in system.
It also clean approach where core device structure doesn't have to bother for pci_dev.