Re: [PATCH 1/4] RDMA/core: Fix ib_dma_max_seg_size()

[Bart Van Assche <bvanassche@acm.org>]

On 10/21/19 8:27 AM, Jason Gunthorpe wrote:
> On Mon, Oct 21, 2019 at 08:03:32AM -0700, Bart Van Assche wrote:
>> On 10/21/19 7:09 AM, Jason Gunthorpe wrote:
>>> On Sun, Oct 20, 2019 at 07:10:27PM -0700, Bart Van Assche wrote:
>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>>> index 6a47ba85c54c..e6c167d03aae 100644
>>>> +++ b/include/rdma/ib_verbs.h
>>>> @@ -4043,9 +4043,7 @@ static inline void ib_dma_unmap_sg_attrs(struct ib_device *dev,
>>>>     */
>>>>    static inline unsigned int ib_dma_max_seg_size(struct ib_device *dev)
>>>>    {
>>>> -	struct device_dma_parameters *p = dev->dma_device->dma_parms;
>>>> -
>>>> -	return p ? p->max_segment_size : UINT_MAX;
>>>> +	return dma_get_max_seg_size(dev->dma_device);
>>>>    }
>>>
>>> Should we get rid of this wrapper?
>>
>> Hi Jason,
>>
>> In general I agree that getting rid of single line inline functions is good.
>> In this case however I'd like to keep the wrapper such that RDMA ULP code
>> does not have to deal with the choice between dev->dma_device and &dev->dev.
>>  From struct ib_device:
>>   /* Do not access @dma_device directly from ULP nor from HW drivers. */
>> struct device                *dma_device;
> 
> Do you think it is a mistake we have dma_device at all?
> 
> Can the modern dma framework let us make the 'struct ib_device' into a
> full dma_device that is still connected to some PCI device?

Hi Jason,

My understanding is that dma_device is passed as the first argument to 
DMA mapping functions. Before PCIe P2P support was introduced in the 
RDMA code, the only struct device members used by DMA mapping functions 
were dma_ops, dma_mask, coherent_dma_mask, bus_dma_mask and dma_parms. I 
think it would have been sufficient to copy all these members from the 
PCI device into struct ib_device before PCIe P2P support was introduced. 
The dma_device pointer however is also passed to the pci_p2pdma_map_sg() 
and pci_p2pdma_unmap_sg() functions. These functions use several 
additional members of struct pci_dev. Although it may be possible to 
eliminate the dma_device member, this may make maintaining the RDMA core 
harder because setup_dma_device() will have to be modified every time a 
DMA mapping function uses an additional member from struct device.

Bart.