LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>,
	joro@8bytes.org, iommu@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org
Cc: tomasz.nowicki@cavium.com, jnair@caviumnetworks.com,
	Robert.Richter@cavium.com, Vadim.Lomovtsev@cavium.com,
	Jan.Glauber@cavium.com, gklkml16@gmail.com
Subject: Re: [PATCH] iommu/iova: Optimise attempts to allocate iova from 32bit address range
Date: Thu, 9 Aug 2018 17:24:47 +0100
Message-ID: <318f9118-df78-e78f-1ae2-72a33cbee28e@arm.com> (raw)
In-Reply-To: <20180807085437.15965-1-ganapatrao.kulkarni@cavium.com>

On 07/08/18 09:54, Ganapatrao Kulkarni wrote:
> As an optimisation for PCI devices, there is always first attempt
> been made to allocate iova from SAC address range. This will lead
> to unnecessary attempts/function calls, when there are no free ranges
> available.
> 
> This patch optimises by adding flag to track previous failed attempts
> and avoids further attempts until replenish happens.

Agh, what I overlooked is that this still suffers from the original 
problem, wherein a large allocation which fails due to fragmentation 
then blocks all subsequent smaller allocations, even if they may have 
succeeded.

For a minimal change, though, what I think we could do is instead of 
just having a flag, track the size of the last 32-bit allocation which 
failed. If we're happy to assume that nobody's likely to mix aligned and 
unaligned allocations within the same domain, then that should be 
sufficiently robust whilst being no more complicated than this version, 
i.e. (modulo thinking up a better name for it):

> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
> This patch is based on comments from Robin Murphy <robin.murphy@arm.com>
> for patch [1]
> 
> [1] https://lkml.org/lkml/2018/4/19/780
> 
>   drivers/iommu/iova.c | 11 ++++++++++-
>   include/linux/iova.h |  1 +
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 83fe262..d97bb5a 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -56,6 +56,7 @@ init_iova_domain(struct iova_domain *iovad, unsigned long granule,
>   	iovad->granule = granule;
>   	iovad->start_pfn = start_pfn;
>   	iovad->dma_32bit_pfn = 1UL << (32 - iova_shift(iovad));
> +	iovad->free_32bit_pfns = true;

	iovad->max_32bit_free = iovad->dma_32bit_pfn;

>   	iovad->flush_cb = NULL;
>   	iovad->fq = NULL;
>   	iovad->anchor.pfn_lo = iovad->anchor.pfn_hi = IOVA_ANCHOR;
> @@ -139,8 +140,10 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, struct iova *free)
>   
>   	cached_iova = rb_entry(iovad->cached32_node, struct iova, node);
>   	if (free->pfn_hi < iovad->dma_32bit_pfn &&
> -	    free->pfn_lo >= cached_iova->pfn_lo)
> +	    free->pfn_lo >= cached_iova->pfn_lo) {
>   		iovad->cached32_node = rb_next(&free->node);
> +		iovad->free_32bit_pfns = true;

		iovad->max_32bit_free = iovad->dma_32bit_pfn;

> +	}
>   
>   	cached_iova = rb_entry(iovad->cached_node, struct iova, node);
>   	if (free->pfn_lo >= cached_iova->pfn_lo)
> @@ -290,6 +293,10 @@ alloc_iova(struct iova_domain *iovad, unsigned long size,
>   	struct iova *new_iova;
>   	int ret;
>   
> +	if (limit_pfn <= iovad->dma_32bit_pfn &&
> +			!iovad->free_32bit_pfns)

			size >= iovad->max_32bit_free)

> +		return NULL;
> +
>   	new_iova = alloc_iova_mem();
>   	if (!new_iova)
>   		return NULL;
> @@ -299,6 +306,8 @@ alloc_iova(struct iova_domain *iovad, unsigned long size,
>   
>   	if (ret) {
>   		free_iova_mem(new_iova);
> +		if (limit_pfn <= iovad->dma_32bit_pfn)
> +			iovad->free_32bit_pfns = false;

			iovad->max_32bit_free = size;

What do you think?

Robin.

>   		return NULL;
>   	}
>   
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 928442d..3810ba9 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -96,6 +96,7 @@ struct iova_domain {
>   						   flush-queues */
>   	atomic_t fq_timer_on;			/* 1 when timer is active, 0
>   						   when not */
> +	bool	free_32bit_pfns;
>   };
>   
>   static inline unsigned long iova_size(struct iova *iova)
> 

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-07  8:54 Ganapatrao Kulkarni
2018-08-09 16:24 ` Robin Murphy [this message]
2018-08-09 17:49   ` Ganapatrao Kulkarni
2018-08-09 20:43     ` Robin Murphy
2018-08-10  9:24       ` Ganapatrao Kulkarni
2018-08-10  9:49         ` Robin Murphy
2018-08-10 10:01           ` Ganapatrao Kulkarni

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=318f9118-df78-e78f-1ae2-72a33cbee28e@arm.com \
    --to=robin.murphy@arm.com \
    --cc=Jan.Glauber@cavium.com \
    --cc=Robert.Richter@cavium.com \
    --cc=Vadim.Lomovtsev@cavium.com \
    --cc=ganapatrao.kulkarni@cavium.com \
    --cc=gklkml16@gmail.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jnair@caviumnetworks.com \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tomasz.nowicki@cavium.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox