2.6.19-rc1: vm stuff

Fix the race between invalidate_inode_pages and do_no_page.

Fix the race between invalidate_inode_pages and do_no_page.

Quoted 2 lines · On Sat, 7 Oct 2006 15:06:21 +0200 (CEST) Nick Piggin <[email protected]> wrote:

Fix the race between invalidate_inode_pages and do_no_page.

- In do_no_page() there's a `goto retry' where we appear to have forgotten to (conditionally) unlock the page.

- In do_no_page() the COW-break code seem to have forgotten to (conditionally) unlock the page which it just COWed?

- In do_no_page(), the unlock_page() which _is_ there doesn't test VM_CAN_INVALIDATE before deciding to unlock the page.

Add an arch_alloc_page to match arch_free_page.

Quoted 2 lines · On Sat, 7 Oct 2006 15:06:04 +0200 (CEST) Nick Piggin <[email protected]> wrote:

Add an arch_alloc_page to match arch_free_page.

umm.. why?

Quoted 9 lines

Quoted 2 lines · On Sat, 7 Oct 2006 15:06:04 +0200 (CEST) Nick Piggin <[email protected]> wrote:

Add an arch_alloc_page to match arch_free_page.

umm.. why?

I had a future patch to more kernel_map_pages into it, but couldn't decide if that's a generic kernel feature that is only implemented in 2 architectures, or an architecture speicifc feature. So I left it out.

But at least Martin wanted a hook here for his volatile pages patches, so I thought I'd submit this patch anyway.

Quoted 10 lines · Nick Piggin

Quoted 9 lines

Quoted 2 lines · On Sat, 7 Oct 2006 15:06:04 +0200 (CEST) Nick Piggin <[email protected]> wrote:

Add an arch_alloc_page to match arch_free_page.

umm.. why?

Quoted 6 lines

I had a future patch to more kernel_map_pages into it, but couldn't decide if that's a generic kernel feature that is only implemented in 2 architectures, or an architecture speicifc feature. So I left it out.

But at least Martin wanted a hook here for his volatile pages patches, so I thought I'd submit this patch anyway.

With Nicks patch I can use arch_alloc_page instead of page_set_stable, but I can still not use arch_free_page instead of page_set_unused because it is done before the check for reserved pages. If reserved pages go away or the arch_free_page call would get moved after the check I could replace page_set_unused as well. So with Nicks patch we are only halfway there..

Quoted 6 lines

With Nicks patch I can use arch_alloc_page instead of page_set_stable, but I can still not use arch_free_page instead of page_set_unused because it is done before the check for reserved pages. If reserved pages go away or the arch_free_page call would get moved after the check I could replace page_set_unused as well. So with Nicks patch we are only halfway there..

Ahh, but with my patchSET I think we are all the way there ;)

Nonlinear mappings are (AFAIKS) simply a virtual memory concept that encodes the virtual address -> file offset differently from linear mappings.

I can't see why the filesystem/pagecache code should need to know anything about it, except for the fact that the ->nopage handler didn't quite pass down enough information (ie. pgoff). But it is more logical to pass pgoff rather than have the ->nopage function calculate it itself anyway. And having the nopage handler install the pte itself is sort of nasty.

This patch introduces a new fault handler that replaces ->nopage and ->populate and (hopefully) ->page_mkwrite. Most of the old mechanism is still in place so there is a lot of duplication and nice cleanups that can be removed if everyone switches over.

The rationale for doing this in the first place is that nonlinear mappings are subject to the pagefault vs invalidate/truncate race too, and it seemed stupid to duplicate the synchronisation logic rather than just consolidate the two.

Comments?

That's pretty nice.

Back when I was writing [the now slated for death] sound/oss/via82xxx_audio.c driver, Linus suggested that I implement
->nopage() for accessing the mmap'able DMA'd audio buffers, rather
than using remap_pfn_range(). It worked out very nicely, because it allowed the sound driver to retrieve $N pages for the mmap'able buffer (passed as an s/g list to the hardware) rather than requiring a single humongous buffer returned by pci_alloc_consistent().

And although probably not your primary motivation, your change does IMO improve this area of the kernel.

Nonlinear mappings are (AFAIKS) simply a virtual memory concept that encodes the virtual address -> file offset differently from linear mappings.

I can't see why the filesystem/pagecache code should need to know anything about it, except for the fact that the ->nopage handler didn't quite pass down enough information (ie. pgoff). But it is more logical to pass pgoff rather than have the ->nopage function calculate it itself anyway. And having the nopage handler install the pte itself is sort of nasty.

