page_mkwrite seems broken

On Fri, 4 Feb 2005, Hugh Dickins wrote in another thread:

I have now looked further, and both points still seem valid to me:
the page_mkwrite calling code looks doubly broken. (Tested?)

Nor has there been any movement on the points raised by Christoph, that aops->page_mkwrite is redundant, and do_wp_page_mk_pte_writable separation unhelpful.

I could probably put page_mkwrite to use in tmpfs (to eliminate its unsatisfactory but never over-troubling shmem_recalc_inode), but not as it currently stands.

Are you planning any movement on this, David?
Or should I have a go sometime?

Hugh

Hi,

What happened with page_mkwrite? It seems to have disappeared both from -mm and generally from the face of the earth...

I am very interested in having such ability for ntfs...

Is anyone still working on this? If not why not? Did it prove impractical or ...?

If no-one is working on this anymore, where do I find the last "current" patch?

Thanks a lot in advance!

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

It got taken out because no one was using it (CacheFS has been removed temorarily).

I'm still attempting to maintain it. If you want I can post it to Andrew again to see if he'll take it back. If you want a direct copy, I'll have to extract it from CacheFS.

David

I don't really mind either way. I am stuck with ntfs at the moment at the point where I am either going to use my own ->nopage handler to allocate on-disk clusters or have a ->page_mkwrite handler do it. The former is not nice as it means we allocate space even when only reading whilst the later is very nice as it only triggers when someone actually does an mmapped write.

So whatever works best for you. I am happy with it appearing in -mm and I am also happy with having my own copy in my ntfs development tree for now. Given we both want it and High said he wanted it, too, it may be the more sensible approach to have it in -mm so we have one common code base to work with/apply fixes to/whatever... Otherwise we may get headaches down the line if we end up with diverging implementations of the same thing and all try to merge ours into the kernel...

Btw. have you addressed the problems Hugh pointed out with it? If not, -mm would perhaps be a good place for us to get it sorted?

Andrew, would you be happy to take the ->page_mkwrite patches again into -mm? I promise you a user of it to appear in -mm as soon as I can knock up the ntfs code for it once I have seen the exact interface I am coding for...

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

A complication to beware of there (and I may be misunderstanding, but the point is worth making). If you have already mmaped readonly zero pages into some mms, you'll need to update those mms with the new shared writable pages once they are allocated. That put me off using page_mkwrite in tmpfs, but Carsten has solved the problem (though not going so far as to use page_mkwrite) with his xip_file_nopage in mm/filemap_xip.c - has to go down the vma_prio_tree like rmap.

