All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joerg Roedel <joro@8bytes.org>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>,
	Mel Gorman <mgorman@suse.de>,
	Johannes Weiner <jweiner@redhat.com>,
	Jerome Glisse <jglisse@redhat.com>,
	jroedel@suse.de, Jay.Cornwall@amd.com, Oded.Gabbay@amd.com,
	John.Bridgman@amd.com, Suravee.Suthikulpanit@amd.com,
	ben.sander@amd.com, David Woodhouse <dwmw2@infradead.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	iommu@lists.linux-foundation.org
Subject: Re: [PATCH 1/3] mmu_notifier: Add mmu_notifier_invalidate_range()
Date: Fri, 25 Jul 2014 23:38:06 +0200	[thread overview]
Message-ID: <20140725213806.GN14017@8bytes.org> (raw)
In-Reply-To: <20140725131639.698f18ff@jbarnes-desktop>

On Fri, Jul 25, 2014 at 01:16:39PM -0700, Jesse Barnes wrote:
> > To allow managing external TLBs the MMU-notifiers need to
> > catch the moment when pages are unmapped but not yet freed.
> > This new notifier catches that moment and notifies the
> > interested subsytem when pages that were unmapped are about
> > to be freed. The new notifier will only be called between
> > invalidate_range_start()/end().
> 
> So if we were actually sharing page tables, we should be able to make
> start/end no-ops and just use this new callback, assuming we didn't
> need to do any other serialization or debug stuff, right?

Well, not completly. What you need with this patch-set is a
invalidate_range and an invalidate_end call-back. There are call sites
of the start/end functions where the TLB flush happens after the _end
notifier (or at least can wait until _end is called). I did not add
invalidate_range calls to these places (yet). But you can easily discard
invalidate_range_start, any flush done in there is useless with shared
page-tables.

I though about removing the need for invalidate_range_end too when
writing the patches, and possible solutions are

	1) Add mmu_notifier_invalidate_range() to all places where
	   start/end is called too. This might add some unnecessary
	   overhead.

	2) Call the invalidate_range() call-back from the
	   mmu_notifier_invalidate_range_end too.

	3) Just let the user register the same function for
	   invalidate_range and invalidate_range_end

I though that option 1) adds overhead that is not needed (but it might
not be too bad, the overhead is an additional iteration over the
mmu_notifer list when there are no call-backs registered).

Option 2) might also be overhead if a user registers different functions
for invalidate_range() and invalidate_range_end(). In the end I came to
the conclusion that option 3) is the best one from an overhead POV.

But probably targeting better usability with one of the other options is
a better choice? I am open for thoughts and suggestions on that.


	Joerg

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Joerg Roedel <joro@8bytes.org>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>,
	Mel Gorman <mgorman@suse.de>,
	Johannes Weiner <jweiner@redhat.com>,
	Jerome Glisse <jglisse@redhat.com>,
	jroedel@suse.de, Jay.Cornwall@amd.com, Oded.Gabbay@amd.com,
	John.Bridgman@amd.com, Suravee.Suthikulpanit@amd.com,
	ben.sander@amd.com, David Woodhouse <dwmw2@infradead.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	iommu@lists.linux-foundation.org
Subject: Re: [PATCH 1/3] mmu_notifier: Add mmu_notifier_invalidate_range()
Date: Fri, 25 Jul 2014 23:38:06 +0200	[thread overview]
Message-ID: <20140725213806.GN14017@8bytes.org> (raw)
In-Reply-To: <20140725131639.698f18ff@jbarnes-desktop>

On Fri, Jul 25, 2014 at 01:16:39PM -0700, Jesse Barnes wrote:
> > To allow managing external TLBs the MMU-notifiers need to
> > catch the moment when pages are unmapped but not yet freed.
> > This new notifier catches that moment and notifies the
> > interested subsytem when pages that were unmapped are about
> > to be freed. The new notifier will only be called between
> > invalidate_range_start()/end().
> 
> So if we were actually sharing page tables, we should be able to make
> start/end no-ops and just use this new callback, assuming we didn't
> need to do any other serialization or debug stuff, right?

Well, not completly. What you need with this patch-set is a
invalidate_range and an invalidate_end call-back. There are call sites
of the start/end functions where the TLB flush happens after the _end
notifier (or at least can wait until _end is called). I did not add
invalidate_range calls to these places (yet). But you can easily discard
invalidate_range_start, any flush done in there is useless with shared
page-tables.

I though about removing the need for invalidate_range_end too when
writing the patches, and possible solutions are

	1) Add mmu_notifier_invalidate_range() to all places where
	   start/end is called too. This might add some unnecessary
	   overhead.

	2) Call the invalidate_range() call-back from the
	   mmu_notifier_invalidate_range_end too.

	3) Just let the user register the same function for
	   invalidate_range and invalidate_range_end