This patch introduces a new fault handler that replaces ->nopage and ->populate and (hopefully) ->page_mkwrite. Most of the old mechanism is still in place so there is a lot of duplication and nice cleanups that can be removed if everyone switches over.

The rationale for doing this in the first place is that nonlinear mappings are subject to the pagefault vs invalidate/truncate race too, and it seemed stupid to duplicate the synchronisation logic rather than just consolidate the two.

- You may find that gcc generates crap code for the initialisation of the `struct fault_data'. If so, filling the fields in by hand one-at-a-time will improve things.

- So is the plan here to migrate all code over to using vm_operations.fault() and to finally remove vm_operations.nopage and .nopfn? If so, that'd be nice.

- As you know, there is a case for constructing that `struct fault_data' all the way up in do_no_page(): so we can pass data back, asking do_no_page() to rerun the fault if we dropped mmap_sem.

- No useful opinion on the substance of this patch, sorry. It's Saturday ;)

- So is the plan here to migrate all code over to using vm_operations.fault() and to finally remove vm_operations.nopage and .nopfn? If so, that'd be nice.

- As you know, there is a case for constructing that `struct fault_data' all the way up in do_no_page(): so we can pass data back, asking do_no_page() to rerun the fault if we dropped mmap_sem.

- No useful opinion on the substance of this patch, sorry. It's Saturday ;)

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

Quoted 4 lines

- So is the plan here to migrate all code over to using vm_operations.fault() and to finally remove vm_operations.nopage and .nopfn? If so, that'd be nice.

Agreed. That would also allow to pass down knowledge of wether we can be interruptible or not (coming from userland or not). Useful in a few case when dealing with strange hw mappings.

Now, fault() still returns a struct page and thus doesn't quite fix the problem I'm exposing in my "User switchable HW mappings & cie" mail I posted today in which case we need to either duplicate the truncate logic in no_pfn() or get rid of no_pfn() and set the PTE from the fault handler . I tend to prefer the later provided that it's strictly limited for mappings that do not have a struct page though.

The truncate logic can't be duplicated because it works on struct pages.

What sounds best, if you use nopfn, is to do your own internal synchronisation against your unmap call. Obviously you can't because you have no ->nopfn_done call with which to drop locks ;)

So, hmm yes I have a good idea for how fault() could take over ->nopfn as
well: just return NULL, set the fault type to VM_FAULT_MINOR, and have the ->fault handler install the pte. It will require a new helper along the lines of vm_insert_page.

I'll code that up in my next patchset.

Quoted 7 lines

The truncate logic can't be duplicated because it works on struct pages.

What sounds best, if you use nopfn, is to do your own internal synchronisation against your unmap call. Obviously you can't because you have no ->nopfn_done call with which to drop locks ;)

Quoted 6 lines

So, hmm yes I have a good idea for how fault() could take over ->nopfn as
well: just return NULL, set the fault type to VM_FAULT_MINOR, and have the ->fault handler install the pte. It will require a new helper along the lines of vm_insert_page.

I'll code that up in my next patchset.

Which is exactly what I was proposing in my other mail :)

Read it, you'll understnad my point about the truncate logic... I sometimes want to return struct page (when the mapping is pointing to backup memory) or map it directly to hardware.

In the later case, with an appropriate helper, I can definitely do my own locking. In the former case, I return struct page's and need the truncate logic.

Yep, I see. You just need to be careful about the PFNMAP logic, so the VM knows whether the pte is backed by a struct page or not.

And going the pageless route means that you must disallow MAP_PRIVATE PROT_WRITE mappings, I trust that isn't a problem for you?

I guess the helper looks something like the following...

--

Index: linux-2.6/include/linux/mm.h
===================================================================

include/linux/mm.h +1 -0

@@ -1105,6 +1105,7 @@ unsigned long vmalloc_to_pfn(void *addr)
 int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
 			unsigned long pfn, unsigned long size, pgprot_t);
 int vm_insert_page(struct vm_area_struct *, unsigned long addr, struct page *);
+int vm_insert_pfn(struct vm_area_struct *, unsigned long addr, unsigned long pfn);
 
 struct page *follow_page(struct vm_area_struct *, unsigned long address,
 			unsigned int foll_flags);

Index: linux-2.6/mm/memory.c
===================================================================

mm/memory.c +27 -0

@@ -1267,6 +1267,44 @@ int vm_insert_page(struct vm_area_struct
 }
 EXPORT_SYMBOL(vm_insert_page);
 
+/**
+ * vm_insert_pfn - insert single pfn into user vma
+ * @vma: user vma to map to
+ * @addr: target user address of this page
+ * @pfn: source kernel pfn
+ *
+ * Similar to vm_inert_page, this allows drivers to insert individual pages
+ * they've allocated into a user vma. Same comments apply
+ */
+int vm_insert_pfn(struct vm_area_struct *vma, unsigned long addr, unsigned long pfn)
+{
+	struct mm_struct *mm = vma->vm_mm;
+	int retval;
+	pte_t *pte;
+	spinlock_t *ptl;
+
+	BUG_ON(is_cow_mapping(vma->vm_flags));
+
+	retval = -ENOMEM;
+	pte = get_locked_pte(mm, addr, &ptl);
+	if (!pte)
+		goto out;
+	retval = -EBUSY;
+	if (!pte_none(*pte))
+		goto out_unlock;
+
+	/* Ok, finally just insert the thing.. */

Quoted 3 lines

Yep, I see. You just need to be careful about the PFNMAP logic, so the VM knows whether the pte is backed by a struct page or not.

I still need to properly get my head around that one. I can't easily change the VMA during the "switch" but I can tweak the flags on the first nopage after one...

Quoted 3 lines

And going the pageless route means that you must disallow MAP_PRIVATE PROT_WRITE mappings, I trust that isn't a problem for you?

Should not but I need to look more closely.

Quoted 5 lines

mm/memory.c +5 -0

+	/* Ok, finally just insert the thing.. */
+	set_pte_at(mm, addr, pte, pfn_pte(pfn, vma->vm_page_prot));
+
+	vma->vm_flags |= VM_PFNMAP;
+	retval = 0;

mm/memory.c int vm_insert_page(struct vm_area_struct · line 1306

Quoted 6 lines

mm/memory.c +6 -0

+out_unlock:
+	pte_unmap_unlock(pte, ptl);
+out:
+	return retval;
+}
+EXPORT_SYMBOL(vm_insert_pfn);

It also needs update_mmu_cache() I suppose.

Quoted 7 lines · Benjamin Herrenschmidt

Quoted 3 lines

Yep, I see. You just need to be careful about the PFNMAP logic, so the VM knows whether the pte is backed by a struct page or not.

I still need to properly get my head around that one. I can't easily change the VMA during the "switch" but I can tweak the flags on the first nopage after one...

You'll want to clear VM_PFNMAP after unmapping all pages from it, before switching to struct page backing.

Quoted 4 lines

Quoted 2 lines

And going the pageless route means that you must disallow MAP_PRIVATE PROT_WRITE mappings, I trust that isn't a problem for you?

Should not but I need to look more closely.

If you do need to, then if your pfns are contiguous in virtual memory, and you can spare vm_pgoff, then you can use remap_pfn_range's method of setting vm_pgoff to the first pfn.

I can add a bit of sanity checking for that as well.

Quoted 5 lines

Quoted 5 lines

mm/memory.c +5 -0

+	/* Ok, finally just insert the thing.. */
+	set_pte_at(mm, addr, pte, pfn_pte(pfn, vma->vm_page_prot));
+
+	vma->vm_flags |= VM_PFNMAP;
+	retval = 0;

Quoted 8 lines

mm/memory.c int vm_insert_page(struct vm_area_struct · line 1306

Quoted 6 lines

mm/memory.c +6 -0

+out_unlock:
+	pte_unmap_unlock(pte, ptl);
+out:
+	return retval;
+}
+EXPORT_SYMBOL(vm_insert_pfn);

It also needs update_mmu_cache() I suppose.

Hmm, but it might not be called from a pagefault. Can we get away with not calling it? Or is it required by some architectures?

Quoted 3 lines

You'll want to clear VM_PFNMAP after unmapping all pages from it, before switching to struct page backing.

Which means having a list of all vma's ... I suppose I can look at the truncate code to do that race free but I was hoping I could avoid it (that's the whole point of using unmap_mapping_range() in fact).

Quoted 4 lines

Quoted 1 line

It also needs update_mmu_cache() I suppose.

Hmm, but it might not be called from a pagefault. Can we get away with not calling it? Or is it required by some architectures?

I think some architectures might be upset if it's not called...

Quoted 7 lines · Benjamin Herrenschmidt

Quoted 3 lines

You'll want to clear VM_PFNMAP after unmapping all pages from it, before switching to struct page backing.

Which means having a list of all vma's ... I suppose I can look at the truncate code to do that race free but I was hoping I could avoid it (that's the whole point of using unmap_mapping_range() in fact).

Yeah I don't think there is any other way to do it.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

It also needs update_mmu_cache() I suppose.

Hmm, but it might not be called from a pagefault. Can we get away with not calling it? Or is it required by some architectures?

I think some architectures might be upset if it's not called...

But would any get upset if it is called from !pagefault path?

Quoted 5 lines · Nick Piggin

Quoted 4 lines · Benjamin Herrenschmidt

Quoted 3 lines

You'll want to clear VM_PFNMAP after unmapping all pages from it, before switching to struct page backing.

Quoted 5 lines

Quoted 3 lines

Which means having a list of all vma's ... I suppose I can look at the truncate code to do that race free but I was hoping I could avoid it (that's the whole point of using unmap_mapping_range() in fact).

Yeah I don't think there is any other way to do it.

What is the problem if I always keep VM_PFNMAP set ?

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

It also needs update_mmu_cache() I suppose.

Hmm, but it might not be called from a pagefault. Can we get away with not calling it? Or is it required by some architectures?

I think some architectures might be upset if it's not called...

But would any get upset if it is called from !pagefault path?

Probably not... the PTE has been filled so it should be safe, but then, I don't know the details of what non-ppc archs do with that callback.

Quoted 12 lines · Benjamin Herrenschmidt

Quoted 10 lines · Nick Piggin

Quoted 7 lines · Benjamin Herrenschmidt

Quoted 3 lines

You'll want to clear VM_PFNMAP after unmapping all pages from it, before switching to struct page backing.

Which means having a list of all vma's ... I suppose I can look at the truncate code to do that race free but I was hoping I could avoid it (that's the whole point of using unmap_mapping_range() in fact).

Yeah I don't think there is any other way to do it.

Quoted 1 line

What is the problem if I always keep VM_PFNMAP set ?

The VM won't see that you have struct pages backing the ptes, and won't do the right refcounting or rmap stuff... But for file backed mappings, all the critical rmap stuff should be set up at mmap time, so you might have another option to simply always do the nopfn thing, as far as the VM is concerned (ie. even when you do have a struct page)

Quoted 6 lines · Nick Piggin

The VM won't see that you have struct pages backing the ptes, and won't do the right refcounting or rmap stuff... But for file backed mappings, all the critical rmap stuff should be set up at mmap time, so you might have another option to simply always do the nopfn thing, as far as the VM is concerned (ie. even when you do have a struct page)

Any reason why it wouldn't work to flip that bit on the first no_page() after a migration ? A migration always involves destroying all PTEs and is done with a per-object mutex held that no_page() takes too, so we can be pretty sure that the first nopage can set that bit before any PTE is actually inserted in the mapping after all the previous ones have been invalidated... That would avoid having to walk the vma's.

Quoted 8 lines · Benjamin Herrenschmidt

Quoted 5 lines · Nick Piggin

The VM won't see that you have struct pages backing the ptes, and won't do the right refcounting or rmap stuff... But for file backed mappings, all the critical rmap stuff should be set up at mmap time, so you might have another option to simply always do the nopfn thing, as far as the VM is concerned (ie. even when you do have a struct page)

Quoted 6 lines

Any reason why it wouldn't work to flip that bit on the first no_page() after a migration ? A migration always involves destroying all PTEs and is done with a per-object mutex held that no_page() takes too, so we can be pretty sure that the first nopage can set that bit before any PTE is actually inserted in the mapping after all the previous ones have been invalidated... That would avoid having to walk the vma's.

Ok I guess that would work. I was kind of thinking that one needs to hold the mmap_sem for writing when changing the flags, but so long as everyone *else* does, then I guess you can get exclusion from just the read lock. And your per-object mutex would prevent concurrent nopages from modifying it.

Quoted 7 lines · Nick Piggin

On Mon, Oct 09, 2006 at 10:07:50PM +1000, Benjamin Herrenschmidt wrote:

Ok I guess that would work. I was kind of thinking that one needs to hold the mmap_sem for writing when changing the flags, but so long as everyone *else* does, then I guess you can get exclusion from just the read lock. And your per-object mutex would prevent concurrent nopages from modifying it.

Wouldn't that confuse concurrent readers?

Could it be an option to make it safe for the fault handler to temporarily drop the mmap_sem read lock given that some conditions TBD are met?
In that case it can retake the mmap_sem write lock, do the VMA flags modifications, downgrade and do the pte modifications using a helper, or even use remap_pfn_range() during the time the write lock is held?

Quoted 9 lines · Thomas Hellstrom

Quoted 7 lines · Nick Piggin

On Mon, Oct 09, 2006 at 10:07:50PM +1000, Benjamin Herrenschmidt wrote:

Ok I guess that would work. I was kind of thinking that one needs to hold the mmap_sem for writing when changing the flags, but so long as everyone *else* does, then I guess you can get exclusion from just the read lock. And your per-object mutex would prevent concurrent nopages from modifying it.

Quoted 1 line

Wouldn't that confuse concurrent readers?

I think it should be safe so long as the entire mapping has been unmapped. After that, there is no read path that should care about that flag bit. So long as it is well commented (and maybe done via a helper in mm/memory.c), I can't yet see a problem with it.

Quoted 6 lines

Could it be an option to make it safe for the fault handler to temporarily drop the mmap_sem read lock given that some conditions TBD are met?
In that case it can retake the mmap_sem write lock, do the VMA flags modifications, downgrade and do the pte modifications using a helper, or even use remap_pfn_range() during the time the write lock is held?

When you drop and retake the mmap_sem, you need to start again from find_vma. At which point you technically probably want to start again from the architecture specfic fault code. It sounds difficult but I won't say it can't be done.

Quoted 11 lines

Quoted 5 lines

Could it be an option to make it safe for the fault handler to temporarily drop the mmap_sem read lock given that some conditions TBD are met?
In that case it can retake the mmap_sem write lock, do the VMA flags modifications, downgrade and do the pte modifications using a helper, or even use remap_pfn_range() during the time the write lock is held?

When you drop and retake the mmap_sem, you need to start again from find_vma. At which point you technically probably want to start again from the architecture specfic fault code. It sounds difficult but I won't say it can't be done.

I can be done with returning NOPAGE_REFAULT but as you said, I don't think it's necessary.

Cheers,
Ben.

Still, even with NOPAGE_REFAULT or the equivalent with the new fault() code, in the case we need to take this route, (and it looks like we won't have to),
I guess we still need to restart from find_vma() in the fault()/nopage() handler to make sure the VMA is still present. The object mutex need to be dropped as well to avoid deadlocks. Sounds complicated.

Quoted 3 lines

Still, even with NOPAGE_REFAULT or the equivalent with the new fault() code,
in the case we need to take this route, (and it looks like we won't have to),
I guess we still need to restart from find_vma() in the fault()/nopage() handler to make sure the VMA is still present. The object mutex need to be dropped as well to avoid deadlocks. Sounds complicated.

But as we said, it should be enough to do the flag change with the object mutex held as long as it's after unmap_mapped_ranges()

Ben.

Wouldn't that confuse concurrent readers?

Could it be an option to make it safe for the fault handler to temporarily drop the mmap_sem read lock given that some conditions TBD are met?
In that case it can retake the mmap_sem write lock, do the VMA flags modifications, downgrade and do the pte modifications using a helper, or even use remap_pfn_range() during the time the write lock is held?

Quoted 11 lines

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

It also needs update_mmu_cache() I suppose.

Hmm, but it might not be called from a pagefault. Can we get away with not calling it? Or is it required by some architectures?

I think some architectures might be upset if it's not called...

But would any get upset if it is called from !pagefault path?

Probably not... the PTE has been filled so it should be safe, but then, I don't know the details of what non-ppc archs do with that callback.

Quoted 7 lines · Nick Piggin

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

It also needs update_mmu_cache() I suppose.

Hmm, but it might not be called from a pagefault. Can we get away with not calling it? Or is it required by some architectures?

I think some architectures might be upset if it's not called...

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

But would any get upset if it is called from !pagefault path?

Probably not... the PTE has been filled so it should be safe, but then, I don't know the details of what non-ppc archs do with that callback.

I guess we can make the rule that it only be called from the ->fault handler anyway. If they want that functionality from ->mmap, then they can use remap_pfn_range, because it isn't so performance critical.

include/linux/mm.h +14 -0

+/*
+ * fault_data is filled in the the pagefault handler and passed to the
+ * vma's ->fault function. That function is responsible for filling in
+ * 'type', which is the type of fault if a page is returned, or the type
+ * of error if NULL is returned.
+ */
+struct fault_data {
+	struct vm_area_struct *vma;
+	unsigned long address;
+	pgoff_t pgoff;
+	unsigned int flags;
+
+	int type;
+};
 
 /*
  * These are the virtual MM functions - opening of an area, closing and

Quoted 15 lines · Nick Piggin

include/linux/mm.h +14 -0

+/*
+ * fault_data is filled in the the pagefault handler and passed to the
+ * vma's ->fault function. That function is responsible for filling in
+ * 'type', which is the type of fault if a page is returned, or the type
+ * of error if NULL is returned.
+ */
+struct fault_data {
+	struct vm_area_struct *vma;
+	unsigned long address;
+	pgoff_t pgoff;
+	unsigned int flags;
+
+	int type;
+};

include/linux/mm.h extern pgprot_t protection_map[16]; · line 224

Quoted 7 lines

/*
* These are the virtual MM functions - opening of an area, closing and

include/linux/mm.h +1 -0

@@ -203,6 +221,7 @@ extern pgprot_t protection_map[16];
 struct vm_operations_struct {
 	void (*open)(struct vm_area_struct * area);
 	void (*close)(struct vm_area_struct * area);
+	struct page * (*fault)(struct fault_data * data);

Please pass the vma as an explicit first argument so that all vm_operations operate on a vma. It's also much cleaner to have the separate between the the object operated on (the vma) and all the fault details (struct fault_data).

Quoted 16 lines · Christoph Hellwig

Quoted 15 lines · Nick Piggin

include/linux/mm.h +14 -0

+/*
+ * fault_data is filled in the the pagefault handler and passed to the
+ * vma's ->fault function. That function is responsible for filling in
+ * 'type', which is the type of fault if a page is returned, or the type
+ * of error if NULL is returned.
+ */
+struct fault_data {
+	struct vm_area_struct *vma;
+	unsigned long address;
+	pgoff_t pgoff;
+	unsigned int flags;
+
+	int type;
+};

Quoted 11 lines

include/linux/mm.h extern pgprot_t protection_map[16]; · line 224

Quoted 7 lines

/*
* These are the virtual MM functions - opening of an area, closing and

include/linux/mm.h +1 -0

@@ -203,6 +221,7 @@ extern pgprot_t protection_map[16];
 struct vm_operations_struct {
 	void (*open)(struct vm_area_struct * area);
 	void (*close)(struct vm_area_struct * area);
+	struct page * (*fault)(struct fault_data * data);

Please pass the vma as an explicit first argument so that all vm_operations operate on a vma. It's also much cleaner to have the separate between the the object operated on (the vma) and all the fault details (struct fault_data).

Hmm... I agree it is more consistent, but OTOH if we're passing a structure I thought it may as well just go in there. But I will change unless anyone comes up with an objection.

Quoted 4 lines · On Tue, 10 Oct 2006 14:13:27 +0200 Nick Piggin <[email protected]> wrote:

Hmm... I agree it is more consistent, but OTOH if we're passing a structure I thought it may as well just go in there. But I will change unless anyone comes up with an objection.

I'd agree that it's more attractive to have the vma* in the argument list, but it presumably adds runtime cost: cycles and stack depth. I don't how much though.

As for the filemap_xip changes, looks nice as far as I can tell. I will test that change for xip.

Actually, filemap_xip needs some attention I think... if xip files can be truncated or invalidated (I assume they can), then we need to lock the page, validate that it is the correct one and not truncated, and return with it locked.

Hi Nick,

Quoted 4 lines · Nick Piggin

Actually, filemap_xip needs some attention I think... if xip files can be truncated or invalidated (I assume they can), then we need to lock the page, validate that it is the correct one and not truncated, and return with it locked.

???

Isn't XIP for "eXecuting In Place" from ROM or FLASH?

How to truncate these? I thought the whole idea of XIP was a pure RO mapping?

They should be valid from mount to umount.

Regards

Ingo Oeser, a bit puzzled about that...

Nonlinear mappings are (AFAIKS) simply a virtual memory concept that encodes the virtual address -> file offset differently from linear mappings.

Quoted 4 lines · Nick Piggin

Nonlinear mappings are (AFAIKS) simply a virtual memory concept that encodes the virtual address -> file offset differently from linear mappings.

Hi Nick,

what is the status of this patch? I'm just trying to line up a kernel tree for the new DRM memory management code, which really would like this...

Dave.