Re: [PATCH] mmu notifiers #v6
> I really want suggestions on Jack's concern about issuing an > invalidate per pte entry or per-pte instead of per-range. I'll answer > that in a separate email. For KVM my patch is already close to optimal > because each single spte invalidate requires a fixed amount of work, > but for GRU a large invalidate-range would be more efficient. > > To address the GRU _valid_ concern, I can create a second version of > my patch with range_begin/end instead of invalidate_pages, that still I don't know how much significance to place on this data, but it is a real data point. I ran the GRU regression test suite on kernels with both types of mmu_notifiers. The kernel/driver using Christoph's patch had 1/7 the number of TLB invalidates as Andrea's patch. This reduction is due to both differences I mentioned yesterday: - different location of callout for address space teardown - range callouts Unfortunately, the current driver does not allow me to quantify which of the differences is most significant. Also, I'll try to post the driver within the next few days. It is still in development but it compiles and can successfully run most workloads on a system simulator. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mmu notifiers #v6
On Wed, Feb 20, 2008 at 11:39:42AM +0100, Andrea Arcangeli wrote: > Given Nick's comments I ported my version of the mmu notifiers to > latest mainline. There are no known bugs AFIK and it's obviously safe > (nothing is allowed to schedule inside rcu_read_lock taken by > mmu_notifier() with my patch). > I ported the GRU driver to use the latest #v6 patch and ran a series of tests on it using our system simulator. The simulator is slow so true stress or swapping is not possible - at least within a finite amount of time. Functionally, the #v6 patch seems to work for the GRU. However, I did notice two significant differences that make the #v6 performance worse for the GRU than Christoph's patch. I think one difference is easily fixable but the other is more difficult: - the location of the mmu_notifier_release() callout is at a different place in the 2 patches. Christoph has the callout BEFORE the call to unmap_vmas() whereas you have it AFTER. The net result is that the GRU does a LOT of 1-page TLB flushes during process teardown. These flushes are not done with Christops's patch. - the range callouts in Christoph's patch benefit the GRU because multiple TLB entries can be flushed with a single GRU instruction (the GRU hardware supports a range flush using a vaddr & length). The #v6 patch does a TLB flush for each page in the range. Flushing on the GRU is slow so being able to flush multiple pages with a single request is a benefit. Seems like the latter difference could be significant for other users of mmu notifiers. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] my mmu notifiers
On Wed, Feb 20, 2008 at 12:11:57AM +0100, Nick Piggin wrote: > On Tue, Feb 19, 2008 at 02:58:51PM +0100, Andrea Arcangeli wrote: > > On Tue, Feb 19, 2008 at 09:43:57AM +0100, Nick Piggin wrote: > > > anything when changing the pte to be _more_ permissive, and I don't > > > > Note that in my patch the invalidate_pages in mprotect can be > > trivially switched to a mprotect_pages with proper params. This will > > prevent page faults completely in the secondary MMU (there will only > > be tlb misses after the tlb flush just like for the core linux pte), > > and it'll allow all the secondary MMU pte blocks (512/1024 at time > > with my PT lock design) to be updated to have proper permissions > > matching the core linux pte. > > Sorry, I realise I still didn't get this through my head yet (and also > have not seen your patch recently). So I don't know exactly what you > are doing... > > But why does _anybody_ (why does Christoph's patches) need to invalidate > when they are going to be more permissive? This should be done lazily by > the driver, I would have thought. Agree. Although for most real applications, the performance difference is probably negligible. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] my mmu notifiers
> On Tue, Feb 19, 2008 at 02:58:51PM +0100, Andrea Arcangeli wrote: > > understand the need for invalidate_begin/invalidate_end pairs at all. > > The need of the pairs is crystal clear to me: range_begin is needed > for GRU _but_only_if_ range_end is called after releasing the > reference that the VM holds on the page. _begin will flush the GRU tlb > and at the same time it will take a mutex that will block further GRU > tlb-miss-interrupts (no idea how they manange those nightmare locking, > I didn't even try to add more locking to KVM and I get away with the > fact KVM takes the pin on the page itself). As it turns out, no actual mutex is required. _begin_ simply increments a count of active range invalidates, _end_ decrements the count. New TLB dropins are deferred while range callouts are active. This would appear to be racy but the GRU has special hardware that simplifies locking. When the GRU sees a TLB invalidate, all outstanding misses & potentially inflight TLB dropins are marked by the GRU with a "kill" bit. When the dropin finally occurs, the dropin is ignored & the instruction is simply restarted. The instruction will fault again & the TLB dropin will be repeated. This is optimized for the case where invalidates are rare - true for users of the GRU. In general, though, I agree. Most users of mmu_notifiers would likely required a mutex or something equivalent. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/6] MMU Notifiers V6
> GRU > - Simple additional hardware TLB (possibly covering multiple instances of > Linux) > - Needs TLB shootdown when the VM unmaps pages. > - Determines page address via follow_page (from interrupt context) but can > fall back to get_user_pages(). > - No page reference possible since no page status is kept.. I applied the latest mmuops patch to a 2.6.24 kernel & updated the GRU driver to use it. As far as I can tell, everything works ok. Although more testing is needed, all current tests of driver functionality are working on both a system simulator and a hardware simulator. The driver itself is still a few weeks from being ready to post but I can send code fragments of the portions related to mmuops or external TLB management if anyone is interested. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mmu notifiers #v5
On Sun, Feb 03, 2008 at 03:17:04AM +0100, Andrea Arcangeli wrote: > On Fri, Feb 01, 2008 at 11:23:57AM -0800, Christoph Lameter wrote: > > Yes so your invalidate_range is still some sort of dysfunctional > > optimization? Gazillions of invalidate_page's will have to be executed > > when tearing down large memory areas. > > I don't know if gru can flush the external TLB references with a > single instruction like the cpu can do by overwriting %cr3. As far as The mechanism obviously is a little different, but the GRU can flush an external TLB in a single read/write/flush of a cacheline (should take a few 100 nsec). Typically, an application uses a single GRU. Threaded applications, however, could use one GRU per thread, so it is possible that multiple TLBs must be flushed. In some cases, the external flush can be avoided if the GRU is not currently in use by the thread. Also, most (but not all) applications that use the GRU do not usually do anything that requires frequent flushing (fortunately). The GRU is intended for HPC-like applications. These don't usually do frequent map/unmap operations or anything else that requires a lot of flushes. I expect that KVM is a lot different. I have most of the GRU code working with the latest mmuops patch. I still have a list of loose ends that I'll get to next week. The most important is the exact handling of the range invalidates. The code that I currently have works (mostly) but has a few endcases that will cause problems. Once I finish, I'll be glad to send you snippets of the code (or all of it) if you would like to take a look. > vmx/svm are concerned, you got to do some fixed amount of work on the > rmap structures and each spte, for each 4k invalidated regardless of > page/pages/range/ranges. You can only share the cost of the lock and > of the memslot lookup, so you lookup and take the lock once and you > drop 512 sptes instead of just 1. Similar to what we do in the main > linux VM by taking the PT lock for every 512 sptes. > > So you worry about gazillions of invalidate_page without taking into > account that even if you call invalidate_range_end(0, -1ULL) KVM will > have to mangle over 4503599627370496 rmap entries anyway. Yes calling > 1 invalidate_range_end instead of 4503599627370496 invalidate_page, > will be faster, but not much faster. For KVM it remains an O(N) > operation, where N is the number of pages. I'm not so sure we need to > worry about my invalidate_pages being limited to 512 entries. > > Perhaps GRU is different, I'm just describing the KVM situation here. > > As far as KVM is concerned it's more sensible to be able to scale when > there are 1024 kvm threads on 1024 cpus and each kvm-vcpu is accessing > a different guest physical page (especially when new chunks of memory > are allocated for the first time) that won't collide the whole time on > naturally aligned 2M chunks of virtual addresses. > > > And that would not be enough to hold of new references? With small tweaks > > this should work with a common scheme. We could also redefine the role > > of _start and _end slightly to just require that the refs are removed when > > _end completes. That would allow the KVM page count ref to work as is now > > and would avoid the individual invalidate_page() callouts. > > I can already only use _end and ignore _start, only remaining problem > is this won't be 100% coherent (my patch is 100% coherent thanks to PT > lock implicitly serializing follow_page/get_user_pages of the KVM/GRU > secondary MMU faults). My approach give a better peace of mind with > full scalability and no lock recursion when it's the KVM page fault > that invokes get_user_pages that invokes the linux page fault routines > that will try to execute _start taking the lock to block the page > fault that is already running... > > > > > The GRU has no page table on its own. It populates TLB entries on > > > > demand > > > > using the linux page table. There is no way it can figure out when to > > > > drop page counts again. The invalidate calls are turned directly into > > > > tlb > > > > flushes. > > > > > > Yes, this is why it can't serialize follow_page with only the PT lock > > > with your patch. KVM may do it once you add start,end to range_end > > > only thanks to the additional pin on the page. > > > > Right but that pin requires taking a refcount which we cannot do. > > GRU can use my patch without the pin. XPMEM obviously can't use my > patch as my invalidate_page[s] are under the PT lock (a feature to fit > GRU/KVM in the simplest way), this is why an incremental patch adding > invalidate_range_start/end would be required to support XPMEM too. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] mmu_notifier: Core code
On Thu, Jan 31, 2008 at 06:39:19PM -0800, Christoph Lameter wrote: > On Thu, 31 Jan 2008, Robin Holt wrote: > > > Jack has repeatedly pointed out needing an unregister outside the > > mmap_sem. I still don't see the benefit to not having the lock in the mm. > > I never understood why this would be needed. ->release removes the > mmu_notifier right now. Christoph - We discussed this earlier this week. Here is part of the mail: > > There currently is no __mmu_notifier_unregister(). Oversite??? > > No need. mmu_notifier_release implies an unregister and I think that is > the most favored way to release resources since it deals with the RCU > quiescent period. I currently unlink the mmu_notifier when the last GRU mapping is closed. For example, if a user does a: gru_create_context(); ... gru_destroy_context(); the mmu_notifier is unlinked and all task tables allocated by the driver are freed. Are you suggesting that I leave tables allocated until the task terminates?? Why is that better? What problem do I cause by trying to free tables as soon as they are not needed? --- > Christoph responded: > > the mmu_notifier is unlinked and all task tables allocated > > by the driver are freed. Are you suggesting that I leave tables > > allocated until the task terminates?? > > You need to leave the mmu_notifier structure allocated until the next > quiescent rcu period unless you use the release notifier. I assumed that I would need to use call_rcu() or synchronize_rcu() before the table is actually freed. That's still on my TODO list. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] mmu_notifier: Core code
On Thu, Jan 31, 2008 at 08:24:44PM -0600, Robin Holt wrote: > On Thu, Jan 31, 2008 at 07:56:12PM -0600, Jack Steiner wrote: > > > @@ -2033,6 +2034,7 @@ void exit_mmap(struct mm_struct *mm) > > > unsigned long end; > > > > > > /* mm's last user has gone, and its about to be pulled down */ > > > + mmu_notifier(invalidate_all, mm, 0); > > > arch_exit_mmap(mm); > > > > > > > The name of the "invalidate_all" callout is not very descriptive. > > Why not use "exit_mmap". That matches the function name, the arch callout > > and better describes what is happening. > > This got removed in a later patch. We now only do the release. Found it, thanks. Christoph, is it time to post a new series of patches? I've got as many fixup patches as I have patches in the original posting. -- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/3] mmu_notifier: Core code
> @@ -2033,6 +2034,7 @@ void exit_mmap(struct mm_struct *mm) > unsigned long end; > > /* mm's last user has gone, and its about to be pulled down */ > + mmu_notifier(invalidate_all, mm, 0); > arch_exit_mmap(mm); > The name of the "invalidate_all" callout is not very descriptive. Why not use "exit_mmap". That matches the function name, the arch callout and better describes what is happening. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/6] mmu_notifier: Callbacks to invalidate address ranges
On Wed, Jan 30, 2008 at 11:41:29AM -0800, Christoph Lameter wrote: > On Wed, 30 Jan 2008, Jack Steiner wrote: > > > I see what you mean. I need to review to mail to see why this changed > > but in the original discussions with Christoph, the invalidate_range > > callouts were suppose to be made BEFORE the pages were put on the freelist. > > Seems that we cannot rely on the invalidate_ranges for correctness at all? > We need to have invalidate_page() always. invalidate_range() is only an > optimization. > I don't understand your point "an optimization". How would invalidate_range as currently defined be correctly used? It _looks_ like it would work only if xpmem/gru/etc takes a refcnt on the page & drops it when invalidate_range is called. That may work (not sure) for xpmem but not for the GRU. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 1/6] mmu_notifier: Core code
On Wed, Jan 30, 2008 at 04:37:49PM +0100, Andrea Arcangeli wrote:
> On Tue, Jan 29, 2008 at 06:29:10PM -0800, Christoph Lameter wrote:
> > +void mmu_notifier_release(struct mm_struct *mm)
> > +{
> > + struct mmu_notifier *mn;
> > + struct hlist_node *n, *t;
> > +
> > + if (unlikely(!hlist_empty(&mm->mmu_notifier.head))) {
> > + rcu_read_lock();
> > + hlist_for_each_entry_safe_rcu(mn, n, t,
> > + &mm->mmu_notifier.head, hlist) {
> > + hlist_del_rcu(&mn->hlist);
>
> This will race and kernel crash against mmu_notifier_register in
> SMP. You should resurrect the per-mmu_notifier_head lock in my last
> patch (except it can be converted from a rwlock_t to a regular
> spinlock_t) and drop the mmap_sem from
> mmu_notifier_register/unregister.
Agree.
That will also resolve the problem we discussed yesterday.
I want to unregister my mmu_notifier when a GRU segment is
unmapped. This would not necessarily be at task termination.
However, the mmap_sem is already held for write by the core
VM at the point I would call the unregister function.
Currently, there is no __mmu_notifier_unregister() defined.
Moving to a different lock solves the problem.
-- jack
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/6] mmu_notifier: Callbacks to invalidate address ranges
On Wed, Jan 30, 2008 at 02:37:20PM +0100, Andrea Arcangeli wrote: > On Tue, Jan 29, 2008 at 06:28:05PM -0600, Jack Steiner wrote: > > On Tue, Jan 29, 2008 at 04:20:50PM -0800, Christoph Lameter wrote: > > > On Wed, 30 Jan 2008, Andrea Arcangeli wrote: > > > > > > > > invalidate_range after populate allows access to memory for which > > > > > ptes > > > > > were zapped and the refcount was released. > > > > > > > > The last refcount is released by the invalidate_range itself. > > > > > > That is true for your implementation and to address Robin's issues. Jack: > > > Is that true for the GRU? > > > > I'm not sure I understand the question. The GRU never (currently) takes > > a reference on a page. It has no mechanism for tracking pages that > > were exported to the external TLBs. > > If you don't have a pin, then things like invalidate_range in > remap_file_pages can't be safe as writes through the external TLBs can > keep going on pages in the freelist. For you to be safe w/o a > page-pin, you need to return in the direction of invalidate_page > inside ptep_clear_flush (or anyway before > page_cache_release/__free_page/put_page...). You're generally not safe > with any invalidate_range that may run after the page pointed by the > pte has been freed (or can be freed by the VM anytime because of being > unpinned cache). Yuck I see what you mean. I need to review to mail to see why this changed but in the original discussions with Christoph, the invalidate_range callouts were suppose to be made BEFORE the pages were put on the freelist. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/6] mmu_notifier: Callbacks to invalidate address ranges
On Tue, Jan 29, 2008 at 04:20:50PM -0800, Christoph Lameter wrote: > On Wed, 30 Jan 2008, Andrea Arcangeli wrote: > > > > invalidate_range after populate allows access to memory for which ptes > > > were zapped and the refcount was released. > > > > The last refcount is released by the invalidate_range itself. > > That is true for your implementation and to address Robin's issues. Jack: > Is that true for the GRU? I'm not sure I understand the question. The GRU never (currently) takes a reference on a page. It has no mechanism for tracking pages that were exported to the external TLBs. --- jack -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] IDE: don't offer IDE_GENERIC on ia64
(resend with correct cc list). On Thu, Aug 11, 2005 at 02:24:43PM -0600, Bjorn Helgaas wrote: > IA64 boxes only have PCI IDE devices, so there's no need to blindly poke > around in I/O port space. Poking at things that don't exist causes MCAs > on HP ia64 systems. H, can you change the test so that IDE_GENERIC is off by default but can still be enabled on IA64. We use the generic IDE driver in our simulator environment - yes, it really works. Longer term, I'll see if it is feasible to switch to using a PCI device. > > Signed-off-by: Bjorn Helgaas <[EMAIL PROTECTED]> > > Index: work-vga/drivers/ide/Kconfig > === > --- work-vga.orig/drivers/ide/Kconfig 2005-08-10 14:57:47.0 -0600 > +++ work-vga/drivers/ide/Kconfig 2005-08-10 14:58:02.0 -0600 > @@ -276,6 +276,7 @@ > > config IDE_GENERIC > tristate "generic/default IDE chipset support" > + depends on !IA64 > default y > help > If unsure, say Y. > - > To unsubscribe from this list: send the line "unsubscribe linux-ia64" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Thanks Jack Steiner ([EMAIL PROTECTED]) 651-683-5302 Principal Engineer SGI - Silicon Graphics, Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] IDE: don't offer IDE_GENERIC on ia64
On Thu, Aug 11, 2005 at 03:42:07PM -0600, Bjorn Helgaas wrote: > On Thursday 11 August 2005 2:56 pm, Jeff Garzik wrote: > > Bjorn Helgaas wrote: > > > On Thursday 11 August 2005 2:36 pm, Jeff Garzik wrote: > > >>Bjorn Helgaas wrote: > > >>> config IDE_GENERIC > > >>> tristate "generic/default IDE chipset support" > > >>>+depends on !IA64 > > >> > > >>hm. Are you POSITIVE that the legacy IDE ports are never enabled? > > >> > > >>In modern Intel chipsets, this still occurs with e.g. combined mode. > > > > > > I don't know about combined mode. If the legacy IDE ports are > > > enabled, shouldn't they be described via ACPI, and hence usable > > > via the ide_pnp - PNPACPI - ACPI path? > > > > No idea... that's more for platform IA64 people to answer :/ > > OK, well, I assert that failure to describe an IDE device that > uses legacy ports would be an ACPI defect. > > Tony, others, does this change give you any heartburn? On > the 460GX and 870 boxes I have, IDE is a PCI device. > > (I have been told that the SGI ia64 simulator depends on > IDE_GENERIC. But it really should make the IDE device > appear in PCI (or describe it via ACPI)). Yes - we use IDE_GENERIC for the simulator. ACPI at this point is not an option for us because (unfortunately) SN is not ACPI compliant. We plan additional ACPI support in the future but it will take a while to get there. I'm checking to see if we have any short term alternatives. More later -- Thanks Jack Steiner ([EMAIL PROTECTED]) 651-683-5302 Principal Engineer SGI - Silicon Graphics, Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
