Re: [PATCH 2/2] IB/core: dma unmap optimizations

[Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]

On 4/15/2015 6:17 PM, Guy Shapiro wrote:
> While unmapping an ODP writable page, the dirty bit of the page is set. In
> order to do so, the head of the compound page is found.
> Currently, the compound head is found even on non-writable pages, where it is
> never used, leading to unnecessary cpu barrier that impacts performance.
>
> This patch moves the search for the compound head to be done only when needed.
>
> Signed-off-by: Guy Shapiro <guysh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Acked-by: Shachar Raindel <raindel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>   drivers/infiniband/core/umem_odp.c |    5 +++--
>   1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> index aba4739..40becdb 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -637,7 +637,6 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
>   		idx = (addr - ib_umem_start(umem)) / PAGE_SIZE;
>   		if (umem->odp_data->page_list[idx]) {
>   			struct page *page = umem->odp_data->page_list[idx];
> -			struct page *head_page = compound_head(page);
>   			dma_addr_t dma = umem->odp_data->dma_list[idx];
>   			dma_addr_t dma_addr = dma & ODP_DMA_ADDR_MASK;
>
> @@ -645,7 +644,8 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
>
>   			ib_dma_unmap_page(dev, dma_addr, PAGE_SIZE,
>   					  DMA_BIDIRECTIONAL);
> -			if (dma & ODP_WRITE_ALLOWED_BIT)
> +			if (dma & ODP_WRITE_ALLOWED_BIT) {
> +				struct page *head_page = compound_head(page);
>   				/*
>   				 * set_page_dirty prefers being called with
>   				 * the page lock. However, MMU notifiers are
> @@ -656,6 +656,7 @@ void ib_umem_odp_unmap_dma_pages(struct ib_umem *umem, u64 virt,
>   				 * be removed.
>   				 */
>   				set_page_dirty(head_page);

Just a nit:

The code is as readable with:
	set_page_dirty(compound_head(page));

and you relax the stack a little bit...

Otherwise:
Looks like a nice optimization.

Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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