On 14/04/2020 22:25, Christoph Hellwig wrote:
> For a long time the DMA API has been implemented inline in dma-mapping.h,
> but the function bodies can be quite large. Move them all out of line.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> include/linux/dma-direct.h | 58 +++++++++
> include/linux/dma-mapping.h | 247 ++++--------------------------------
> kernel/dma/direct.c | 9 --
> kernel/dma/mapping.c | 164 ++++++++++++++++++++++++
> 4 files changed, 244 insertions(+), 234 deletions(-)
>
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index 24b8684aa21d..da689ad5fffd 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -85,4 +85,62 @@ int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
> void *cpu_addr, dma_addr_t dma_addr, size_t size,
> unsigned long attrs);
> int dma_direct_supported(struct device *dev, u64 mask);
> +dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
> + unsigned long offset, size_t size, enum dma_data_direction dir,
> + unsigned long attrs);
> +int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
> + enum dma_data_direction dir, unsigned long attrs);
> +dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs);
> +
> +#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
> + defined(CONFIG_SWIOTLB)
> +void dma_direct_sync_single_for_device(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir);
> +void dma_direct_sync_sg_for_device(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> +#else
> +static inline void dma_direct_sync_single_for_device(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir)
> +{
> +}
> +static inline void dma_direct_sync_sg_for_device(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> +{
> +}
> +#endif
> +
> +#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
> + defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) || \
> + defined(CONFIG_SWIOTLB)
> +void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs);
> +void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sgl,
> + int nents, enum dma_data_direction dir, unsigned long attrs);
> +void dma_direct_sync_single_for_cpu(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir);
> +void dma_direct_sync_sg_for_cpu(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> +#else
> +static inline void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> +}
> +static inline void dma_direct_unmap_sg(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir,
> + unsigned long attrs)
> +{
> +}
> +static inline void dma_direct_sync_single_for_cpu(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir)
> +{
> +}
> +static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> +{
> +}
> +#endif
> +
> +size_t dma_direct_max_mapping_size(struct device *dev);
> +
> #endif /* _LINUX_DMA_DIRECT_H */
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 330ad58fbf4d..793ad775cd54 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -188,73 +188,6 @@ static inline int dma_mmap_from_global_coherent(struct vm_area_struct *vma,
> }
> #endif /* CONFIG_DMA_DECLARE_COHERENT */
>
> -static inline bool dma_is_direct(const struct dma_map_ops *ops)
> -{
> - return likely(!ops);
> -}
> -
> -/*
> - * All the dma_direct_* declarations are here just for the indirect call bypass,
> - * and must not be used directly drivers!
> - */
> -dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
> - unsigned long offset, size_t size, enum dma_data_direction dir,
> - unsigned long attrs);
> -int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
> - enum dma_data_direction dir, unsigned long attrs);
> -dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs);
> -
> -#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
> - defined(CONFIG_SWIOTLB)
> -void dma_direct_sync_single_for_device(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir);
> -void dma_direct_sync_sg_for_device(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> -#else
> -static inline void dma_direct_sync_single_for_device(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir)
> -{
> -}
> -static inline void dma_direct_sync_sg_for_device(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> -{
> -}
> -#endif
> -
> -#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
> - defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) || \
> - defined(CONFIG_SWIOTLB)
> -void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs);
> -void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sgl,
> - int nents, enum dma_data_direction dir, unsigned long attrs);
> -void dma_direct_sync_single_for_cpu(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir);
> -void dma_direct_sync_sg_for_cpu(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> -#else
> -static inline void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs)
> -{
> -}
> -static inline void dma_direct_unmap_sg(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> -}
> -static inline void dma_direct_sync_single_for_cpu(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir)
> -{
> -}
> -static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> -{
> -}
> -#endif
> -
> -size_t dma_direct_max_mapping_size(struct device *dev);
> -
> #ifdef CONFIG_HAS_DMA
> #include <asm/dma-mapping.h>
>
> @@ -271,164 +204,6 @@ static inline void set_dma_ops(struct device *dev,
> dev->dma_ops = dma_ops;
> }
>
> -static inline dma_addr_t dma_map_page_attrs(struct device *dev,
> - struct page *page, size_t offset, size_t size,
> - enum dma_data_direction dir, unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - dma_addr_t addr;
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
> - else
> - addr = ops->map_page(dev, page, offset, size, dir, attrs);
> - debug_dma_map_page(dev, page, offset, size, dir, addr);
> -
> - return addr;
> -}
> -
> -static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_unmap_page(dev, addr, size, dir, attrs);
> - else if (ops->unmap_page)
> - ops->unmap_page(dev, addr, size, dir, attrs);
> - debug_dma_unmap_page(dev, addr, size, dir);
> -}
> -
> -/*
> - * dma_maps_sg_attrs returns 0 on error and > 0 on success.
> - * It should never return a value < 0.
> - */
> -static inline int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
> - int nents, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - int ents;
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
> - else
> - ents = ops->map_sg(dev, sg, nents, dir, attrs);
> - BUG_ON(ents < 0);
> - debug_dma_map_sg(dev, sg, nents, ents, dir);
> -
> - return ents;
> -}
> -
> -static inline void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
> - int nents, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - debug_dma_unmap_sg(dev, sg, nents, dir);
> - if (dma_is_direct(ops))
> - dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
> - else if (ops->unmap_sg)
> - ops->unmap_sg(dev, sg, nents, dir, attrs);
> -}
> -
> -static inline dma_addr_t dma_map_resource(struct device *dev,
> - phys_addr_t phys_addr,
> - size_t size,
> - enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - dma_addr_t addr = DMA_MAPPING_ERROR;
> -
> - BUG_ON(!valid_dma_direction(dir));
> -
> - /* Don't allow RAM to be mapped */
> - if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
> - return DMA_MAPPING_ERROR;
> -
> - if (dma_is_direct(ops))
> - addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
> - else if (ops->map_resource)
> - addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
> -
> - debug_dma_map_resource(dev, phys_addr, size, dir, addr);
> - return addr;
> -}
> -
> -static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (!dma_is_direct(ops) && ops->unmap_resource)
> - ops->unmap_resource(dev, addr, size, dir, attrs);
> - debug_dma_unmap_resource(dev, addr, size, dir);
> -}
> -
> -static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
> - size_t size,
> - enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_single_for_cpu(dev, addr, size, dir);
> - else if (ops->sync_single_for_cpu)
> - ops->sync_single_for_cpu(dev, addr, size, dir);
> - debug_dma_sync_single_for_cpu(dev, addr, size, dir);
> -}
> -
> -static inline void dma_sync_single_for_device(struct device *dev,
> - dma_addr_t addr, size_t size,
> - enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_single_for_device(dev, addr, size, dir);
> - else if (ops->sync_single_for_device)
> - ops->sync_single_for_device(dev, addr, size, dir);
> - debug_dma_sync_single_for_device(dev, addr, size, dir);
> -}
> -
> -static inline void
> -dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
> - int nelems, enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
> - else if (ops->sync_sg_for_cpu)
> - ops->sync_sg_for_cpu(dev, sg, nelems, dir);
> - debug_dma_sync_sg_for_cpu(dev, sg, nelems, dir);
> -}
> -
> -static inline void
> -dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
> - int nelems, enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_sg_for_device(dev, sg, nelems, dir);
> - else if (ops->sync_sg_for_device)
> - ops->sync_sg_for_device(dev, sg, nelems, dir);
> - debug_dma_sync_sg_for_device(dev, sg, nelems, dir);
> -
> -}
>
> static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
> {
> @@ -439,6 +214,28 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
> return 0;
> }
>
> +dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
> + size_t offset, size_t size, enum dma_data_direction dir,
> + unsigned long attrs);
> +void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
> + enum dma_data_direction dir, unsigned long attrs);
> +int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
> + enum dma_data_direction dir, unsigned long attrs);
> +void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
> + int nents, enum dma_data_direction dir,
> + unsigned long attrs);
> +dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs);
> +void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
> + enum dma_data_direction dir, unsigned long attrs);
> +void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size,
> + enum dma_data_direction dir);
> +void dma_sync_single_for_device(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir);
> +void dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
> + int nelems, enum dma_data_direction dir);
> +void dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
> + int nelems, enum dma_data_direction dir);
> void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
> gfp_t flag, unsigned long attrs);
> void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 8f4bbdaf965e..f1a9099a4b5b 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -260,7 +260,6 @@ void dma_direct_sync_single_for_device(struct device *dev,
> if (!dev_is_dma_coherent(dev))
> arch_sync_dma_for_device(paddr, size, dir);
> }
> -EXPORT_SYMBOL(dma_direct_sync_single_for_device);
May be this is correct and allowed (no idea) but removing exported
symbols at least deserves a mention in the commit log, does not it?
The rest of the series is fine and works. Thanks,
--
Alexey
On Wed, Apr 15, 2020 at 12:26:04PM +1000, Alexey Kardashevskiy wrote:
> May be this is correct and allowed (no idea) but removing exported
> symbols at least deserves a mention in the commit log, does not it?
>
> The rest of the series is fine and works. Thanks,
Maybe I can throw in a line, but the point is that dma_direct_*
was exported as dma_* called them inline. Now dma_* is out of line
and exported instead, which always was the actual API.
On 15/04/2020 16:18, Christoph Hellwig wrote:
> On Wed, Apr 15, 2020 at 12:26:04PM +1000, Alexey Kardashevskiy wrote:
>> May be this is correct and allowed (no idea) but removing exported
>> symbols at least deserves a mention in the commit log, does not it?
>>
>> The rest of the series is fine and works. Thanks,
>
> Maybe I can throw in a line, but the point is that dma_direct_*
> was exported as dma_* called them inline. Now dma_* is out of line
> and exported instead, which always was the actual API.
They become inline in 2/4.
And the fact they were exported leaves possibility that there is a
driver somewhere relying on these symbols or distro kernel won't build
because the symbol disappeared from exports (I do not know what KABI
guarantees or if mainline kernel cares). I do not care in particular but
some might, a line separated with empty lines in the commit log would do.
--
Alexey
On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
> And the fact they were exported leaves possibility that there is a
> driver somewhere relying on these symbols or distro kernel won't build
> because the symbol disappeared from exports (I do not know what KABI
> guarantees or if mainline kernel cares).
We absolutely do not care. In fact for abuses of APIs that drivers
should not use we almost care to make them private and break people
abusing them.
> I do not care in particular but
> some might, a line separated with empty lines in the commit log would do.
I'll add a blurb for the next version.
On 17/04/2020 17:58, Christoph Hellwig wrote:
> On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
>> And the fact they were exported leaves possibility that there is a
>> driver somewhere relying on these symbols or distro kernel won't build
>> because the symbol disappeared from exports (I do not know what KABI
>> guarantees or if mainline kernel cares).
>
> We absolutely do not care. In fact for abuses of APIs that drivers
> should not use we almost care to make them private and break people
> abusing them.
ok :)
>> I do not care in particular but
>> some might, a line separated with empty lines in the commit log would do.
>
> I'll add a blurb for the next version.
Has it gone anywhere? Thanks,
--
Alexey
On Tue, May 05, 2020 at 02:18:37PM +1000, Alexey Kardashevskiy wrote:
>
>
> On 17/04/2020 17:58, Christoph Hellwig wrote:
> > On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
> >> And the fact they were exported leaves possibility that there is a
> >> driver somewhere relying on these symbols or distro kernel won't build
> >> because the symbol disappeared from exports (I do not know what KABI
> >> guarantees or if mainline kernel cares).
> >
> > We absolutely do not care. In fact for abuses of APIs that drivers
> > should not use we almost care to make them private and break people
> > abusing them.
>
> ok :)
>
> >> I do not care in particular but
> >> some might, a line separated with empty lines in the commit log would do.
> >
> > I'll add a blurb for the next version.
>
>
> Has it gone anywhere? Thanks,
I've been hoping for the sg_buf helpers to land first, as they need
backporting and would conflict. Do you urgently need the series?
On 09/05/2020 18:19, Christoph Hellwig wrote:
> On Tue, May 05, 2020 at 02:18:37PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 17/04/2020 17:58, Christoph Hellwig wrote:
>>> On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
>>>> And the fact they were exported leaves possibility that there is a
>>>> driver somewhere relying on these symbols or distro kernel won't build
>>>> because the symbol disappeared from exports (I do not know what KABI
>>>> guarantees or if mainline kernel cares).
>>>
>>> We absolutely do not care. In fact for abuses of APIs that drivers
>>> should not use we almost care to make them private and break people
>>> abusing them.
>>
>> ok :)
>>
>>>> I do not care in particular but
>>>> some might, a line separated with empty lines in the commit log would do.
>>>
>>> I'll add a blurb for the next version.
>>
>>
>> Has it gone anywhere? Thanks,
>
> I've been hoping for the sg_buf helpers to land first, as they need
> backporting and would conflict. Do you urgently need the series?
Nah, not that urgent. Thanks,
--
Alexey
On 09/05/2020 18:19, Christoph Hellwig wrote:
> On Tue, May 05, 2020 at 02:18:37PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 17/04/2020 17:58, Christoph Hellwig wrote:
>>> On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
>>>> And the fact they were exported leaves possibility that there is a
>>>> driver somewhere relying on these symbols or distro kernel won't build
>>>> because the symbol disappeared from exports (I do not know what KABI
>>>> guarantees or if mainline kernel cares).
>>>
>>> We absolutely do not care. In fact for abuses of APIs that drivers
>>> should not use we almost care to make them private and break people
>>> abusing them.
>>
>> ok :)
>>
>>>> I do not care in particular but
>>>> some might, a line separated with empty lines in the commit log would do.
>>>
>>> I'll add a blurb for the next version.
>>
>>
>> Has it gone anywhere? Thanks,
>
> I've been hoping for the sg_buf helpers to land first, as they need
> backporting and would conflict. Do you urgently need the series?
Any progress with sg_buf helpers stuff? Thanks,
--
Alexey
On 03/06/2020 14:13, Alexey Kardashevskiy wrote:
>
>
> On 09/05/2020 18:19, Christoph Hellwig wrote:
>> On Tue, May 05, 2020 at 02:18:37PM +1000, Alexey Kardashevskiy wrote:
>>>
>>>
>>> On 17/04/2020 17:58, Christoph Hellwig wrote:
>>>> On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
>>>>> And the fact they were exported leaves possibility that there is a
>>>>> driver somewhere relying on these symbols or distro kernel won't build
>>>>> because the symbol disappeared from exports (I do not know what KABI
>>>>> guarantees or if mainline kernel cares).
>>>>
>>>> We absolutely do not care. In fact for abuses of APIs that drivers
>>>> should not use we almost care to make them private and break people
>>>> abusing them.
>>>
>>> ok :)
>>>
>>>>> I do not care in particular but
>>>>> some might, a line separated with empty lines in the commit log would do.
>>>>
>>>> I'll add a blurb for the next version.
>>>
>>>
>>> Has it gone anywhere? Thanks,
>>
>> I've been hoping for the sg_buf helpers to land first, as they need
>> backporting and would conflict. Do you urgently need the series?
>
> Any progress with sg_buf helpers stuff? Thanks,
Any luck there? I'd really like to cross this off my todo list :) Thanks,
--
Alexey
On Tue, Jul 07, 2020 at 10:43:10AM +1000, Alexey Kardashevskiy wrote:
> Any luck there? I'd really like to cross this off my todo list :) Thanks,
We had another incident with new net code poking into dma internals
blocking this series. That is now sorted out, so the series is back
on track.