(That code is a little different in -mm, partly because of my page table locking changes, partly because of Nick's ZERO_PAGE changes.)

Hmm, strictly speaking, it should be substituting the new page when VM_LOCKED: whether that's worth the effort of implementing....

Hugh

Now you got me completely confused. Just when I thought I was understanding things. (-; Let me repeat what you say with some questions thrown in... Please bear with me and help me beat some clue into my head... (-:

When you say "zero pages" you mean just normal page cache pages that are fully zero because they are in a sparse region of a file?

Or do you mean ZERO_PAGE(address) stuff like in xip_file_nopage()?

And when you say "mapped readonly zero pages into some mms" you mean that there are several processes which have done mmap(PROT_READ, MAP_SHARED) on the same file, right?

Or do you mean mmap(PROT_READ|PROT_WRITE, MAP_SHARED)?

Or something else?

If your answer above is that the pages are normal page cache pages, then:

Is it to reflect the fact that the pages are now marked writable?

Or is it to reflect the fact that the pages are now allocated on disk (i.e. update the page buffers)?

Or am I barking up the wrong tree and the reasons are altogether different?

If your answer above was ZERO_PAGE(), then:

Is it to get rid of the zero page and replace it with the _real_, now allocated and writable page?

I have now read filemap_xip.c as it is in Linus kernel and see the ZERO_PAGE(). I guess that that is what you were talking about above all along and not normal page cache pages that happen to be zero. Correct?

In which case am I correct in saying that as long as I use filemap_nopage() and filemap_populate(), I can ignore your comment about updating mms as ZERO_PAGE() will _never_ be mapped and in fact just normal page cache pages containing zeroes will be mapped?

If that is correct then great. Otherwise I have missed the plot and would be very grateful if you were to impart some clue upon me. (-:

Thanks a lot for your help! Much appreciated!

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Sorry for confusing you. I can't answer many of your questions, because I don't know what you're doing or intending to do. But you expressed an aversion to allocating pages unnecessarily. Probably that made me think of memory allocation where you meant disk allocation.

Cutting a lot of questions...

Nothing special needs doing if you choose to use normal page cache pages even for the holes.

Yes.

Yes.

Yes.

Sorry for the confusion: I was just trying to warn you of some difficulties and their solution, if you were intending to pursue an alternative path.

Hugh

Great! I have no intention of using ZERO_PAGE(). Just normal page cache pages that are memset() to zero when sparse. Phew. /me relaxes (((-:

No need to apologize!

If I had wanted to use the ZERO_PAGE() then you would be right and I would have missed all those things you said, but I never even knew about ZERO_PAGE(). (-: I could well see that as a nice optimization at some point but for now I want it to work, not conserve memory. (-:

Thank you very much for your comments!

In case you are curious, ntfs allows logical blocks of between 512 bytes and many hundreds of kiB in size (but always power of 2). So to write to a mmap()ed sparse file using a PAGE_CACHE_SIZE page into the middle of a large, sparse logical block, I need to allocate the whole block on disk and cause all page cache pages to be zeroed and marked dirty. To do this from writepage() is not possible due to deadlocks. 1) because the page is locked already and I would need to lock all the other pages in that logical block so we get into deadlock city with out of order page locking (I now only lock in ascending page index order and this requires no page with a higher index to be locked and dropping page lock in writepage is royal pain in the backside) and 2) because I am not meant to go allocating memory for more pages when the system is low on memory and running writepage exactly so it can reclaim some memory...

How I want to use page_mkdirty is that when it is run for a sparse logical block of size > PAGE_CACHE_SIZE, I allocate the logical block and get hold of all the pages (locked) that lie in that block and bring them uptodate (by zeroing if not uptodate already) and then mark them all dirty and release them again so the zeroes will make it to disk later. Not sure whether to do the allocations even for logical blocks <= PAGE_CACHE_SIZE or just leave those to writepage...

In fact before allocating the block I plan to simply do a page cache read (via read_cache_page() which will give me uptodate, cleared pages) then do the allocation, then mark the pages dirty and that's it. Writepage will later cause the buffers in the pages to be mapped to the new on-disk location and will write the dirty pages to disk. (I may map the buffers in the pages there an then as an optimization given I have the pages and I know the on-disk location but I am not sure I will do that, at least probably not initially as it only makes the code more complex for very little gain.)

There is another ntfs complication and this is initialized size. This is an evil beast that say that anything between initialized size and the real file size (inode->i_size), no matter whether it is allocated on disk or is a sparse hole or a mixture of the two, is to be read as zeroes. The annoying thing here is that if you have a 1TiB file that is fully allocated on disk but has an initialized size of 0, and you write 1 byte somewhere towards the end of the file (or even at the end), you need to write to disk zeroes between file offset = initialized size (0 in this example) and the position of the write, in this case 1TiB. Doing that from writepage could never fly. But from page_mkdirty() it should work, again in same way as above for the sparse holes, I will read_cache_page() followed by set_page_dirty() for all pages between initialized size and the offset of the write. It just means that the first write to such an mmaped file would take a _very_ long time in the specific example above...

Note for the above I plan to leverage fs/ntfs/file.c::ntfs_attr_extend_initialized() or at least an adapted form of it. This function does the above described but for the file write(2) case where a user opens a file and writes somewhere beyond the initialized size...

I hope that explains what and why I am doing and I also hope that if you were not interested you didn't bother reading it all and hence never see this sentence. (-;

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

The attached patch adds a new VMA operation to notify a filesystem or other driver about the MMU generating a fault because userspace attempted to write to a page mapped through a read-only PTE.

This facility permits the filesystem or driver to:

(*) Implement storage allocation/reservation on attempted write, and so to
deal with problems such as ENOSPC more gracefully (perhaps by generating SIGBUS).

(*) Delay making the page writable until the contents have been written to a
backing cache. This is useful for NFS/AFS when using FS-Cache/CacheFS. It permits the filesystem to have some guarantee about the state of the cache.

---
warthog>diffstat -p1 page-mkwrite-2614rc4mm1.diff

 include/linux/mm.h |    4 +
 mm/memory.c        |  125 ++++++++++++++++++++++++++++++++++++++++++-----------
 mm/mmap.c          |    9 ++-
 mm/mprotect.c      |    8 ++-
 4 files changed, 117 insertions(+), 29 deletions(-)

diff -uNrp linux-2.6.14-rc4-mm1/include/linux/mm.h linux-2.6.14-rc4-mm1-cachefs/include/linux/mm.h
@@ -196,6 +196,10 @@ struct vm_operations_struct {
 	void (*close)(struct vm_area_struct * area);
 	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int *type);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
+
+	/* notification that a previously read-only page is about to become
+	 * writable, if an error is returned it will cause a SIGBUS */
+	int (*page_mkwrite)(struct vm_area_struct *vma, struct page *page);
 #ifdef CONFIG_NUMA
 	int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
 	struct mempolicy *(*get_policy)(struct vm_area_struct *vma,

diff -uNrp linux-2.6.14-rc4-mm1/mm/mmap.c linux-2.6.14-rc4-mm1-cachefs/mm/mmap.c
@@ -1058,7 +1058,8 @@ munmap_back:
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = vm_flags;
-	vma->vm_page_prot = protection_map[vm_flags & 0x0f];
+	vma->vm_page_prot = protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma->vm_pgoff = pgoff;
 
 	if (file) {

@@ -1092,6 +1093,9 @@ munmap_back:
 		if (error)
 			goto free_vma;
 	}
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		vma->vm_page_prot = protection_map[vm_flags &
+					(VM_READ|VM_WRITE|VM_EXEC)];
 
 	/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
 	 * shmem_zero_setup (perhaps called through /dev/zero's ->mmap)

@@ -1926,7 +1930,8 @@ unsigned long do_brk(unsigned long addr,
 	vma->vm_end = addr + len;
 	vma->vm_pgoff = pgoff;
 	vma->vm_flags = flags;
-	vma->vm_page_prot = protection_map[flags & 0x0f];
+	vma->vm_page_prot = protection_map[flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 out:
 	mm->total_vm += len >> PAGE_SHIFT;

diff -uNrp linux-2.6.14-rc4-mm1/mm/mprotect.c linux-2.6.14-rc4-mm1-cachefs/mm/mprotect.c
@@ -106,6 +106,7 @@ mprotect_fixup(struct vm_area_struct *vm
 	unsigned long oldflags = vma->vm_flags;
 	long nrpages = (end - start) >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	unsigned int mask;
 	pgprot_t newprot;
 	pgoff_t pgoff;
 	int error;

@@ -140,8 +141,6 @@ mprotect_fixup(struct vm_area_struct *vm
 		}
 	}
 
-	newprot = protection_map[newflags & 0xf];
-
 	/*
 	 * First try to merge with previous and/or next vma.
 	 */

@@ -168,6 +167,11 @@ mprotect_fixup(struct vm_area_struct *vm
 	}
 
 success:
+	mask = VM_READ|VM_WRITE|VM_EXEC|VM_SHARED;
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		mask &= ~VM_SHARED;
+	newprot = protection_map[newflags & mask];
+
 	/*
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.

I've only given it a quick look, it looks pretty good, but too hastily thrown together, without understanding of the intervening changes:

There really is quite a difference between mm/*.c in -mm and Linus kernel at present. Is all this planned to be merged as soon as 2.6.14 is out or is -mm just a playground for now with no mainline merge intentions?

Just asking so I know whether to work against stock kernels or -mm for the moment...

[snip some corrections I am in no position to comment on at the moment]

Cool, that reduces the size of the patch. (-:

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

It certainly won't all be merged as soon as 2.6.14 is out, some of it has only just got into -mm. Andrew's current intention is to merge the early part of the changes soonish after 2.6.14 gets out, but he's not likely to merge it all into 2.6.15.

But we aren't using -mm as a playground: it is likely to go forward, provided it doesn't show regressions of some kind while it's in -mm.

I'd recommend -mm for now. page_mkwrite will want a spell in there too, won't it?

Hugh

Ok, sounds good. As long as they at least start converging...

Cool.

Great, thanks, will do.

Sure. But if the other mm changes in -mm were not going forward it would be a little silly to get page_mkwrite to work there only to have to rewrite it in order to get it merged...

If Linus really is going to release .14 in the next few days, page_mkwrite is never going to make it into .15 anyway, no matter what... But .16 would be a realistic target I would have thought which seems to fit in nicely with the plans for -mm. (-:

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

I attempted to

I attempted to forward port your patch; unfortunately, I'm not fully conversant with some of the VM stuff.

It's as it was in your patch.

I'll try and fix the changes.

David

You're right. I've added a release. That may explain a memory leak I was seeing that I couldn't find.

I take it then that:

(1) the write_access parameter to do_file_page() is there purely so that handle_pte_fault() can jump to it rather than calling it since they have the same parameter set and return value;

(2) and that do_file_page() always installs a read-only PTE so that do_wp_page() will be called subsequently on a write attempt.

David

The attached patch adds a new VMA operation to notify a filesystem or other driver about the MMU generating a fault because userspace attempted to write to a page mapped through a read-only PTE.

This facility permits the filesystem or driver to:

(*) Implement storage allocation/reservation on attempted write, and so to
deal with problems such as ENOSPC more gracefully (perhaps by generating SIGBUS).

(*) Delay making the page writable until the contents have been written to a
backing cache. This is useful for NFS/AFS when using FS-Cache/CacheFS. It permits the filesystem to have some guarantee about the state of the cache.

---
warthog>diffstat -p1 page-mkwrite-2614rc4mm1-2.diff

 include/linux/mm.h |    4 ++
 mm/memory.c        |   95 +++++++++++++++++++++++++++++++++++++++--------------
 mm/mmap.c          |    9 +++--
 mm/mprotect.c      |    8 +++-
 4 files changed, 88 insertions(+), 28 deletions(-)

diff -uNrp linux-2.6.14-rc4-mm1/include/linux/mm.h linux-2.6.14-rc4-mm1-cachefs/include/linux/mm.h
@@ -196,6 +196,10 @@ struct vm_operations_struct {
 	void (*close)(struct vm_area_struct * area);
 	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int *type);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
+
+	/* notification that a previously read-only page is about to become
+	 * writable, if an error is returned it will cause a SIGBUS */
+	int (*page_mkwrite)(struct vm_area_struct *vma, struct page *page);
 #ifdef CONFIG_NUMA
 	int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
 	struct mempolicy *(*get_policy)(struct vm_area_struct *vma,

diff -uNrp linux-2.6.14-rc4-mm1/mm/mmap.c linux-2.6.14-rc4-mm1-cachefs/mm/mmap.c
@@ -1058,7 +1058,8 @@ munmap_back:
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = vm_flags;
-	vma->vm_page_prot = protection_map[vm_flags & 0x0f];
+	vma->vm_page_prot = protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma->vm_pgoff = pgoff;
 
 	if (file) {

@@ -1092,6 +1093,9 @@ munmap_back:
 		if (error)
 			goto free_vma;
 	}
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		vma->vm_page_prot = protection_map[vm_flags &
+					(VM_READ|VM_WRITE|VM_EXEC)];
 
 	/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
 	 * shmem_zero_setup (perhaps called through /dev/zero's ->mmap)

@@ -1926,7 +1930,8 @@ unsigned long do_brk(unsigned long addr,
 	vma->vm_end = addr + len;
 	vma->vm_pgoff = pgoff;
 	vma->vm_flags = flags;
-	vma->vm_page_prot = protection_map[flags & 0x0f];
+	vma->vm_page_prot = protection_map[flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 out:
 	mm->total_vm += len >> PAGE_SHIFT;

diff -uNrp linux-2.6.14-rc4-mm1/mm/mprotect.c linux-2.6.14-rc4-mm1-cachefs/mm/mprotect.c
@@ -106,6 +106,7 @@ mprotect_fixup(struct vm_area_struct *vm
 	unsigned long oldflags = vma->vm_flags;
 	long nrpages = (end - start) >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	unsigned int mask;
 	pgprot_t newprot;
 	pgoff_t pgoff;
 	int error;

@@ -140,8 +141,6 @@ mprotect_fixup(struct vm_area_struct *vm
 		}
 	}
 
-	newprot = protection_map[newflags & 0xf];
-
 	/*
 	 * First try to merge with previous and/or next vma.
 	 */

@@ -168,6 +167,11 @@ mprotect_fixup(struct vm_area_struct *vm
 	}
 
 success:
+	mask = VM_READ|VM_WRITE|VM_EXEC|VM_SHARED;
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		mask &= ~VM_SHARED;
+	newprot = protection_map[newflags & mask];
+
 	/*
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.

The attached patch adds a new VMA operation to notify a filesystem or other driver about the MMU generating a fault because userspace attempted to write to a page mapped through a read-only PTE.

This facility permits the filesystem or driver to:

(*) Implement storage allocation/reservation on attempted write, and so to
deal with problems such as ENOSPC more gracefully (perhaps by generating SIGBUS).

(*) Delay making the page writable until the contents have been written to a
backing cache. This is useful for NFS/AFS when using FS-Cache/CacheFS. It permits the filesystem to have some guarantee about the state of the cache.

Updated to 2.6.14-git14.

---
warthog>diffstat -p1 page-mkwrite-2614git14.diff

 include/linux/mm.h |    4 ++
 mm/memory.c        |   95 +++++++++++++++++++++++++++++++++++++++--------------
 mm/mmap.c          |   12 +++++-
 mm/mprotect.c      |   11 +++++-
 4 files changed, 94 insertions(+), 28 deletions(-)

diff -uNrp linux-2.6.14-git14/include/linux/mm.h linux-2.6.14-git14-pagenotify/include/linux/mm.h
@@ -196,6 +196,10 @@ struct vm_operations_struct {
 	void (*close)(struct vm_area_struct * area);
 	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int *type);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
+
+	/* notification that a previously read-only page is about to become
+	 * writable, if an error is returned it will cause a SIGBUS */
+	int (*page_mkwrite)(struct vm_area_struct *vma, struct page *page);
 #ifdef CONFIG_NUMA
 	int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
 	struct mempolicy *(*get_policy)(struct vm_area_struct *vma,

diff -uNrp linux-2.6.14-git14/mm/mmap.c linux-2.6.14-git14-pagenotify/mm/mmap.c
@@ -1058,7 +1058,8 @@ munmap_back:
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = vm_flags;
-	vma->vm_page_prot = protection_map[vm_flags & 0x0f];
+	vma->vm_page_prot = protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma->vm_pgoff = pgoff;
 
 	if (file) {

@@ -1093,6 +1094,12 @@ munmap_back:
 			goto free_vma;
 	}
 
+	/* Don't make the VMA automatically writable if it's shared, but the
+	 * backer wishes to know when pages are first written to */
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		vma->vm_page_prot =
+			protection_map[vm_flags & (VM_READ|VM_WRITE|VM_EXEC)];
+
 	/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
 	 * shmem_zero_setup (perhaps called through /dev/zero's ->mmap)
 	 * that memory reservation must be checked; but that reservation

@@ -1926,7 +1933,8 @@ unsigned long do_brk(unsigned long addr,
 	vma->vm_end = addr + len;
 	vma->vm_pgoff = pgoff;
 	vma->vm_flags = flags;
-	vma->vm_page_prot = protection_map[flags & 0x0f];
+	vma->vm_page_prot = protection_map[flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 out:
 	mm->total_vm += len >> PAGE_SHIFT;

diff -uNrp linux-2.6.14-git14/mm/mprotect.c linux-2.6.14-git14-pagenotify/mm/mprotect.c
@@ -106,6 +106,7 @@ mprotect_fixup(struct vm_area_struct *vm
 	unsigned long oldflags = vma->vm_flags;
 	long nrpages = (end - start) >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	unsigned int mask;
 	pgprot_t newprot;
 	pgoff_t pgoff;
 	int error;

@@ -140,8 +141,6 @@ mprotect_fixup(struct vm_area_struct *vm
 		}
 	}
 
-	newprot = protection_map[newflags & 0xf];
-
 	/*
 	 * First try to merge with previous and/or next vma.
 	 */

@@ -168,6 +167,14 @@ mprotect_fixup(struct vm_area_struct *vm
 	}
 
 success:
+	/* Don't make the VMA automatically writable if it's shared, but the
+	 * backer wishes to know when pages are first written to */
+	mask = VM_READ|VM_WRITE|VM_EXEC|VM_SHARED;
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		mask &= ~VM_SHARED;
+
+	newprot = protection_map[newflags & mask];
+
 	/*
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.

(*) account and limit number of dirty pages

This is one piece of the puzze needed to make shared writable mapping work safely in FUSE.

But doesn't apply against 2.6.15-rc3 or -rc3-mm1.

Miklos

Hmmm... It would appear that "The latest snapshot for the stable Linux kernel tree" is a bit out of date on www.kernel.org. I should've checked the dates.

David

The attached patch adds a new VMA operation to notify a filesystem or other driver about the MMU generating a fault because userspace attempted to write to a page mapped through a read-only PTE.

This facility permits the filesystem or driver to:

(*) Implement storage allocation/reservation on attempted write, and so to
deal with problems such as ENOSPC more gracefully (perhaps by generating SIGBUS).

(*) Delay making the page writable until the contents have been written to a
backing cache. This is useful for NFS/AFS when using FS-Cache/CacheFS. It permits the filesystem to have some guarantee about the state of the cache.

(*) Account and limit number of dirty pages. This is one piece of the puzzle
needed to make shared writable mapping work safely in FUSE.

Updated to 2.6.15-rc3.

---
warthog>diffstat -p1 page-mkwrite-2615rc3.diff

 include/linux/mm.h |    4 ++
 mm/memory.c        |   95 +++++++++++++++++++++++++++++++++++++++--------------
 mm/mmap.c          |   12 +++++-
 mm/mprotect.c      |   11 +++++-
 4 files changed, 94 insertions(+), 28 deletions(-)

diff -uNrp linux-2.6.15-rc3/include/linux/mm.h linux-2.6.15-rc3-page-mkwrite/include/linux/mm.h
@@ -197,6 +197,10 @@ struct vm_operations_struct {
 	void (*close)(struct vm_area_struct * area);
 	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int *type);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
+
+	/* notification that a previously read-only page is about to become
+	 * writable, if an error is returned it will cause a SIGBUS */
+	int (*page_mkwrite)(struct vm_area_struct *vma, struct page *page);
 #ifdef CONFIG_NUMA
 	int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
 	struct mempolicy *(*get_policy)(struct vm_area_struct *vma,

diff -uNrp linux-2.6.15-rc3/mm/mmap.c linux-2.6.15-rc3-page-mkwrite/mm/mmap.c
@@ -1058,7 +1058,8 @@ munmap_back:
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = vm_flags;
-	vma->vm_page_prot = protection_map[vm_flags & 0x0f];
+	vma->vm_page_prot = protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma->vm_pgoff = pgoff;
 
 	if (file) {

@@ -1082,6 +1083,12 @@ munmap_back:
 			goto free_vma;
 	}
 
+	/* Don't make the VMA automatically writable if it's shared, but the
+	 * backer wishes to know when pages are first written to */
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		vma->vm_page_prot =
+			protection_map[vm_flags & (VM_READ|VM_WRITE|VM_EXEC)];
+
 	/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
 	 * shmem_zero_setup (perhaps called through /dev/zero's ->mmap)
 	 * that memory reservation must be checked; but that reservation

@@ -1915,7 +1922,8 @@ unsigned long do_brk(unsigned long addr,
 	vma->vm_end = addr + len;
 	vma->vm_pgoff = pgoff;
 	vma->vm_flags = flags;
-	vma->vm_page_prot = protection_map[flags & 0x0f];
+	vma->vm_page_prot = protection_map[flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 out:
 	mm->total_vm += len >> PAGE_SHIFT;

diff -uNrp linux-2.6.15-rc3/mm/mprotect.c linux-2.6.15-rc3-page-mkwrite/mm/mprotect.c
@@ -106,6 +106,7 @@ mprotect_fixup(struct vm_area_struct *vm
 	unsigned long oldflags = vma->vm_flags;
 	long nrpages = (end - start) >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	unsigned int mask;
 	pgprot_t newprot;
 	pgoff_t pgoff;
 	int error;

@@ -132,8 +133,6 @@ mprotect_fixup(struct vm_area_struct *vm
 		}
 	}
 
-	newprot = protection_map[newflags & 0xf];
-
 	/*
 	 * First try to merge with previous and/or next vma.
 	 */

@@ -160,6 +159,14 @@ mprotect_fixup(struct vm_area_struct *vm
 	}
 
 success:
+	/* Don't make the VMA automatically writable if it's shared, but the
+	 * backer wishes to know when pages are first written to */
+	mask = VM_READ|VM_WRITE|VM_EXEC|VM_SHARED;
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		mask &= ~VM_SHARED;
+
+	newprot = protection_map[newflags & mask];
+
 	/*
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.

The attached patch adds a new VMA operation to notify a filesystem or other driver about the MMU generating a fault because userspace attempted to write to a page mapped through a read-only PTE.

This facility permits the filesystem or driver to:

(*) Implement storage allocation/reservation on attempted write, and so to
deal with problems such as ENOSPC more gracefully (perhaps by generating SIGBUS).

(*) Delay making the page writable until the contents have been written to a
backing cache. This is useful for NFS/AFS when using FS-Cache/CacheFS. It permits the filesystem to have some guarantee about the state of the cache.

(*) Account and limit number of dirty pages. This is one piece of the puzzle
needed to make shared writable mapping work safely in FUSE.

Updated to 2.6.15-mm2.

---
warthog>diffstat -p1 page-mkwrite-2615mm2.diff

 include/linux/mm.h |    4 ++
 mm/memory.c        |   99 ++++++++++++++++++++++++++++++++++++++++-------------
 mm/mmap.c          |   12 +++++-
 mm/mprotect.c      |   11 ++++-
 4 files changed, 98 insertions(+), 28 deletions(-)

diff -uNrp linux-2.6.15-mm2/include/linux/mm.h linux-2.6.15-mm2-page-mkwrite/include/linux/mm.h
@@ -199,6 +199,10 @@ struct vm_operations_struct {
 	void (*close)(struct vm_area_struct * area);
 	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int *type);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
+
+	/* notification that a previously read-only page is about to become
+	 * writable, if an error is returned it will cause a SIGBUS */
+	int (*page_mkwrite)(struct vm_area_struct *vma, struct page *page);
 #ifdef CONFIG_NUMA
 	int (*set_policy)(struct vm_area_struct *vma, struct mempolicy *new);
 	struct mempolicy *(*get_policy)(struct vm_area_struct *vma,

diff -uNrp linux-2.6.15-mm2/mm/mmap.c linux-2.6.15-mm2-page-mkwrite/mm/mmap.c
@@ -1058,7 +1058,8 @@ munmap_back:
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = vm_flags;
-	vma->vm_page_prot = protection_map[vm_flags & 0x0f];
+	vma->vm_page_prot = protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma->vm_pgoff = pgoff;
 
 	if (file) {

@@ -1082,6 +1083,12 @@ munmap_back:
 			goto free_vma;
 	}
 
+	/* Don't make the VMA automatically writable if it's shared, but the
+	 * backer wishes to know when pages are first written to */
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		vma->vm_page_prot =
+			protection_map[vm_flags & (VM_READ|VM_WRITE|VM_EXEC)];
+
 	/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
 	 * shmem_zero_setup (perhaps called through /dev/zero's ->mmap)
 	 * that memory reservation must be checked; but that reservation

@@ -1915,7 +1922,8 @@ unsigned long do_brk(unsigned long addr,
 	vma->vm_end = addr + len;
 	vma->vm_pgoff = pgoff;
 	vma->vm_flags = flags;
-	vma->vm_page_prot = protection_map[flags & 0x0f];
+	vma->vm_page_prot = protection_map[flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 out:
 	mm->total_vm += len >> PAGE_SHIFT;

diff -uNrp linux-2.6.15-mm2/mm/mprotect.c linux-2.6.15-mm2-page-mkwrite/mm/mprotect.c
@@ -106,6 +106,7 @@ mprotect_fixup(struct vm_area_struct *vm
 	unsigned long oldflags = vma->vm_flags;
 	long nrpages = (end - start) >> PAGE_SHIFT;
 	unsigned long charged = 0;
+	unsigned int mask;
 	pgprot_t newprot;
 	pgoff_t pgoff;
 	int error;

@@ -132,8 +133,6 @@ mprotect_fixup(struct vm_area_struct *vm
 		}
 	}
 
-	newprot = protection_map[newflags & 0xf];
-
 	/*
 	 * First try to merge with previous and/or next vma.
 	 */

@@ -160,6 +159,14 @@ mprotect_fixup(struct vm_area_struct *vm
 	}
 
 success:
+	/* Don't make the VMA automatically writable if it's shared, but the
+	 * backer wishes to know when pages are first written to */
+	mask = VM_READ|VM_WRITE|VM_EXEC|VM_SHARED;
+	if (vma->vm_ops && vma->vm_ops->page_mkwrite)
+		mask &= ~VM_SHARED;
+
+	newprot = protection_map[newflags & mask];
+
 	/*
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.

.....

page_mkwrite?? No, never heard of it round here, you must be mistaken ;)

But seriously, Andrew dropped it from 2.6.13-rc5-mm1, for expedient reasons:
- Dropped cachefs and the cachefs-for-AFS patches. These get in the way of memory management testing a bit, and they're being redone anyway.

So Andrew's 2.6.13-rc4-mm1 directory should contain its last public state (by which time I'd fixed up those various things I'd found to be broken).

But David may have redone a lot since then, I don't know: he's the one to ask. (And I'm afraid I've done my best to make the old patch not apply to current -mm.)

Hugh

Hi,

(-:

Right, thanks. I was wondering whether they had been fixed.

That can be fixed, if David has not done it already... (-:

This is what I am working on in ntfs as my top priority at present, so I really want to get it fixed and merged and I am willing to put in the time necessary to make it happen as I really hate having to instantiate holes on read access for files with logical blocks of size above PAGE_{CACHE_,}SIZE, it just feels wrong...

Best regards,

Anton

Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/