On Mon, Jun 20, 2016 at 12:37:40PM +0800, Wei Hu (Xavier) wrote: > > > On 2016/6/17 17:58, Leon Romanovsky wrote: > >On Thu, Jun 16, 2016 at 10:35:16PM +0800, Lijun Ou wrote: > >>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 > >>Signed-off-by: Nenglong Zhao > >>Signed-off-by: Lijun Ou > >>--- > ><...> > > > >>+ > >>+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; > >>+} > >>+ > ><...> > > > >>+ > >>+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 > > > >