Re: [PATCH v10 08/22] IB/hns: Add icm support

On Mon, Jun 20, 2016 at 12:37:40PM +0800, Wei Hu (Xavier) wrote:

On 2016/6/17 17:58, Leon Romanovsky wrote:

quoted

On Thu, Jun 16, 2016 at 10:35:16PM +0800, Lijun Ou wrote:

quoted

This patch mainly added icm support for RoCE. It initializes icm
which managers the relative memory blocks for RoCE. The data
structures of RoCE will be located in it. For example, CQ table,
QP table and MTPT table so on.

Signed-off-by: Wei Hu <redacted>
Signed-off-by: Nenglong Zhao <zhaonenglong-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Lijun Ou <redacted>
---

<...>

quoted

+
+static int hns_roce_alloc_icm_pages(struct scatterlist *mem, int order,
+				    gfp_t gfp_mask)
+{
+	struct page *page;
+
+	page = alloc_pages(gfp_mask, order);
+	if (!page)
+		return -ENOMEM;
+
+	sg_set_page(mem, page, PAGE_SIZE << order, 0);
+
+	return 0;
+}
+
+static int hns_roce_alloc_icm_coherent(struct device *dev,
+				       struct scatterlist *mem, int order,
+				       gfp_t gfp_mask)
+{
+	void *buf = dma_alloc_coherent(dev, PAGE_SIZE << order,
+				       &sg_dma_address(mem), gfp_mask);
+	if (!buf)
+		return -ENOMEM;
+
+	sg_set_buf(mem, buf, PAGE_SIZE << order);
+	WARN_ON(mem->offset);
+	sg_dma_len(mem) = PAGE_SIZE << order;
+	return 0;
+}
+

<...>

quoted

+
+static void hns_roce_free_icm_pages(struct hns_roce_dev *hr_dev,
+				    struct hns_roce_icm_chunk *chunk)
+{
+	int i;
+
+	if (chunk->nsg > 0)
+		dma_unmap_sg(&hr_dev->pdev->dev, chunk->mem, chunk->npages,
+			     DMA_BIDIRECTIONAL);
+
+	for (i = 0; i < chunk->npages; ++i)
+		__free_pages(sg_page(&chunk->mem[i]),
+			     get_order(chunk->mem[i].length));

You used alloc_pages for this allocation, so why are you using
__free_pages instead of free_pages?

Hi, Leon
    The function prototype of these functions as below：
        static inline struct page * alloc_pages(gfp_t gfp_mask, unsigned int
order);
        void free_pages(unsigned long addr, unsigned int order);
        void __free_pages(struct page *page, unsigned int order);

    The type of the first parameter of free_pages is same with the type of
return value of alloc_pages.
    Maybe it is better to call __free_pages to release memory that allocated
by calling alloc_pages.

OK, I see.

Another question which you didn't answer [1].

"I wonder if you have the same needs for ICM as it is in mlx4 device.
Do you have firmware?"

[1] http://marc.info/?l=linux-rdma&m=146545553104913&w=2

Regards
Wei Hu

quoted