Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
with dma_alloc_coherent/dma_free_coherent() helps to reduce
code size, and simplify the code, and coherent DMA will not
clear the cache every time.
Signed-off-by: Cai Huoqing <[email protected]>
---
drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 5b9953105752..380445f98a49 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -1165,32 +1165,22 @@ static int hns_roce_alloc_cmq_desc(struct hns_roce_dev *hr_dev,
{
int size = ring->desc_num * sizeof(struct hns_roce_cmq_desc);
- ring->desc = kzalloc(size, GFP_KERNEL);
+ ring->desc = dma_alloc_coherent(hr_dev->dev, size,
+ &ring->desc_dma_addr, GFP_KERNEL);
if (!ring->desc)
return -ENOMEM;
- ring->desc_dma_addr = dma_map_single(hr_dev->dev, ring->desc, size,
- DMA_BIDIRECTIONAL);
- if (dma_mapping_error(hr_dev->dev, ring->desc_dma_addr)) {
- ring->desc_dma_addr = 0;
- kfree(ring->desc);
- ring->desc = NULL;
-
- return -ENOMEM;
- }
-
return 0;
}
static void hns_roce_free_cmq_desc(struct hns_roce_dev *hr_dev,
struct hns_roce_v2_cmq_ring *ring)
{
- dma_unmap_single(hr_dev->dev, ring->desc_dma_addr,
- ring->desc_num * sizeof(struct hns_roce_cmq_desc),
- DMA_BIDIRECTIONAL);
+ dma_free_coherent(hr_dev->dev,
+ ring->desc_num * sizeof(struct hns_roce_cmq_desc),
+ ring->desc, ring->desc_dma_addr);
ring->desc_dma_addr = 0;
- kfree(ring->desc);
}
static int init_csq(struct hns_roce_dev *hr_dev,
--
2.25.1
On Sun, Sep 26, 2021 at 02:11:15PM +0800, Cai Huoqing wrote:
> Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
> with dma_alloc_coherent/dma_free_coherent() helps to reduce
> code size, and simplify the code, and coherent DMA will not
> clear the cache every time.
>
> Signed-off-by: Cai Huoqing <[email protected]>
> ---
> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
> 1 file changed, 5 insertions(+), 15 deletions(-)
Given I don't see any dma_sync_single calls for this mapping, isn't
this a correctness fix too?
Jason
On Mon, Sep 27, 2021 at 08:59:13AM -0300, Jason Gunthorpe wrote:
> On Sun, Sep 26, 2021 at 02:11:15PM +0800, Cai Huoqing wrote:
> > Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
> > with dma_alloc_coherent/dma_free_coherent() helps to reduce
> > code size, and simplify the code, and coherent DMA will not
> > clear the cache every time.
> >
> > Signed-off-by: Cai Huoqing <[email protected]>
> > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
> > 1 file changed, 5 insertions(+), 15 deletions(-)
>
> Given I don't see any dma_sync_single calls for this mapping, isn't
> this a correctness fix too?
HNS folks?
Jason
On 2021/10/5 3:52, Jason Gunthorpe wrote:
> On Mon, Sep 27, 2021 at 08:59:13AM -0300, Jason Gunthorpe wrote:
>> On Sun, Sep 26, 2021 at 02:11:15PM +0800, Cai Huoqing wrote:
>>> Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
>>> with dma_alloc_coherent/dma_free_coherent() helps to reduce
>>> code size, and simplify the code, and coherent DMA will not
>>> clear the cache every time.
>>>
>>> Signed-off-by: Cai Huoqing <[email protected]>
>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
>>> 1 file changed, 5 insertions(+), 15 deletions(-)
>>
>> Given I don't see any dma_sync_single calls for this mapping, isn't
>> this a correctness fix too?
>
> HNS folks?
>
> Jason
> .
>
Our SoC can keep cache coherent, so there is no exception even if
dma_sync_single* is not called, but the driver should not make
assumptions about SoC.
So using dma_alloc_coherent() instead of kmalloc/dma_map_single()
can simplify the code and achieve the same purpose.
Wenpeng Liang
On 09 10月 21 17:50:50, Wenpeng Liang wrote:
>
>
> On 2021/10/5 3:52, Jason Gunthorpe wrote:
> > On Mon, Sep 27, 2021 at 08:59:13AM -0300, Jason Gunthorpe wrote:
> >> On Sun, Sep 26, 2021 at 02:11:15PM +0800, Cai Huoqing wrote:
> >>> Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
> >>> with dma_alloc_coherent/dma_free_coherent() helps to reduce
> >>> code size, and simplify the code, and coherent DMA will not
> >>> clear the cache every time.
> >>>
> >>> Signed-off-by: Cai Huoqing <[email protected]>
> >>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
> >>> 1 file changed, 5 insertions(+), 15 deletions(-)
> >>
> >> Given I don't see any dma_sync_single calls for this mapping, isn't
> >> this a correctness fix too?
> >
> > HNS folks?
> >
> > Jason
> > .
> >
>
> Our SoC can keep cache coherent, so there is no exception even if
> dma_sync_single* is not called, but the driver should not make
> assumptions about SoC.
>
> So using dma_alloc_coherent() instead of kmalloc/dma_map_single()
> can simplify the code and achieve the same purpose.
>
> Wenpeng Liang
Hi Liang
Thanks for your feedback.
If you think my patch is correct, you can give a Reviewed-by: to it.
You can also give a Tested-by: to it, if the test on hardware was made.
Thanks
Cai
On 2021/10/9 18:42, Cai Huoqing wrote:
> On 09 10月 21 17:50:50, Wenpeng Liang wrote:
>>
>>
>> On 2021/10/5 3:52, Jason Gunthorpe wrote:
>>> On Mon, Sep 27, 2021 at 08:59:13AM -0300, Jason Gunthorpe wrote:
>>>> On Sun, Sep 26, 2021 at 02:11:15PM +0800, Cai Huoqing wrote:
>>>>> Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
>>>>> with dma_alloc_coherent/dma_free_coherent() helps to reduce
>>>>> code size, and simplify the code, and coherent DMA will not
>>>>> clear the cache every time.
>>>>>
>>>>> Signed-off-by: Cai Huoqing <[email protected]>
>>>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
>>>>> 1 file changed, 5 insertions(+), 15 deletions(-)
>>>>
>>>> Given I don't see any dma_sync_single calls for this mapping, isn't
>>>> this a correctness fix too?
>>>
>>> HNS folks?
>>>
>>> Jason
>>> .
>>>
>>
>> Our SoC can keep cache coherent, so there is no exception even if
>> dma_sync_single* is not called, but the driver should not make
>> assumptions about SoC.
>>
>> So using dma_alloc_coherent() instead of kmalloc/dma_map_single()
>> can simplify the code and achieve the same purpose.
>>
>> Wenpeng Liang
>
>
> Hi Liang
>
> Thanks for your feedback.
>
> If you think my patch is correct, you can give a Reviewed-by: to it.
> You can also give a Tested-by: to it, if the test on hardware was made.
>
> Thanks
> Cai
> .
>
Hi, Cai
After testing, it seems ok to me.
Reviewed-by: Wenpeng Liang <[email protected]>
Tested-by: Wenpeng Liang <[email protected]>
Thanks,
Wenpeng
On Sun, Sep 26, 2021 at 02:11:15PM +0800, Cai Huoqing wrote:
> Replacing kmalloc/kfree/dma_map_single/dma_unmap_single()
> with dma_alloc_coherent/dma_free_coherent() helps to reduce
> code size, and simplify the code, and coherent DMA will not
> clear the cache every time.
>
> Signed-off-by: Cai Huoqing <[email protected]>
> Reviewed-by: Wenpeng Liang <[email protected]>
> Tested-by: Wenpeng Liang <[email protected]>
> ---
> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 20 +++++---------------
> 1 file changed, 5 insertions(+), 15 deletions(-)
Applied to for-next, thanks
Jason