Re: RDMA Read: Local protection error

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]

On Thu, May 26, 2016 at 08:19:33PM +0300, Sagi Grimberg wrote:
> 
> >>>>>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)?
> 
> --
> 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);

Sagi,
I'm wondering if allocation from ZONE_DMA is the right way to
replace ZONE_NORMAL allocations.

I don't remember if memory compaction works on ZONE_DMA and it makes
me nervous to think about long and multiple alloc/dealloc MR scenario.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]