From: Jason Gunthorpe <jgg@ziepe.ca>
To: Pasha Tatashin <pasha.tatashin@soleen.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, rientjes@google.com,
dwmw2@infradead.org, baolu.lu@linux.intel.com, joro@8bytes.org,
will@kernel.org, robin.murphy@arm.com, iommu@lists.linux.dev
Subject: Re: [RFC v2 2/3] iommu/intel: synchronize page table map and unmap operations
Date: Mon, 29 Apr 2024 11:56:29 -0300 [thread overview]
Message-ID: <20240429145629.GO231144@ziepe.ca> (raw)
In-Reply-To: <20240426034323.417219-3-pasha.tatashin@soleen.com>
On Fri, Apr 26, 2024 at 03:43:22AM +0000, Pasha Tatashin wrote:
> Since, we are going to update parent page table entries when lower
> level page tables become emtpy and we add them to the free list.
> We need a way to synchronize the operation.
>
> Use domain->pgd_lock to protect all map and unmap operations.
> This is reader/writer lock. At the beginning everything is going to be
> read only mode, however, later, when free page table on unmap is added
> we will add a writer section as well.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++--
> drivers/iommu/intel/iommu.h | 3 +++
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 1bfb6eccad05..8c7e596728b5 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -995,11 +995,13 @@ static void dma_pte_free_pagetable(struct dmar_domain *domain,
> unsigned long last_pfn,
> int retain_level)
> {
> + read_lock(&domain->pgd_lock);
I think no to this.
This is a very performance sensitive path for the DMA API, we really
do want to see a lockless RCU scheme to manage this overhead here.
This would be fine for a VFIO user, which I guess is your use case.
IMHO it is not a good idea to fiddle around the edges like this. We
need to get the iommu code to having shared algorithms for the radix
tree so we can actually implement something good here and share
it. Every driver has the same problem and needs the same complicated
fix.
I keep threatening to work on that but have yet to start..
Jason
next prev parent reply other threads:[~2024-04-29 14:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 3:43 [RFC v2 0/3] iommu/intel: Free empty page tables on unmaps Pasha Tatashin
2024-04-26 3:43 ` [RFC v2 1/3] iommu/intel: Use page->_mapcount to count number of entries in IOMMU Pasha Tatashin
2024-04-26 3:43 ` [RFC v2 2/3] iommu/intel: synchronize page table map and unmap operations Pasha Tatashin
2024-04-29 14:56 ` Jason Gunthorpe [this message]
2024-04-26 3:43 ` [RFC v2 3/3] iommu/intel: free empty page tables on unmaps Pasha Tatashin
2024-04-26 6:42 ` [RFC v2 0/3] iommu/intel: Free " David Hildenbrand
2024-04-26 13:49 ` Pasha Tatashin
2024-04-26 14:39 ` David Hildenbrand
2024-04-26 19:39 ` Matthew Wilcox
2024-04-29 14:46 ` Jason Gunthorpe
Reply instructions:
You may reply publicly 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=20240429145629.GO231144@ziepe.ca \
--to=jgg@ziepe.ca \
--cc=akpm@linux-foundation.org \
--cc=baolu.lu@linux.intel.com \
--cc=dwmw2@infradead.org \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=pasha.tatashin@soleen.com \
--cc=rientjes@google.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).