* [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs"
@ 2021-05-26 14:45 Frederic Barrat
2021-05-27 2:13 ` Alexey Kardashevskiy
2021-06-06 11:34 ` Michael Ellerman
0 siblings, 2 replies; 5+ messages in thread
From: Frederic Barrat @ 2021-05-26 14:45 UTC (permalink / raw)
To: linuxppc-dev, aik; +Cc: zdai
This reverts commit 3c0468d4451eb6b4f6604370639f163f9637a479.
That commit was breaking alignment guarantees for the DMA address when
allocating coherent mappings, as described in
Documentation/core-api/dma-api-howto.rst
It was also noticed by Mellanox' driver:
[ 1515.763621] mlx5_core c002:01:00.0: mlx5_frag_buf_alloc_node:146:(pid 13402): unexpected map alignment: 0x0800000000c61000, page_shift=16
[ 1515.763635] mlx5_core c002:01:00.0: mlx5_cqwq_create:181:(pid
13402): mlx5_frag_buf_alloc_node() failed, -12
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
arch/powerpc/kernel/iommu.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 57d6b85e9b96..2af89a5e379f 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -898,7 +898,6 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
unsigned int order;
unsigned int nio_pages, io_order;
struct page *page;
- size_t size_io = size;
size = PAGE_ALIGN(size);
order = get_order(size);
@@ -925,9 +924,8 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
memset(ret, 0, size);
/* Set up tces to cover the allocated range */
- size_io = IOMMU_PAGE_ALIGN(size_io, tbl);
- nio_pages = size_io >> tbl->it_page_shift;
- io_order = get_iommu_order(size_io, tbl);
+ nio_pages = size >> tbl->it_page_shift;
+ io_order = get_iommu_order(size, tbl);
mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
mask >> tbl->it_page_shift, io_order, 0);
if (mapping == DMA_MAPPING_ERROR) {
@@ -942,9 +940,10 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
void *vaddr, dma_addr_t dma_handle)
{
if (tbl) {
- size_t size_io = IOMMU_PAGE_ALIGN(size, tbl);
- unsigned int nio_pages = size_io >> tbl->it_page_shift;
+ unsigned int nio_pages;
+ size = PAGE_ALIGN(size);
+ nio_pages = size >> tbl->it_page_shift;
iommu_free(tbl, dma_handle, nio_pages);
size = PAGE_ALIGN(size);
free_pages((unsigned long)vaddr, get_order(size));
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs"
2021-05-26 14:45 [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs" Frederic Barrat
@ 2021-05-27 2:13 ` Alexey Kardashevskiy
2021-05-27 6:10 ` Frederic Barrat
2021-06-06 11:34 ` Michael Ellerman
1 sibling, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2021-05-27 2:13 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev; +Cc: zdai
On 27/05/2021 00:45, Frederic Barrat wrote:
> This reverts commit 3c0468d4451eb6b4f6604370639f163f9637a479.
>
> That commit was breaking alignment guarantees for the DMA address when
> allocating coherent mappings, as described in
> Documentation/core-api/dma-api-howto.rst
>
> It was also noticed by Mellanox' driver:
> [ 1515.763621] mlx5_core c002:01:00.0: mlx5_frag_buf_alloc_node:146:(pid 13402): unexpected map alignment: 0x0800000000c61000, page_shift=16
> [ 1515.763635] mlx5_core c002:01:00.0: mlx5_cqwq_create:181:(pid
> 13402): mlx5_frag_buf_alloc_node() failed, -12
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Should it be
Fixes: 3c0468d4451e ("powerpc/kernel/iommu: Align size for
IOMMU_PAGE_SIZE() to save TCEs")
?
Anyway,
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
I should have known better in the first place, sorry :-/ Thanks,
> ---
> arch/powerpc/kernel/iommu.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 57d6b85e9b96..2af89a5e379f 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -898,7 +898,6 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
> unsigned int order;
> unsigned int nio_pages, io_order;
> struct page *page;
> - size_t size_io = size;
>
> size = PAGE_ALIGN(size);
> order = get_order(size);
> @@ -925,9 +924,8 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
> memset(ret, 0, size);
>
> /* Set up tces to cover the allocated range */
> - size_io = IOMMU_PAGE_ALIGN(size_io, tbl);
> - nio_pages = size_io >> tbl->it_page_shift;
> - io_order = get_iommu_order(size_io, tbl);
> + nio_pages = size >> tbl->it_page_shift;
> + io_order = get_iommu_order(size, tbl);
> mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
> mask >> tbl->it_page_shift, io_order, 0);
> if (mapping == DMA_MAPPING_ERROR) {
> @@ -942,9 +940,10 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
> void *vaddr, dma_addr_t dma_handle)
> {
> if (tbl) {
> - size_t size_io = IOMMU_PAGE_ALIGN(size, tbl);
> - unsigned int nio_pages = size_io >> tbl->it_page_shift;
> + unsigned int nio_pages;
>
> + size = PAGE_ALIGN(size);
> + nio_pages = size >> tbl->it_page_shift;
> iommu_free(tbl, dma_handle, nio_pages);
> size = PAGE_ALIGN(size);
> free_pages((unsigned long)vaddr, get_order(size));
>
--
Alexey
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs"
2021-05-27 2:13 ` Alexey Kardashevskiy
@ 2021-05-27 6:10 ` Frederic Barrat
2021-06-01 1:23 ` Michael Ellerman
0 siblings, 1 reply; 5+ messages in thread
From: Frederic Barrat @ 2021-05-27 6:10 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: zdai
On 27/05/2021 04:13, Alexey Kardashevskiy wrote:
> On 27/05/2021 00:45, Frederic Barrat wrote:
>> This reverts commit 3c0468d4451eb6b4f6604370639f163f9637a479.
>>
>> That commit was breaking alignment guarantees for the DMA address when
>> allocating coherent mappings, as described in
>> Documentation/core-api/dma-api-howto.rst
>>
>> It was also noticed by Mellanox' driver:
>> [ 1515.763621] mlx5_core c002:01:00.0:
>> mlx5_frag_buf_alloc_node:146:(pid 13402): unexpected map alignment:
>> 0x0800000000c61000, page_shift=16
>> [ 1515.763635] mlx5_core c002:01:00.0: mlx5_cqwq_create:181:(pid
>> 13402): mlx5_frag_buf_alloc_node() failed, -12
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>
> Should it be
>
> Fixes: 3c0468d4451e ("powerpc/kernel/iommu: Align size for
> IOMMU_PAGE_SIZE() to save TCEs")
>
> ?
I had been wondering the same... I can see many revert commits with and
without a "Fixes:" line. Since here the offending commit was merged in
the latest merge window, I was thinking Fixes doesn't really bring
anything, it will all stay internal to v5.13 development. I'd be happy
to hear of the expected way of handling it. I'm guessing a big part of
the answer is whether the tooling looking for a "Fixes" line is also
looking for reverts.
Fred
>
> Anyway,
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs"
2021-05-27 6:10 ` Frederic Barrat
@ 2021-06-01 1:23 ` Michael Ellerman
0 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2021-06-01 1:23 UTC (permalink / raw)
To: Frederic Barrat, Alexey Kardashevskiy, linuxppc-dev; +Cc: zdai
Frederic Barrat <fbarrat@linux.ibm.com> writes:
> On 27/05/2021 04:13, Alexey Kardashevskiy wrote:
>> On 27/05/2021 00:45, Frederic Barrat wrote:
>>> This reverts commit 3c0468d4451eb6b4f6604370639f163f9637a479.
>>>
>>> That commit was breaking alignment guarantees for the DMA address when
>>> allocating coherent mappings, as described in
>>> Documentation/core-api/dma-api-howto.rst
>>>
>>> It was also noticed by Mellanox' driver:
>>> [ 1515.763621] mlx5_core c002:01:00.0:
>>> mlx5_frag_buf_alloc_node:146:(pid 13402): unexpected map alignment:
>>> 0x0800000000c61000, page_shift=16
>>> [ 1515.763635] mlx5_core c002:01:00.0: mlx5_cqwq_create:181:(pid
>>> 13402): mlx5_frag_buf_alloc_node() failed, -12
>>>
>>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>>
>> Should it be
>>
>> Fixes: 3c0468d4451e ("powerpc/kernel/iommu: Align size for
>> IOMMU_PAGE_SIZE() to save TCEs")
>>
>> ?
>
>
> I had been wondering the same... I can see many revert commits with and
> without a "Fixes:" line. Since here the offending commit was merged in
> the latest merge window, I was thinking Fixes doesn't really bring
> anything, it will all stay internal to v5.13 development.
It is still valuable, even if the fix lands in the same release as the
original commit.
The original commit could have been backported elsewhere, so the Fixes
tag is still useful in that case.
> I'd be happy to hear of the expected way of handling it. I'm guessing
> a big part of the answer is whether the tooling looking for a "Fixes"
> line is also looking for reverts.
Tooling should look for both Fixes and reverts, because not everyone is
going to remember to add Fixes tags to a revert.
But I think it's still helpful to include the Fixes tag, I sometimes
grep the commit log for "Fixes: <sha>" and I'm sure others do too.
I've added it to this commit, no need to resend.
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs"
2021-05-26 14:45 [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs" Frederic Barrat
2021-05-27 2:13 ` Alexey Kardashevskiy
@ 2021-06-06 11:34 ` Michael Ellerman
1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2021-06-06 11:34 UTC (permalink / raw)
To: Frederic Barrat, aik, linuxppc-dev; +Cc: zdai
On Wed, 26 May 2021 16:45:40 +0200, Frederic Barrat wrote:
> This reverts commit 3c0468d4451eb6b4f6604370639f163f9637a479.
>
> That commit was breaking alignment guarantees for the DMA address when
> allocating coherent mappings, as described in
> Documentation/core-api/dma-api-howto.rst
>
> It was also noticed by Mellanox' driver:
> [ 1515.763621] mlx5_core c002:01:00.0: mlx5_frag_buf_alloc_node:146:(pid 13402): unexpected map alignment: 0x0800000000c61000, page_shift=16
> [ 1515.763635] mlx5_core c002:01:00.0: mlx5_cqwq_create:181:(pid
> 13402): mlx5_frag_buf_alloc_node() failed, -12
Applied to powerpc/fixes.
[1/1] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs"
https://git.kernel.org/powerpc/c/59cc84c802eb923805e7bba425976a3df5ce35d8
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-06 11:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 14:45 [PATCH] Revert "powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE() to save TCEs" Frederic Barrat
2021-05-27 2:13 ` Alexey Kardashevskiy
2021-05-27 6:10 ` Frederic Barrat
2021-06-01 1:23 ` Michael Ellerman
2021-06-06 11:34 ` Michael Ellerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.