fault vs truncate/invalidate race fix

Fix the race between invalidate_inode_pages and do_no_page.

Andrea Arcangeli identified a subtle race between invalidation of pages from pagecache with userspace mappings, and do_no_page.

The issue is that invalidation has to shoot down all mappings to the page, before it can be discarded from the pagecache. Between shooting down ptes to a particular page, and actually dropping the struct page from the pagecache, do_no_page from any process might fault on that page and establish a new mapping to the page just before it gets discarded from the pagecache.

The most common case where such invalidation is used is in file truncation. This case was catered for by doing a sort of open-coded seqlock between the file's i_size, and its truncate_count.

Truncation will decrease i_size, then increment truncate_count before unmapping userspace pages; do_no_page will read truncate_count, then find the page if it is within i_size, and then check truncate_count under the page table lock and back out and retry if it had subsequently been changed (ptl will serialise against unmapping, and ensure a potentially updated truncate_count is actually visible).

Complexity and documentation issues aside, the locking protocol fails in the case where we would like to invalidate pagecache inside i_size. do_no_page can come in anytime and filemap_nopage is not aware of the invalidation in progress (as it is when it is outside i_size). The end result is that dangling (->mapping == NULL) pages that appear to be from a particular file may be mapped into userspace with nonsense data. Valid mappings to the same place will see a different page.

Andrea implemented two working fixes, one using a real seqlock, another using a page->flags bit. He also proposed using the page lock in do_no_page, but that was initially considered too heavyweight. However, it is not a global or per-file lock, and the page cacheline is modified in do_no_page to increment _count and _mapcount anyway, so a further modification should not be a large performance hit. Scalability is not an issue.

This patch implements this latter approach. ->nopage implementations return with the page locked if it is possible for their underlying file to be invalidated (in that case, they must set a special vm_flags bit to indicate so). do_no_page only unlocks the page after setting up the mapping completely. invalidation is excluded because it holds the page lock during invalidation of each page (and ensures that the page is not mapped while holding the lock).

This also allows significant simplifications in do_no_page, because we have the page locked in the right place in the pagecache from the start.

Quoted 47 lines · On Wed, 21 Feb 2007 05:50:05 +0100 (CET) Nick Piggin <[email protected]> wrote:

Fix the race between invalidate_inode_pages and do_no_page.

Andrea Arcangeli identified a subtle race between invalidation of pages from pagecache with userspace mappings, and do_no_page.

The issue is that invalidation has to shoot down all mappings to the page, before it can be discarded from the pagecache. Between shooting down ptes to a particular page, and actually dropping the struct page from the pagecache, do_no_page from any process might fault on that page and establish a new mapping to the page just before it gets discarded from the pagecache.

The most common case where such invalidation is used is in file truncation. This case was catered for by doing a sort of open-coded seqlock between the file's i_size, and its truncate_count.

Truncation will decrease i_size, then increment truncate_count before unmapping userspace pages; do_no_page will read truncate_count, then find the page if it is within i_size, and then check truncate_count under the page table lock and back out and retry if it had subsequently been changed (ptl will serialise against unmapping, and ensure a potentially updated truncate_count is actually visible).

Complexity and documentation issues aside, the locking protocol fails in the case where we would like to invalidate pagecache inside i_size. do_no_page can come in anytime and filemap_nopage is not aware of the invalidation in progress (as it is when it is outside i_size). The end result is that dangling (->mapping == NULL) pages that appear to be from a particular file may be mapped into userspace with nonsense data. Valid mappings to the same place will see a different page.

Andrea implemented two working fixes, one using a real seqlock, another using a page->flags bit. He also proposed using the page lock in do_no_page, but that was initially considered too heavyweight. However, it is not a global or per-file lock, and the page cacheline is modified in do_no_page to increment _count and _mapcount anyway, so a further modification should not be a large performance hit. Scalability is not an issue.

This patch implements this latter approach. ->nopage implementations return with the page locked if it is possible for their underlying file to be invalidated (in that case, they must set a special vm_flags bit to indicate so). do_no_page only unlocks the page after setting up the mapping completely. invalidation is excluded because it holds the page lock during invalidation of each page (and ensures that the page is not mapped while holding the lock).

Quoted 4 lines

This also allows significant simplifications in do_no_page, because we have the page locked in the right place in the pagecache from the start.

Why was truncate_inode_pages_range() altered to unmap the page if it got mapped again?

Oh. Because the unmap_mapping_range() call got removed from vmtruncate(). Why? (Please send suitable updates to the changelog).

I guess truncate of a mmapped area isn't sufficiently common to worry about the inefficiency of this change.

Lots of memory barriers got removed in memory.c, unchangeloggedly.

Gratuitous renaming of locals in do_no_page() makes the change hard to review. Should have been a separate patch.

In fact, the patch would have been heaps clearer if that renaming had been a separate patch.

Quoted 6 lines

Why was truncate_inode_pages_range() altered to unmap the page if it got mapped again?

Oh. Because the unmap_mapping_range() call got removed from vmtruncate(). Why? (Please send suitable updates to the changelog).

We have to ensure it is unmapped, and be prepared to unmap it while under the page lock.

Quoted 2 lines

I guess truncate of a mmapped area isn't sufficiently common to worry about the inefficiency of this change.

Yeah, and it should be more efficient for files that aren't mmapped, because we don't have to take i_mmap_lock for them.

Quoted 1 line

Lots of memory barriers got removed in memory.c, unchangeloggedly.

Yeah they were all for the lockless truncate_count checks. Now that we use the page lock, we don't need barriers.

Quoted 5 lines

Gratuitous renaming of locals in do_no_page() makes the change hard to review. Should have been a separate patch.

In fact, the patch would have been heaps clearer if that renaming had been a separate patch.

Shall I?

Quoted 4 lines · On Wed, 7 Mar 2007 07:57:27 +0100 Nick Piggin <[email protected]> wrote:

Quoted 4 lines

Why was truncate_inode_pages_range() altered to unmap the page if it got mapped again?

Quoted 5 lines

Quoted 2 lines

Oh. Because the unmap_mapping_range() call got removed from vmtruncate(). Why? (Please send suitable updates to the changelog).

We have to ensure it is unmapped, and be prepared to unmap it while under the page lock.

But vmtruncate() dropped i_size, so nobody will map this page into pagetables from then on.

Quoted 14 lines

Quoted 2 lines

I guess truncate of a mmapped area isn't sufficiently common to worry about the inefficiency of this change.

Yeah, and it should be more efficient for files that aren't mmapped, because we don't have to take i_mmap_lock for them.

Quoted 1 line

Lots of memory barriers got removed in memory.c, unchangeloggedly.

Yeah they were all for the lockless truncate_count checks. Now that we use the page lock, we don't need barriers.

Quoted 3 lines

Gratuitous renaming of locals in do_no_page() makes the change hard to review. Should have been a separate patch.

Quoted 4 lines

Quoted 2 lines

In fact, the patch would have been heaps clearer if that renaming had been a separate patch.

Shall I?

If you don't have anything better to do, yes please ;)

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 (later) ->nopfn. 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.

After this patch, MAP_NONBLOCK no longer sets up ptes for pages present in pagecache. Seems like a fringe functionality anyway.

NOPAGE_REFAULT is removed. This should be implemented with ->fault, and no users have hit mainline yet.

Quoted 15 lines · On Wed, 21 Feb 2007 05:50:17 +0100 (CET) Nick Piggin <[email protected]> wrote:

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 (later) ->nopfn. 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.

Quoted 5 lines

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.

It's awkward to layer a largely do-nothing patch like this on top of a significant functional change. Makes it harder to isolate the source of regressions, harder to revert the do-something patch.