I though that option 1) adds overhead that is not needed (but it might
not be too bad, the overhead is an additional iteration over the
mmu_notifer list when there are no call-backs registered).

Option 2) might also be overhead if a user registers different functions
for invalidate_range() and invalidate_range_end(). In the end I came to
the conclusion that option 3) is the best one from an overhead POV.

But probably targeting better usability with one of the other options is
a better choice? I am open for thoughts and suggestions on that.


	Joerg


  parent reply	other threads:[~2014-07-25 21:38 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-24 14:35 [PATCH 0/3] mmu_notifier: Allow to manage CPU external TLBs Joerg Roedel
2014-07-24 14:35 ` Joerg Roedel
2014-07-24 14:35 ` Joerg Roedel
     [not found] ` <1406212541-25975-1-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2014-07-24 14:35   ` [PATCH 1/3] mmu_notifier: Add mmu_notifier_invalidate_range() Joerg Roedel
2014-07-24 14:35     ` Joerg Roedel
2014-07-24 14:35     ` Joerg Roedel
     [not found]     ` <1406212541-25975-2-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2014-07-25 20:16       ` Jesse Barnes
2014-07-25 20:16         ` Jesse Barnes
2014-07-25 20:16         ` Jesse Barnes
2014-07-25 20:43         ` Jerome Glisse
2014-07-25 20:43           ` Jerome Glisse
2014-07-25 20:43           ` Jerome Glisse
2014-07-25 21:38         ` Joerg Roedel [this message]
2014-07-25 21:38           ` Joerg Roedel
     [not found]           ` <20140725213806.GN14017-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2014-07-25 21:42             ` Jesse Barnes
2014-07-25 21:42               ` Jesse Barnes
2014-07-25 21:42               ` Jesse Barnes
2014-07-25 21:57               ` Joerg Roedel
2014-07-25 21:57                 ` Joerg Roedel
2014-07-25 21:47           ` Jerome Glisse
2014-07-25 21:47             ` Jerome Glisse
2014-07-25 21:47             ` Jerome Glisse
2014-07-24 14:35   ` [PATCH 2/3] mmu_notifier: Call mmu_notifier_invalidate_range() from VMM Joerg Roedel
2014-07-24 14:35     ` Joerg Roedel
2014-07-24 14:35     ` Joerg Roedel
2014-07-24 14:35   ` [PATCH 3/3] mmu_notifier: Add the call-back for mmu_notifier_invalidate_range() Joerg Roedel
2014-07-24 14:35     ` Joerg Roedel
2014-07-24 14:35     ` Joerg Roedel
2014-07-24 14:44 ` [PATCH 0/3] mmu_notifier: Allow to manage CPU external TLBs Andrea Arcangeli
2014-07-24 14:44   ` Andrea Arcangeli
2014-07-24 23:33 ` Andrew Morton
2014-07-24 23:33   ` Andrew Morton
2014-07-25  3:10   ` Sander, Ben
2014-07-25  3:10     ` Sander, Ben
     [not found]   ` <20140724163303.df34065a3c3b26c0a4b3bab1-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2014-07-25  7:47     ` Joerg Roedel
2014-07-25  7:47       ` Joerg Roedel
2014-07-25  7:47       ` Joerg Roedel
  -- strict thread matches above, loose matches on Subject: below --
2014-07-29 16:18 [PATCH 0/3 v2] " Joerg Roedel
2014-07-29 16:18 ` [PATCH 1/3] mmu_notifier: Add mmu_notifier_invalidate_range() Joerg Roedel
2014-07-29 16:18   ` Joerg Roedel
2014-09-09 15:43 [PATCH 0/3 v3] mmu_notifier: Allow to manage CPU external TLBs Joerg Roedel
2014-09-09 15:43 ` [PATCH 1/3] mmu_notifier: Add mmu_notifier_invalidate_range() Joerg Roedel
2014-09-09 15:43   ` Joerg Roedel
2014-09-09 15:43   ` Joerg Roedel
2014-10-28 17:13 [PATCH 0/3 v4] mmu_notifier: Allow to manage CPU external TLBs Joerg Roedel
     [not found] ` <1414516440-910-1-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2014-10-28 17:13   ` [PATCH 1/3] mmu_notifier: Add mmu_notifier_invalidate_range() Joerg Roedel
2014-10-28 17:13     ` Joerg Roedel
2014-10-28 17:13     ` Joerg Roedel

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=20140725213806.GN14017@8bytes.org \
    --to=joro@8bytes.org \
    --cc=Jay.Cornwall@amd.com \
    --cc=John.Bridgman@amd.com \
    --cc=Oded.Gabbay@amd.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ben.sander@amd.com \
    --cc=dwmw2@infradead.org \
    --cc=hughd@google.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jbarnes@virtuousgeek.org \
    --cc=jglisse@redhat.com \
    --cc=jroedel@suse.de \
    --cc=jweiner@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=riel@redhat.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.