Re: RDMA Read: Local protection error
From: Chuck Lever <hidden>
Date: 2016-05-26 17:57:32
On May 26, 2016, at 1:19 PM, Sagi Grimberg [off-list ref] wrote:quoted
quoted
quoted
quoted
quoted
When debugging is disabled, kzalloc returns page-aligned addresses:Is it defined some where that regular kzalloc/kmalloc guaranties to return a page-aligned address as you see in your testing ? if so the debug mode should behave the same. Otherwise we can consider using any flag allocation that can force that if such exists. Let's get other people's input here.My understanding is that the fact that k[mz]alloc() returns a page-aligned buffer if the allocation size is > PAGE_SIZE / 2 is a side effect of the implementation and not something callers of that function should rely on. I think the only assumption k[mz]alloc() callers should rely on is that the allocated memory respects ARCH_KMALLOC_MINALIGN.I agree. mlx4_alloc_priv_pages() is carefully designed to correct the alignment of the buffer, so it already assumes that it is not getting a page-aligned buffer. The alignment isn't the problem here, though. It's that the buffer contains a page-boundary. That is guaranteed to be the case for HCAs that support more than 512 sges, so that will have to be addressed (at least in mlx5).rrr... I think we should make the pages allocations dma coherent in order to fix that... Nice catch Chunk.Does this untested patch help (if so, mlx5 will need an identical patch)?
Thanks, Sagi. Is it safe? Yes, IPoIB and NFS/RDMA work after the patch is applied. Is it effective? I booted with slub_debug=zfpu, and I am not able to reproduce the Local Protection Error WC flushes. However, it's not clear whether that's because DMA-coherent memory is not allocated out of a slab cache, and thus it is not subject to SLUB debugging. <shrug> To test it more thoroughly, mlx4_alloc_priv_pages() could allocate a two-page buffer and push the address of the array up far enough that it would cross the page boundary. I didn't try that. (Also, your patch can delete the definition of MLX4_MR_PAGES_ALIGN) Tested-by: Chuck Lever <redacted>
quoted hunk
--diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h b/drivers/infiniband/hw/mlx4/mlx4_ib.h index ba328177eae9..78e9b3addfea 100644 --- a/drivers/infiniband/hw/mlx4/mlx4_ib.h +++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h@@ -139,7 +139,6 @@ struct mlx4_ib_mr { u32 max_pages; struct mlx4_mr mmr; struct ib_umem *umem; - void *pages_alloc;}; struct mlx4_ib_mw {diff --git a/drivers/infiniband/hw/mlx4/mr.c b/drivers/infiniband/hw/mlx4/mr.c index b04f6238e7e2..becb4a65c755 100644 --- a/drivers/infiniband/hw/mlx4/mr.c +++ b/drivers/infiniband/hw/mlx4/mr.c@@ -278,30 +278,13 @@ mlx4_alloc_priv_pages(struct ib_device *device, int max_pages){ int size = max_pages * sizeof(u64); - int add_size; - int ret; - - add_size = max_t(int, MLX4_MR_PAGES_ALIGN - ARCH_KMALLOC_MINALIGN, 0); - mr->pages_alloc = kzalloc(size + add_size, GFP_KERNEL); - if (!mr->pages_alloc) + mr->pages = dma_alloc_coherent(device->dma_device, size, + &mr->page_map, GFP_KERNEL); + if (!mr->pages) return -ENOMEM; - mr->pages = PTR_ALIGN(mr->pages_alloc, MLX4_MR_PAGES_ALIGN); - - mr->page_map = dma_map_single(device->dma_device, mr->pages, - size, DMA_TO_DEVICE); - - if (dma_mapping_error(device->dma_device, mr->page_map)) { - ret = -ENOMEM; - goto err; - } - return 0; -err: - kfree(mr->pages_alloc); - - return ret; } static void@@ -311,9 +294,8 @@ mlx4_free_priv_pages(struct mlx4_ib_mr *mr) struct ib_device *device = mr->ibmr.device; int size = mr->max_pages * sizeof(u64); - dma_unmap_single(device->dma_device, mr->page_map, - size, DMA_TO_DEVICE); - kfree(mr->pages_alloc); + dma_free_coherent(device->dma_device, size, + mr->pages, mr->page_map); mr->pages = NULL; }}@@ -532,19 +514,8 @@ int mlx4_ib_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents, unsigned int sg_offset){ struct mlx4_ib_mr *mr = to_mmr(ibmr); - int rc; mr->npages = 0; - ib_dma_sync_single_for_cpu(ibmr->device, mr->page_map, - sizeof(u64) * mr->max_pages, - DMA_TO_DEVICE); - - rc = ib_sg_to_pages(ibmr, sg, sg_nents, sg_offset, mlx4_set_page); - - ib_dma_sync_single_for_device(ibmr->device, mr->page_map, - sizeof(u64) * mr->max_pages, - DMA_TO_DEVICE); - - return rc; + return ib_sg_to_pages(ibmr, sg, sg_nents, sg_offset, mlx4_set_page); } -- -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html