Quoted 2 lines

After this patch, MAP_NONBLOCK no longer sets up ptes for pages present in pagecache. Seems like a fringe functionality anyway.

Does Ingo agree?

Quoted 2 lines

NOPAGE_REFAULT is removed. This should be implemented with ->fault, and no users have hit mainline yet.

Did benh agree with that?

The patch unchangeloggedly adds a basic new structure to core mm (fault_data). Would be nice to document its fields, especially `flags'.

Please add less pointless blank lines.

How well has this been tested? The ocfs2 changes? gfs2? We should at least give those guys a heads-up.

Does anybody really pass a NULL `type' arg into filemap_nopage()?

This patch seems to churn things around an awful lot for minimal benefit.

Quoted 1 line · Andrew Morton

This patch seems to churn things around an awful lot for minimal benefit.

Well it fixes the whole design of the nonlinear fault path.

If it doesn't look very impressive, it could be because it leaves all the old crud around for backwards compatibility (the worst offenders are removed in patch 6/6).

If you look at the patchset as a whole, it removes about 250 lines, mostly of (non trivial) duplicated code in filemap.c memory.c shmem.c fremap.c, that is nonlinear pages specific and doesn't get anywhere near the testing that the linear fault path does.

A minimal fix for nonlinear pages would have required changing all ->populate handlers, which I simply thought was not very productive considering the testing and coverage issues, and that I was going to rewrite the nonlinear path anyway.

If you like, you can consider patches 1,2,3 as the fix, and ignore nonlinear (hey, it doesn't even bother checking truncate_count today!).

Then 4,5,6 is the fault/nonlinear rewrite, take it or leave it. I thought you would have liked the patches...

Quoted 18 lines · Nick Piggin

If it doesn't look very impressive, it could be because it leaves all the old crud around for backwards compatibility (the worst offenders are removed in patch 6/6).

If you look at the patchset as a whole, it removes about 250 lines, mostly of (non trivial) duplicated code in filemap.c memory.c shmem.c fremap.c, that is nonlinear pages specific and doesn't get anywhere near the testing that the linear fault path does.

A minimal fix for nonlinear pages would have required changing all ->populate handlers, which I simply thought was not very productive considering the testing and coverage issues, and that I was going to rewrite the nonlinear path anyway.

If you like, you can consider patches 1,2,3 as the fix, and ignore nonlinear (hey, it doesn't even bother checking truncate_count today!).

Quoted 2 lines

Then 4,5,6 is the fault/nonlinear rewrite, take it or leave it. I thought you would have liked the patches...

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI
- and remove all the nonlinear code. (This would split fremap areas into separate vmas)

Quoted 6 lines

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI - and remove all the nonlinear code. (This would split fremap areas into separate vmas)

I'm rather regretting having merged it - I don't think it has been used for much.

Paolo's UML speedup patches might use nonlinear though.

Quoted 11 lines · Andrew Morton

Quoted 6 lines

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI - and remove all the nonlinear code. (This would split fremap areas into separate vmas)

I'm rather regretting having merged it - I don't think it has been used for much.

Paolo's UML speedup patches might use nonlinear though.

yes, i wrote the first, prototype version of that for UML, it needs an extended version of the syscall, sys_remap_file_pages_prot():

http://redhat.com/~mingo/remap-file-pages-patches/remap-file-pages-prot-2.6.4-rc1-mm1-A1

i also wrote an x86 hypervisor kind of thing for UML, called 'sys_vcpu()', which allows UML to execute guest user-mode in a box, which also relies on sys_remap_file_pages_prot():

http://redhat.com/~mingo/remap-file-pages-patches/vcpu-2.6.4-rc2-mm1-A2

which reduced the UML guest syscall overhead from 30 usecs to 4 usecs (with native syscalls taking 2 usecs, on the box i tested, years ago).

So it certainly looked useful to me - but wasnt really picked up widely.

We'll always have the option to get rid of it (and hence completely reverse the decision to merge it) without breaking the ABI, by emulating the API via mremap(). That eliminates the UML speedup though. So no need to feel sorry about having merged it, we can easily revisit that years-old 'do we want it' decision, without any ABI worries.

Depending on whether anyone wants it, and what features they want, we could emulate the old syscall, and make a new restricted one which is much less intrusive.
For example, if we can operate only on MAP_ANONYMOUS memory and specify that nonlinear mappings effectively mlock the pages, then we can get rid of all the objrmap and unmap_mapping_range handling, forget about the writeout and msync problems...

Quoted 7 lines · Nick Piggin

Depending on whether anyone wants it, and what features they want, we could emulate the old syscall, and make a new restricted one which is much less intrusive.
For example, if we can operate only on MAP_ANONYMOUS memory and specify that nonlinear mappings effectively mlock the pages, then we can get rid of all the objrmap and unmap_mapping_range handling, forget about the writeout and msync problems...

Anonymous-only would make it a doorstop for Oracle, since its entire motive for using it is to window into objects larger than user virtual

address spaces (this likely also applies to UML, though they should really chime in to confirm). Restrictions to tmpfs and/or ramfs would likely be liveable, though I suspect some things might want to do it to shm segments (I'll ask about that one). There's definitely no need for a persistent backing store for the object to be remapped in Oracle's case, in any event. It's largely the in-core destination and source of IO, not something saved on-disk itself.

Quoted 11 lines · Bill Irwin

Quoted 7 lines · Nick Piggin

Depending on whether anyone wants it, and what features they want, we could emulate the old syscall, and make a new restricted one which is much less intrusive.
For example, if we can operate only on MAP_ANONYMOUS memory and specify that nonlinear mappings effectively mlock the pages, then we can get rid of all the objrmap and unmap_mapping_range handling, forget about the writeout and msync problems...

Anonymous-only would make it a doorstop for Oracle, since its entire motive for using it is to window into objects larger than user virtual

Uh, duh yes I don't mean MAP_ANONYMOUS, I was just thinking of the shmem inode that sits behind MAP_ANONYMOUS|MAP_SHARED. Of course if you don't have a file descriptor to get a pgoff, then remap_file_pages is a doorstop for everyone ;)

Quoted 7 lines

address spaces (this likely also applies to UML, though they should really chime in to confirm). Restrictions to tmpfs and/or ramfs would likely be liveable, though I suspect some things might want to do it to shm segments (I'll ask about that one). There's definitely no need for a persistent backing store for the object to be remapped in Oracle's case, in any event. It's largely the in-core destination and source of IO, not something saved on-disk itself.

Yeah, tmpfs/shm segs are what I was thinking about. If UML can live with that as well, then I think it might be a good option.

Quoted 26 lines · Nick Piggin

Quoted 11 lines · Bill Irwin

Quoted 7 lines · Nick Piggin

Depending on whether anyone wants it, and what features they want, we could emulate the old syscall, and make a new restricted one which is much less intrusive.
For example, if we can operate only on MAP_ANONYMOUS memory and specify that nonlinear mappings effectively mlock the pages, then we can get rid of all the objrmap and unmap_mapping_range handling, forget about the writeout and msync problems...

Anonymous-only would make it a doorstop for Oracle, since its entire motive for using it is to window into objects larger than user virtual

Uh, duh yes I don't mean MAP_ANONYMOUS, I was just thinking of the shmem inode that sits behind MAP_ANONYMOUS|MAP_SHARED. Of course if you don't have a file descriptor to get a pgoff, then remap_file_pages is a doorstop for everyone ;)

Quoted 7 lines

address spaces (this likely also applies to UML, though they should really chime in to confirm). Restrictions to tmpfs and/or ramfs would likely be liveable, though I suspect some things might want to do it to shm segments (I'll ask about that one). There's definitely no need for a persistent backing store for the object to be remapped in Oracle's case, in any event. It's largely the in-core destination and source of IO, not something saved on-disk itself.

Quoted 2 lines

Yeah, tmpfs/shm segs are what I was thinking about. If UML can live with that as well, then I think it might be a good option.

Oh, hmm.... if you can truncate these things then you still need to force unmap so you still need i_mmap_nonlinear.

But come to think of it, I still don't think nonlinear mappings are too bad as they are ;)

Quoted 6 lines · Nick Piggin

Quoted 3 lines

Yeah, tmpfs/shm segs are what I was thinking about. If UML can live with that as well, then I think it might be a good option.

Oh, hmm.... if you can truncate these things then you still need to force unmap so you still need i_mmap_nonlinear.

Well, we don't need truncate(), but MADV_REMOVE for memory hotunplug, which is way similar I guess.

About the restriction to tmpfs, I have just discovered
'[PATCH] mm: tracking shared dirty pages' (commit d08b3851da41d0ee60851f2c75b118e1f7a5fc89), which already partially conflicts with remap_file_pages for file-based mmaps (and that's fully fine, for now).

Even if UML does not need it, till now if there is a VMA protection and a page hasn't been remapped with remap_file_pages, the VMA protection is used (just because it makes sense).

However, it is only used when the PTE is first created - we can never change protections on a VMA - so it vma_wants_writenotify() is true (on all file-based and on no shmfs based mapping, right?), and we write-protect the VMA, it will always be write-protected.

That's no problem for UML, but for any other user (I guess I'll have to prevent callers from trying such stuff - I started from a pretty generic patch).

Quoted 2 lines

But come to think of it, I still don't think nonlinear mappings are too bad as they are ;)

Btw, I really like removing ->populate and merging the common code together. filemap_populate and shmem_populate are so obnoxiously different that I already wanted to do that (after merging remap_file_pages() core).

Also, I'm curious. Since my patches are already changing remap_file_pages() code, should they be absolutely merged after yours?

Quoted 20 lines · Blaisorblade

Quoted 5 lines · Nick Piggin

Quoted 2 lines

Yeah, tmpfs/shm segs are what I was thinking about. If UML can live with that as well, then I think it might be a good option.

Oh, hmm.... if you can truncate these things then you still need to force unmap so you still need i_mmap_nonlinear.

Well, we don't need truncate(), but MADV_REMOVE for memory hotunplug, which is way similar I guess.

About the restriction to tmpfs, I have just discovered '[PATCH] mm: tracking shared dirty pages' (commit d08b3851da41d0ee60851f2c75b118e1f7a5fc89), which already partially conflicts with remap_file_pages for file-based mmaps (and that's fully fine, for now).

Even if UML does not need it, till now if there is a VMA protection and a page hasn't been remapped with remap_file_pages, the VMA protection is used (just because it makes sense).

Quoted 4 lines

However, it is only used when the PTE is first created - we can never change protections on a VMA - so it vma_wants_writenotify() is true (on all file-based and on no shmfs based mapping, right?), and we write-protect the VMA, it will always be write-protected.

Yes, I believe that is the case, however I wonder if that is going to be a problem for you to distinguish between write faults for clean writable ptes, and write faults for readonly ptes?

Quoted 2 lines

Also, I'm curious. Since my patches are already changing remap_file_pages() code, should they be absolutely merged after yours?

Is there a big clash? I don't think I did a great deal to fremap.c (mainly just removing stuff)...

Quoted 21 lines · Nick Piggin

Quoted 20 lines · Blaisorblade

Quoted 5 lines · Nick Piggin

Quoted 2 lines

Yeah, tmpfs/shm segs are what I was thinking about. If UML can live with that as well, then I think it might be a good option.

Oh, hmm.... if you can truncate these things then you still need to force unmap so you still need i_mmap_nonlinear.

Well, we don't need truncate(), but MADV_REMOVE for memory hotunplug, which is way similar I guess.

About the restriction to tmpfs, I have just discovered '[PATCH] mm: tracking shared dirty pages' (commit d08b3851da41d0ee60851f2c75b118e1f7a5fc89), which already partially conflicts with remap_file_pages for file-based mmaps (and that's fully fine, for now).

Even if UML does not need it, till now if there is a VMA protection and a page hasn't been remapped with remap_file_pages, the VMA protection is used (just because it makes sense).

Quoted 8 lines

Quoted 4 lines

However, it is only used when the PTE is first created - we can never change protections on a VMA - so it vma_wants_writenotify() is true (on all file-based and on no shmfs based mapping, right?), and we write-protect the VMA, it will always be write-protected.

Yes, I believe that is the case, however I wonder if that is going to be a problem for you to distinguish between write faults for clean writable ptes, and write faults for readonly ptes?

I wouldn't be able to distinguish them, but am I going to get write faults for clean ptes when vma_wants_writenotify() is false (as seems to be for tmpfs)? I guess not.

For tmpfs pages, clean writable PTEs are mapped as writable so they won't give any problem, since vma_wants_writenotify() is false for tmpfs. Correct?

Yes, that should be the case. So would this mean that nonlinear protections don't work on regular files? I guess that's OK if Oracle and UML both use tmpfs/shm?

Yes, that should be the case. So would this mean that nonlinear protections don't work on regular files? I guess that's OK if Oracle and UML both use tmpfs/shm?

Quoted 35 lines · Blaisorblade

Quoted 21 lines · Nick Piggin

Quoted 20 lines · Blaisorblade

Quoted 5 lines · Nick Piggin

Quoted 2 lines

Yeah, tmpfs/shm segs are what I was thinking about. If UML can live with that as well, then I think it might be a good option.

Oh, hmm.... if you can truncate these things then you still need to force unmap so you still need i_mmap_nonlinear.

Well, we don't need truncate(), but MADV_REMOVE for memory hotunplug, which is way similar I guess.

About the restriction to tmpfs, I have just discovered '[PATCH] mm: tracking shared dirty pages' (commit d08b3851da41d0ee60851f2c75b118e1f7a5fc89), which already partially conflicts with remap_file_pages for file-based mmaps (and that's fully fine, for now).

Even if UML does not need it, till now if there is a VMA protection and a page hasn't been remapped with remap_file_pages, the VMA protection is used (just because it makes sense).

Quoted 8 lines

Quoted 4 lines

However, it is only used when the PTE is first created - we can never change protections on a VMA - so it vma_wants_writenotify() is true (on all file-based and on no shmfs based mapping, right?), and we write-protect the VMA, it will always be write-protected.

Yes, I believe that is the case, however I wonder if that is going to be a problem for you to distinguish between write faults for clean writable ptes, and write faults for readonly ptes?

I wouldn't be able to distinguish them, but am I going to get write faults for clean ptes when vma_wants_writenotify() is false (as seems to be for tmpfs)? I guess not.

Quoted 3 lines

For tmpfs pages, clean writable PTEs are mapped as writable so they won't give any problem, since vma_wants_writenotify() is false for tmpfs. Correct?

Yes, that should be the case. So would this mean that nonlinear protections don't work on regular files?

I guess that's OK if Oracle and UML both use
tmpfs/shm?

Quoted 13 lines · Nick Piggin

Quoted 12 lines

Quoted 4 lines

Yes, I believe that is the case, however I wonder if that is going to be a problem for you to distinguish between write faults for clean writable ptes, and write faults for readonly ptes?

I wouldn't be able to distinguish them, but am I going to get write faults for clean ptes when vma_wants_writenotify() is false (as seems to be for tmpfs)? I guess not.

For tmpfs pages, clean writable PTEs are mapped as writable so they won't give any problem, since vma_wants_writenotify() is false for tmpfs. Correct?

Quoted 2 lines

Yes, that should be the case. So would this mean that nonlinear protections don't work on regular files?

They still work in most cases (including for UML), but if the initial mmap() specified PROT_WRITE, that is ignored, for pages which are not remapped via remap_file_pages(). UML uses PROT_NONE for the initial mmap, so that's no problem.

Quoted 15 lines · Blaisorblade

Quoted 14 lines · Nick Piggin

Quoted 11 lines

Quoted 3 lines

Yes, I believe that is the case, however I wonder if that is going to be a problem for you to distinguish between write faults for clean writable ptes, and write faults for readonly ptes?

I wouldn't be able to distinguish them, but am I going to get write faults for clean ptes when vma_wants_writenotify() is false (as seems to be for tmpfs)? I guess not.

For tmpfs pages, clean writable PTEs are mapped as writable so they won't give any problem, since vma_wants_writenotify() is false for tmpfs. Correct?

Yes, that should be the case. So would this mean that nonlinear protections don't work on regular files?

Quoted 4 lines

They still work in most cases (including for UML), but if the initial mmap() specified PROT_WRITE, that is ignored, for pages which are not remapped via remap_file_pages(). UML uses PROT_NONE for the initial mmap, so that's no problem.

But how are you going to distinguish a write fault on a readonly pte for dirty page accounting vs a read-only nonlinear protection?

You can't store any more data in a present pte AFAIK, so you'd have to have some out of band data. At which point, you may as well just forget about vma_wants_writenotify vmas, considering that everybody is using shmem/ramfs.

Quoted 7 lines · Nick Piggin

Depending on whether anyone wants it, and what features they want, we could emulate the old syscall, and make a new restricted one which is much less intrusive.
For example, if we can operate only on MAP_ANONYMOUS memory and specify that nonlinear mappings effectively mlock the pages, then we can get rid of all the objrmap and unmap_mapping_range handling, forget about the writeout and msync problems...

Anonymous-only would make it a doorstop for Oracle, since its entire motive for using it is to window into objects larger than user virtual address spaces (this likely also applies to UML, though they should really chime in to confirm).

Restrictions to tmpfs and/or ramfs would
likely be liveable, though I suspect some things might want to do it to shm segments (I'll ask about that one).

There's definitely no need for a
persistent backing store for the object to be remapped in Oracle's case, in any event. It's largely the in-core destination and source of IO, not something saved on-disk itself.

-- wli

Quoted 5 lines

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI
- and remove all the nonlinear code. (This would split fremap areas into separate vmas)

I'm rather regretting having merged it - I don't think it has been used for much.
Paolo's UML speedup patches might use nonlinear though.

Quoted 5 lines · On Wed, 7 Mar 2007 09:27:55 +0100 Ingo Molnar <[email protected]> wrote:

Quoted 5 lines

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI
- and remove all the nonlinear code. (This would split fremap areas into separate vmas)

Quoted 3 lines · Andrew Morton

I'm rather regretting having merged it - I don't think it has been used for much.
Paolo's UML speedup patches might use nonlinear though.

Guess what major real-life application not only uses nonlinear daily but would even be very happy to see it extended with non-vma-creating protections and more?

It's not terribly typical for things to be
truncated while remap_file_pages() is doing its work, though it's been proposed as a method of dynamism. It won't stress remap_file_pages() vs. truncate() in any meaningful way, though, as userspace will be rather diligent about clearing in-use data out of the file offset range to be truncated away anyway, and all that via O_DIRECT.

Quoted 3 lines

Guess what major real-life application not only uses nonlinear daily but would even be very happy to see it extended with non-vma-creating protections and more?

uh-oh. SQL server?

Quoted 6 lines

It's not terribly typical for things to be
truncated while remap_file_pages() is doing its work, though it's been proposed as a method of dynamism. It won't stress remap_file_pages() vs. truncate() in any meaningful way, though, as userspace will be rather diligent about clearing in-use data out of the file offset range to be truncated away anyway, and all that via O_DIRECT.

The problem here isn't related to truncate or direct-IO. It's just plain-old MAP_SHARED. nonlinear VMAs are now using the old-style dirty-memory management. msync() is basically a no-op and the code is wildly tricky and pretty much untested. The chances that we broke it are considerable.

Quoted 18 lines

If it doesn't look very impressive, it could be because it leaves all the old crud around for backwards compatibility (the worst offenders are removed in patch 6/6).

If you look at the patchset as a whole, it removes about 250 lines, mostly of (non trivial) duplicated code in filemap.c memory.c shmem.c fremap.c, that is nonlinear pages specific and doesn't get anywhere near the testing that the linear fault path does.

A minimal fix for nonlinear pages would have required changing all ->populate handlers, which I simply thought was not very productive considering the testing and coverage issues, and that I was going to rewrite the nonlinear path anyway.

If you like, you can consider patches 1,2,3 as the fix, and ignore nonlinear (hey, it doesn't even bother checking truncate_count today!).

Quoted 2 lines

Then 4,5,6 is the fault/nonlinear rewrite, take it or leave it. I thought you would have liked the patches...

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI
- and remove all the nonlinear code. (This would split fremap areas into separate vmas)

Dirty page accounting doesn't work either on
non-linear mappings

Quoted 2 lines

Dirty page accounting doesn't work either on
non-linear mappings

It doesn't? Confused - these things don't have anything to do with each other do they?

Quoted 5 lines

Quoted 2 lines

Dirty page accounting doesn't work either on
non-linear mappings

It doesn't? Confused - these things don't have anything to do with each other do they?

Look in page_mkclean(). Where does it handle non-linear mappings?

Quoted 8 lines · On Wed, 07 Mar 2007 09:51:57 +0100 Miklos Szeredi <[email protected]> wrote:

Quoted 5 lines

Quoted 2 lines

Dirty page accounting doesn't work either on
non-linear mappings

It doesn't? Confused - these things don't have anything to do with each other do they?

Look in page_mkclean(). Where does it handle non-linear mappings?

OK, I'd forgotten about that. It won't break dirty memory accounting, but it'll potentially break dirty memory balancing.

If we have the wrong page (due to nonlinear), page_check_address() will fail and we'll leave the pte dirty. That puts us back to the pre-2.6.17 algorithms and I guess it'll break the msync guarantees.

Peter, I thought we went through the nonlinear problem ages ago and decided it was OK?

Quoted 18 lines · Andrew Morton

Quoted 8 lines · On Wed, 07 Mar 2007 09:51:57 +0100 Miklos Szeredi <[email protected]> wrote:

Quoted 5 lines

Quoted 2 lines

Dirty page accounting doesn't work either on
non-linear mappings

It doesn't? Confused - these things don't have anything to do with each other do they?

Look in page_mkclean(). Where does it handle non-linear mappings?

OK, I'd forgotten about that. It won't break dirty memory accounting, but it'll potentially break dirty memory balancing.

If we have the wrong page (due to nonlinear), page_check_address() will fail and we'll leave the pte dirty. That puts us back to the pre-2.6.17 algorithms and I guess it'll break the msync guarantees.

Quoted 2 lines

Peter, I thought we went through the nonlinear problem ages ago and decided it was OK?

msync breakage is bad, but otherwise I don't know that we care about dirty page writeout efficiency.

But I think we discovered that those msync changes are bogus anyway becuase there is a small race window where pte could be dirtied without page being set dirty?

Quoted 3 lines

But I think we discovered that those msync changes are bogus anyway becuase there is a small race window where pte could be dirtied without page being set dirty?

Dunno, I don't recall that. We dirty the page before the pte...

Quoted 3 lines · On Wed, 7 Mar 2007 10:18:23 +0100 Nick Piggin <[email protected]> wrote:

msync breakage is bad, but otherwise I don't know that we care about dirty page writeout efficiency.

Well. We made so many changes to support the synchronous dirty-the-page-when-we-dirty-the-pte thing that I'm rather doubtful that the old-style approach still works. It might seem to, most of the time. But if it _is_ subtly broken, boy it's going to take a long time for us to find out.

Quoted 3 lines

But I think we discovered that those msync changes are bogus anyway becuase there is a small race window where pte could be dirtied without page being set dirty?

Dunno, I don't recall that. We dirty the page before the pte...

Quoted 3 lines

Look in page_mkclean(). Where does it handle non-linear mappings?

OK, I'd forgotten about that. It won't break dirty memory accounting, but it'll potentially break dirty memory balancing.

If we have the wrong page (due to nonlinear), page_check_address() will fail and we'll leave the pte dirty.

Quoted 8 lines · On Wed, 07 Mar 2007 09:51:57 +0100 Miklos Szeredi <[email protected]> wrote:

Quoted 5 lines

Quoted 2 lines

Dirty page accounting doesn't work either on
non-linear mappings

It doesn't? Confused - these things don't have anything to do with each other do they?

Look in page_mkclean(). Where does it handle non-linear mappings?

OK, I'd forgotten about that. It won't break dirty memory accounting, but it'll potentially break dirty memory balancing.

If we have the wrong page (due to nonlinear), page_check_address() will fail and we'll leave the pte dirty. That puts us back to the pre-2.6.17 algorithms and I guess it'll break the msync guarantees.

Peter, I thought we went through the nonlinear problem ages ago and decided it was OK?

Quoted 14 lines · Andrew Morton

Quoted 8 lines · On Wed, 07 Mar 2007 09:51:57 +0100 Miklos Szeredi <[email protected]> wrote:

Quoted 5 lines

Quoted 2 lines

Dirty page accounting doesn't work either on
non-linear mappings

It doesn't? Confused - these things don't have anything to do with each other do they?

Look in page_mkclean(). Where does it handle non-linear mappings?

OK, I'd forgotten about that. It won't break dirty memory accounting, but it'll potentially break dirty memory balancing.

Quoted 6 lines

If we have the wrong page (due to nonlinear), page_check_address() will fail and we'll leave the pte dirty. That puts us back to the pre-2.6.17 algorithms and I guess it'll break the msync guarantees.

Peter, I thought we went through the nonlinear problem ages ago and decided it was OK?

Can recollect as much, I modelled it after page_referenced() and can't find any VM_NONLINEAR specific code in there either.

Will have a hard look, but if its broken, then page_referenced if equally broken it seems, which would make page reclaim funny in the light of nonlinear mappings.

Quoted 7 lines · Peter Zijlstra

Can recollect as much, I modelled it after page_referenced() and can't find any VM_NONLINEAR specific code in there either.

Will have a hard look, but if its broken, then page_referenced if equally broken it seems, which would make page reclaim funny in the light of nonlinear mappings.

page_referenced is just an heuristic, and it ignores nonlinear mappings and the page which will get filtered down to try_to_unmap.

Page reclaim is already "funny" for nonlinear mappings, page_referenced is the least of its worries ;) It works, though.

Quoted 9 lines · Nick Piggin

Quoted 7 lines · Peter Zijlstra

Can recollect as much, I modelled it after page_referenced() and can't find any VM_NONLINEAR specific code in there either.

Will have a hard look, but if its broken, then page_referenced if equally broken it seems, which would make page reclaim funny in the light of nonlinear mappings.

Quoted 5 lines

page_referenced is just an heuristic, and it ignores nonlinear mappings and the page which will get filtered down to try_to_unmap.

Page reclaim is already "funny" for nonlinear mappings, page_referenced is the least of its worries ;) It works, though.

Or, to be more helpful, unmap_mapping_range is what it should be modelled on.

*sigh* yes was looking at all that code, thats gonna be darn slow though, but I'll whip up a patch.

Quoted 2 lines

*sigh* yes was looking at all that code, thats gonna be darn slow though, but I'll whip up a patch.

Well, if it's going to be darn slow, maybe it's better to go with mingo's plan on emulating nonlinear vmas with linear ones. That'll be

darn slow as well, but at least it will be much less complicated.

Quoted 5 lines · Miklos Szeredi

Quoted 2 lines

*sigh* yes was looking at all that code, thats gonna be darn slow though, but I'll whip up a patch.

Well, if it's going to be darn slow, maybe it's better to go with mingo's plan on emulating nonlinear vmas with linear ones. That'll be

There are real users who want these fast, though.

Quoted 1 line

darn slow as well, but at least it will be much less complicated.

IMO, the best thing to do is just restore msync behaviour, and comment the fact that we ignore nonlinears. We need to restore msync behaviour to fix races in regular mappings anyway, at least for now.

Quoted 4 lines · Nick Piggin

Quoted 3 lines · Miklos Szeredi

Quoted 2 lines

*sigh* yes was looking at all that code, thats gonna be darn slow though, but I'll whip up a patch.

Quoted 4 lines

Quoted 2 lines

Well, if it's going to be darn slow, maybe it's better to go with mingo's plan on emulating nonlinear vmas with linear ones. That'll be

There are real users who want these fast, though.

Yeah, why don't we have a tree per nonlinear vma to find these pages?

wli mentions shadow page tables..

Quoted 5 lines

Quoted 1 line

darn slow as well, but at least it will be much less complicated.

IMO, the best thing to do is just restore msync behaviour, and comment the fact that we ignore nonlinears. We need to restore msync behaviour to fix races in regular mappings anyway, at least for now.

Seems to be the best quick solution indeed.

Quoted 5 lines

Quoted 1 line

There are real users who want these fast, though.

Yeah, why don't we have a tree per nonlinear vma to find these pages?

wli mentions shadow page tables..

We could do something more efficient, but I thought that half the point was that they didn't carry any of this extra memory, and they could be really fast to set up at the expense of efficiency elsewhere.

I don't see it being a big deal. I doubt anybody is writing out huge amounts of data via nonlinear mappings.

Quoted 6 lines · Nick Piggin

Quoted 5 lines

Quoted 1 line

There are real users who want these fast, though.

Yeah, why don't we have a tree per nonlinear vma to find these pages?

wli mentions shadow page tables..

Quoted 3 lines

We could do something more efficient, but I thought that half the point was that they didn't carry any of this extra memory, and they could be really fast to set up at the expense of efficiency elsewhere.

I'm failing to understand this :-(

That extra memory, and apparently they don't want the inefficiency either.

Quoted 3 lines

I don't see it being a big deal. I doubt anybody is writing out huge amounts of data via nonlinear mappings.

Well, now they don't, but it could be done or even exploited as a DoS.

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Nick Piggin

Quoted 5 lines

Quoted 1 line

There are real users who want these fast, though.

Yeah, why don't we have a tree per nonlinear vma to find these pages?

wli mentions shadow page tables..

Quoted 7 lines

Quoted 3 lines

We could do something more efficient, but I thought that half the point was that they didn't carry any of this extra memory, and they could be really fast to set up at the expense of efficiency elsewhere.

I'm failing to understand this :-(

That extra memory, and apparently they don't want the inefficiency

Quoted 1 line

either.

Sorry, I didn't understand your misunderstandings ;)

Quoted 1 line

Well, now they don't, but it could be done or even exploited as a DoS.

But so could nonlinear page reclaim. I think we need to restrict nonlinear mappings to root if we're worried about that.

Quoted 9 lines · Nick Piggin

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Nick Piggin

Quoted 5 lines

Quoted 1 line

There are real users who want these fast, though.

Yeah, why don't we have a tree per nonlinear vma to find these pages?

wli mentions shadow page tables..

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

We could do something more efficient, but I thought that half the point was that they didn't carry any of this extra memory, and they could be really fast to set up at the expense of efficiency elsewhere.

I'm failing to understand this :-(

That extra memory, and apparently they don't want the inefficiency

s/T/W/

Quoted 3 lines

Quoted 1 line

either.

Sorry, I didn't understand your misunderstandings ;)

Bah, my brain is thick and foggy today. Let us try again;

Nonlinear vmas exist because many vmas are expensive somehow, right? Nonlinear vmas keep the page mapping in the page tables and screw rmaps.

This 'extra memory' you mentioned would be the overhead of tracking the actual ranges?

And apparently now we want it to not suck on the rmap case :-(

Anyway, if used on a non writeback capable backing store (ramfs) page_mkclean will never be called. If also mlocked (I think oracle does this) then page reclaim will pass over too.

So we're only interested in the bdi_cap_accounting_dirty and VM_SHARED case, right?

Tracking these ranges on a per-vma basis would avoid taking the mm wide mmap_sem and so would be cheaper than regular vmas.

Would that still be too expensive?

Quoted 4 lines

Quoted 1 line

Well, now they don't, but it could be done or even exploited as a DoS.

But so could nonlinear page reclaim. I think we need to restrict nonlinear mappings to root if we're worried about that.

Can't we just 'fix' it?

Quoted 4 lines

Tracking these ranges on a per-vma basis would avoid taking the mm wide mmap_sem and so would be cheaper than regular vmas.

Would that still be too expensive?

Well you can today remap N pages in a file, arbitrarily for sizeof(pte_t)*tiny bit for the upper page tables + small constant for the vma.

At best, you need an extra pointer to pte / vaddr, so you'd basically double memory overhead.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Well, now they don't, but it could be done or even exploited as a DoS.

But so could nonlinear page reclaim. I think we need to restrict nonlinear mappings to root if we're worried about that.

Can't we just 'fix' it?

The thing is, I don't think anybody who uses these things cares about any of the 'problems' you want to fix, do they? We are interested in dirty pages only for the correctness issue, rather than performance. Same as reclaim.

Quoted 11 lines · Nick Piggin

Quoted 4 lines

Tracking these ranges on a per-vma basis would avoid taking the mm wide mmap_sem and so would be cheaper than regular vmas.

Would that still be too expensive?

Well you can today remap N pages in a file, arbitrarily for sizeof(pte_t)*tiny bit for the upper page tables + small constant for the vma.

At best, you need an extra pointer to pte / vaddr, so you'd basically double memory overhead.

I was hoping some form of range compression would gain something, but if its a fully random mapping, then yes a shadow page table would be needed (still looking into what a pte_chain is)

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Well, now they don't, but it could be done or even exploited as a DoS.

But so could nonlinear page reclaim. I think we need to restrict nonlinear mappings to root if we're worried about that.

Can't we just 'fix' it?

Quoted 4 lines

The thing is, I don't think anybody who uses these things cares about any of the 'problems' you want to fix, do they? We are interested in dirty pages only for the correctness issue, rather than performance. Same as reclaim.

If so, we can just stick to the dead slow but correct 'scan the full vma' page_mkclean() and nobody would ever trigger it.

What is the DoS scenario wrt reclaim? We really ought to fix that if real, those UML farms run on nothing but nonlinear reclaim I'd think.

Quoted 5 lines

Quoted 4 lines

The thing is, I don't think anybody who uses these things cares about any of the 'problems' you want to fix, do they? We are interested in dirty pages only for the correctness issue, rather than performance. Same as reclaim.

Quoted 2 lines

If so, we can just stick to the dead slow but correct 'scan the full vma' page_mkclean() and nobody would ever trigger it.

Not if we restricted it to root and mlocked tmpfs. But then why wouldn't you just do it with the much more efficient msync walk, so that if root does want to do writeout via these things, it does not blow up?

Quoted 2 lines

What is the DoS scenario wrt reclaim? We really ought to fix that if real, those UML farms run on nothing but nonlinear reclaim I'd think.

I guess you can just increase the computational complexity of reclaim quite easily.

Quoted 5 lines · Nick Piggin

Quoted 5 lines

Quoted 4 lines

The thing is, I don't think anybody who uses these things cares about any of the 'problems' you want to fix, do they? We are interested in dirty pages only for the correctness issue, rather than performance. Same as reclaim.

Quoted 7 lines

Quoted 2 lines

If so, we can just stick to the dead slow but correct 'scan the full vma' page_mkclean() and nobody would ever trigger it.

Not if we restricted it to root and mlocked tmpfs. But then why wouldn't you just do it with the much more efficient msync walk, so that if root does want to do writeout via these things, it does not blow up?

This is all used on ram based filesystems right, they all have BDI_CAP_NO_WRITEBACK afaik, so page_mkclean will never get called anyway. Mlock doesn't avoid getting page_mkclean called.

Those who use this on a 'real' filesystem will get hit in the face by a linear scanning page_mkclean(), but AFAIK nobody does this anyway.

Restricting it to root for such filesystems is unwanted, that'd severely handicap both UML and Oracle as I understand it (are there other users of this feature around?)

msync() might never get called and then we're back with the old behaviour where we can surprise the VM with a ton of dirty pages.

Quoted 5 lines

Quoted 2 lines

What is the DoS scenario wrt reclaim? We really ought to fix that if real, those UML farms run on nothing but nonlinear reclaim I'd think.

I guess you can just increase the computational complexity of reclaim quite easily.

Right, on first glance it doesn't look to be too bad, but I should take a closer look.

Quoted 15 lines · Peter Zijlstra

Quoted 5 lines · Nick Piggin

Quoted 5 lines

Quoted 4 lines

The thing is, I don't think anybody who uses these things cares about any of the 'problems' you want to fix, do they? We are interested in dirty pages only for the correctness issue, rather than performance. Same as reclaim.

Quoted 7 lines

Quoted 2 lines

If so, we can just stick to the dead slow but correct 'scan the full vma' page_mkclean() and nobody would ever trigger it.

Not if we restricted it to root and mlocked tmpfs. But then why wouldn't you just do it with the much more efficient msync walk, so that if root does want to do writeout via these things, it does not blow up?

Quoted 6 lines

This is all used on ram based filesystems right, they all have BDI_CAP_NO_WRITEBACK afaik, so page_mkclean will never get called anyway. Mlock doesn't avoid getting page_mkclean called.

Those who use this on a 'real' filesystem will get hit in the face by a linear scanning page_mkclean(), but AFAIK nobody does this anyway.

But somebody might do it. I just don't know why you'd want to make this _worse_ when the msync option would work?

Quoted 3 lines

Restricting it to root for such filesystems is unwanted, that'd severely handicap both UML and Oracle as I understand it (are there other users of this feature around?)

Why? I think they all use tmpfs backings, don't they?

Quoted 2 lines

msync() might never get called and then we're back with the old behaviour where we can surprise the VM with a ton of dirty pages.

But we're root. With your patch, root *can't* do nonlinear writeback well. Ever. With msync, at least you give them enough rope.

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

What is the DoS scenario wrt reclaim? We really ought to fix that if real, those UML farms run on nothing but nonlinear reclaim I'd think.

I guess you can just increase the computational complexity of reclaim quite easily.

Quoted 2 lines

Right, on first glance it doesn't look to be too bad, but I should take a closer look.

Well I don't think UML uses nonlinear yet anyway, does it? Can they make do with restricting nonlinear to mlocked vmas, I wonder? Probably not.

Quoted 3 lines

Well I don't think UML uses nonlinear yet anyway, does it? Can they make do with restricting nonlinear to mlocked vmas, I wonder? Probably not.

I think it does, but lets ask, Jeff?

True. We could even guesstimate the nonlinear dirty pages by subtracting the result of page_mkclean() from page_mapcount() and force an msync(MS_ASYNC) on said mapping (or all (nonlinear) mappings of the related file) when some threshold gets exceeded.

Quoted 4 lines · Peter Zijlstra

True. We could even guesstimate the nonlinear dirty pages by subtracting the result of page_mkclean() from page_mapcount() and force an msync(MS_ASYNC) on said mapping (or all (nonlinear) mappings of the related file) when some threshold gets exceeded.

Almost, but not quite, we'd need to extract another value from the page_mkclean() run, the actual number of mappings encountered. The return value only sums the number of dirty mappings encountered.

s390 would already work I guess.

Certainly doable.

I'm not at all happy with this, but plain disallowing remap_file_pages on bdis without BDI_CAP_NO_WRITEBACK seems to offend some people, hence restrict it to root only.

Quoted 4 lines · Peter Zijlstra

I'm not at all happy with this, but plain disallowing remap_file_pages on bdis without BDI_CAP_NO_WRITEBACK seems to offend some people, hence restrict it to root only.

I don't think that's a viable approach. Nonlinear mappings would normally be used by databases, and you don't want to limit databases to be run by root only.

Quoted 7 lines · Linus Torvalds

Quoted 4 lines · Peter Zijlstra

I'm not at all happy with this, but plain disallowing remap_file_pages on bdis without BDI_CAP_NO_WRITEBACK seems to offend some people, hence restrict it to root only.

Quoted 3 lines

I don't think that's a viable approach. Nonlinear mappings would normally be used by databases, and you don't want to limit databases to be run by root only.

It was claimed that they use it on tmpfs only, not on a 'real' filesystem.

Partial revert of commit: 204ec841fbea3e5138168edbc3a76d46747cc987

Non-linear vmas aren't properly handled by page_mkclean() and fixing that would result in linear scans of all related non-linear vmas per page_mkclean() invocation.

This is deemed too costly, hence re-instate the msync scan for non-linear vmas.

However this can lead to double IO:

- pages get instanciated with RO mapping
- page takes write fault, and gets marked with PG_dirty
- page gets tagged for writeout and calls page_mkclean()
- page_mkclean() fails to find the dirty pte (and clean it)
- writeout happens and PG_dirty gets cleared.
- user calls msync, the dirty pte is found and the page marked with PG_dirty
- the page gets writen out _again_ even though its not re-dirtied.

To minimize this reset the protection when creating a nonlinear vma.

I'm not at all happy with this, but plain disallowing remap_file_pages on bdis without BDI_CAP_NO_WRITEBACK seems to offend some people, hence restrict it to root only.

Quoted 18 lines

Partial revert of commit: 204ec841fbea3e5138168edbc3a76d46747cc987

Non-linear vmas aren't properly handled by page_mkclean() and fixing that would result in linear scans of all related non-linear vmas per page_mkclean() invocation.

This is deemed too costly, hence re-instate the msync scan for non-linear vmas.

However this can lead to double IO:

- pages get instanciated with RO mapping
- page takes write fault, and gets marked with PG_dirty
- page gets tagged for writeout and calls page_mkclean()
- page_mkclean() fails to find the dirty pte (and clean it)
- writeout happens and PG_dirty gets cleared.
- user calls msync, the dirty pte is found and the page marked with PG_dirty
- the page gets writen out _again_ even though its not re-dirtied.

Quoted 5 lines

To minimize this reset the protection when creating a nonlinear vma.

I'm not at all happy with this, but plain disallowing remap_file_pages on bdis without BDI_CAP_NO_WRITEBACK seems to offend some people, hence restrict it to root only.

Root only for !BDI_CAP_NO_WRITEBACK mappings doesn't make sense because:

- just encourages insecure applications

- there are no current users that want this and presumable no future uses either

- it's a maintenance burden: I'll have to layer the m/ctime update patch on top of this

- the only pro for this has been that Nick thinks it cool ;)

I think the proper way to deal with this is to

- allow BDI_CAP_NO_WRITEBACK (tmpfs/ramfs) uses, makes database people happy

- for !BDI_CAP_NO_WRITEBACK emulate using do_mmap_pgoff(), should be trivial, no userspace ABI breakage

However this still leaves the non-linear reclaim (Nick pointed it out as a potential DoS and other people have corroborated this). I have no idea on that to do about that.

Quoted 3 lines

However this still leaves the non-linear reclaim (Nick pointed it out as a potential DoS and other people have corroborated this). I have no idea on that to do about that.

OK, but that is a completely different problem, not affecting page_mkclean() or msync().

And it doesn't sound too hard to solve: when current algorithm doesn't seem to be making progress, then it will have to be done the hard way, searching for for all nonlinear ptes of a page to unmap.

Quoted 7 lines · Miklos Szeredi

Quoted 3 lines

However this still leaves the non-linear reclaim (Nick pointed it out as a potential DoS and other people have corroborated this). I have no idea on that to do about that.

OK, but that is a completely different problem, not affecting page_mkclean() or msync().

Quoted 3 lines

And it doesn't sound too hard to solve: when current algorithm doesn't seem to be making progress, then it will have to be done the hard way, searching for for all nonlinear ptes of a page to unmap.

Ah, you see, but that is when you've already lost.

The DoS is about the computational complexity of the reclaim, not if it will ever come out of it with free pages.

Quoted 8 lines

Quoted 3 lines

And it doesn't sound too hard to solve: when current algorithm doesn't seem to be making progress, then it will have to be done the hard way, searching for for all nonlinear ptes of a page to unmap.

Ah, you see, but that is when you've already lost.

The DoS is about the computational complexity of the reclaim, not if it will ever come out of it with free pages.

If we really want to, we could limit it to mlock for !root. This is a reasonable way to solve the problem, and UML could fall back on vma emulated version if they didn't want to use mlock memory...

Or we could limit the size/number of nonlinear vmas that could be created.

But just quietly, I think there are probably a lot of other ways to perform a local DoS anyway ;)

Quoted 18 lines

Partial revert of commit: 204ec841fbea3e5138168edbc3a76d46747cc987

Non-linear vmas aren't properly handled by page_mkclean() and fixing that would result in linear scans of all related non-linear vmas per page_mkclean() invocation.

This is deemed too costly, hence re-instate the msync scan for non-linear vmas.

However this can lead to double IO:

- pages get instanciated with RO mapping
- page takes write fault, and gets marked with PG_dirty
- page gets tagged for writeout and calls page_mkclean()
- page_mkclean() fails to find the dirty pte (and clean it)
- writeout happens and PG_dirty gets cleared.
- user calls msync, the dirty pte is found and the page marked with PG_dirty
- the page gets writen out _again_ even though its not re-dirtied.

Quoted 5 lines

To minimize this reset the protection when creating a nonlinear vma.

I'm not at all happy with this, but plain disallowing remap_file_pages on bdis without BDI_CAP_NO_WRITEBACK seems to offend some people, hence restrict it to root only.

Root only for !BDI_CAP_NO_WRITEBACK mappings doesn't make sense because:

- just encourages insecure applications

- there are no current users that want this and presumable no future uses either

- it's a maintenance burden: I'll have to layer the m/ctime update patch on top of this

- the only pro for this has been that Nick thinks it cool ;)

I think the proper way to deal with this is to

- allow BDI_CAP_NO_WRITEBACK (tmpfs/ramfs) uses, makes database people happy

- for !BDI_CAP_NO_WRITEBACK emulate using do_mmap_pgoff(), should be trivial, no userspace ABI breakage

Quoted 2 lines

- it's a maintenance burden: I'll have to layer the m/ctime update patch on top of this

But you have to update m/ctime for BDI_CAP_NO_WRITEBACK mappings anyway don't you?

Quoted 1 line

- the only pro for this has been that Nick thinks it cool ;)

Nonlinear in general, rather than this specifically.

Quoted 3 lines

Well I don't think UML uses nonlinear yet anyway, does it? Can they make do with restricting nonlinear to mlocked vmas, I wonder? Probably not.

I think it does, but lets ask, Jeff?

Quoted 15 lines · Nick Piggin

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

The thing is, I don't think anybody who uses these things cares about any of the 'problems' you want to fix, do they? We are interested in dirty pages only for the correctness issue, rather than performance. Same as reclaim.

Quoted 7 lines

Quoted 2 lines

If so, we can just stick to the dead slow but correct 'scan the full vma' page_mkclean() and nobody would ever trigger it.

Not if we restricted it to root and mlocked tmpfs. But then why wouldn't you just do it with the much more efficient msync walk, so that if root does want to do writeout via these things, it does not blow up?

Quoted 6 lines

This is all used on ram based filesystems right, they all have BDI_CAP_NO_WRITEBACK afaik, so page_mkclean will never get called anyway. Mlock doesn't avoid getting page_mkclean called.

Those who use this on a 'real' filesystem will get hit in the face by a linear scanning page_mkclean(), but AFAIK nobody does this anyway.

But somebody might do it. I just don't know why you'd want to make this _worse_ when the msync option would work?

Quoted 3 lines

Restricting it to root for such filesystems is unwanted, that'd severely handicap both UML and Oracle as I understand it (are there other users of this feature around?)

Why? I think they all use tmpfs backings, don't they?

Quoted 2 lines

msync() might never get called and then we're back with the old behaviour where we can surprise the VM with a ton of dirty pages.

But we're root. With your patch, root *can't* do nonlinear writeback well. Ever. With msync, at least you give them enough rope.

Quoted 5 lines

Quoted 2 lines

msync() might never get called and then we're back with the old behaviour where we can surprise the VM with a ton of dirty pages.

But we're root. With your patch, root *can't* do nonlinear writeback well. Ever. With msync, at least you give them enough rope.

Restricting to root doesn't buy you much, nobody wants to be root. Restricting to mlock is similarly pointless. UML _will_ want to get swapped out if there's no activity.

Restricting to tmpfs makes sense, but it's probably not what UML wants.

Quoted 1 line

Well, now they don't, but it could be done or even exploited as a DoS.

But so could nonlinear page reclaim. I think we need to restrict nonlinear mappings to root if we're worried about that.

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Well, now they don't, but it could be done or even exploited as a DoS.

On Wed, Mar 07, 2007 at 12:00:36PM +0100, Nick Piggin wrote:

Quoted 2 lines

But so could nonlinear page reclaim. I think we need to restrict nonlinear mappings to root if we're worried about that.

Please not root. The users really don't want to be privileged. UML itself is at least partly for use as privilege isolation of the guest workload. Oracle has some of the same concerns itself, which is part of why it uses separate processes heavily, even: to isolate instances from each other.

Quoted 2 lines · Nick Piggin

Then 4,5,6 is the fault/nonlinear rewrite, take it or leave it. I thought you would have liked the patches...

btw., if we decide that nonlinear isnt worth the continuing maintainance pain, we could internally implement/emulate sys_remap_file_pages() via a call to mremap() and essentially deprecate it, without breaking the ABI
- and remove all the nonlinear code. (This would split fremap areas into separate vmas)

Apart from a handful of trivial if (pte_file()) cases throughout mm/, our maintainance burden basically now amounts to the following patch. Even the rmap.c change looks bigger than it is because I split out the nonlinear unmapping code from try_to_unmap_file. Not too bad, eh? :)

After these patches, I don't think there is too much burden. The main thing left really is just the objrmap stuff, but that is just handled with a minimal 'dumb' algorithm that doesn't cost much.

Quoted 3 lines

After these patches, I don't think there is too much burden. The main thing left really is just the objrmap stuff, but that is just handled with a minimal 'dumb' algorithm that doesn't cost much.

ok. What do you think about the sys_remap_file_pages_prot() thing that Paolo has done in a nicely split up form - does that complicate things in any fundamental way? That is what is useful to UML.

Quoted 3 lines · Nick Piggin

Quoted 3 lines

After these patches, I don't think there is too much burden. The main thing left really is just the objrmap stuff, but that is just handled with a minimal 'dumb' algorithm that doesn't cost much.

Quoted 3 lines · Ingo Molnar

ok. What do you think about the sys_remap_file_pages_prot() thing that Paolo has done in a nicely split up form - does that complicate things in any fundamental way? That is what is useful to UML.

Oracle would love it. You don't want to know how far back I've been asked to backport that.

Quoted 3 lines

Quoted 3 lines

ok. What do you think about the sys_remap_file_pages_prot() thing that Paolo has done in a nicely split up form - does that complicate things in any fundamental way? That is what is useful to UML.

Quoted 2 lines

Oracle would love it. You don't want to know how far back I've been asked to backport that.

ok, cool! Then the first step would be for you to talk to Paolo and to pick up the patches, review them, nurse it in -mm, etc. Suffering in silence is just a pointless act of masochism, not an efficient upstream-merge tactic ;-)

Quoted 3 lines · Nick Piggin

After these patches, I don't think there is too much burden. The main thing left really is just the objrmap stuff, but that is just handled with a minimal 'dumb' algorithm that doesn't cost much.

ok. What do you think about the sys_remap_file_pages_prot() thing that Paolo has done in a nicely split up form - does that complicate things in any fundamental way? That is what is useful to UML.

Does anybody really pass a NULL `type' arg into filemap_nopage()?

Quoted 2 lines

NOPAGE_REFAULT is removed. This should be implemented with ->fault, and no users have hit mainline yet.

Did benh agree with that?

Quoted 5 lines

Quoted 3 lines

NOPAGE_REFAULT is removed. This should be implemented with ->fault, and no users have hit mainline yet.

Did benh agree with that?

I won't use NOPAGE_REFAULT, I use NOPFN_REFAULT and that has hit mainline. I will switch to ->fault when I have time to adapt the code, in the meantime, NOPFN_REFAULT should stay.

Quoted 4 lines · Benjamin Herrenschmidt

Quoted 4 lines

Quoted 3 lines

NOPAGE_REFAULT is removed. This should be implemented with ->fault, and no users have hit mainline yet.

Quoted 5 lines

Quoted 1 line

Did benh agree with that?

I won't use NOPAGE_REFAULT, I use NOPFN_REFAULT and that has hit mainline. I will switch to ->fault when I have time to adapt the code, in the meantime, NOPFN_REFAULT should stay.

I think I removed not only NOFPN_REFAULT, but also nopfn itself, *and* adapted the code for you ;) it is in patch 5/6, sent a while ago.

Remove ->nopfn and reimplement the existing handlers with ->fault

Signed-off-by Nick Piggin <[email protected]>