Skip to contentSkip to conversation

Add page becoming writable notification

PATCH RESEND In review David Howells 3 participants 5 messages Oct 14, 2004 – Oct 15, 2004

Subsystems (4)
Memory management VFS MM core Page cache
lore
Share & tools
lore

David Howells

author
#0 lore raw

PATCH RESEND 4/6 Add page becoming writable notification

Because Christoph asked so nicely, I'm resending this patch with a wider distribution. The patch currently resides in the -mm kernels. It is being used by the in-kernel AFS filesystem and by the NFS caching patches that haven't yet been passed to akpm.

---

The attached patch creates a facility by which a filesystem, character device or block device can detect a page mapped to an inode is about to become writable. This provides the facility at two levels: a vma operation and an address space operation. This can be used by a netfs to synchronise with a cache writing the page to disc.

Signed-off-by David Howells <[email protected]>

---

 include/linux/fs.h |    3 +
 include/linux/mm.h |    4 ++
 mm/filemap.c       |   19 +++++++++-
 mm/memory.c        |   96 ++++++++++++++++++++++++++++++++++++++++-------------
 4 files changed, 99 insertions(+), 23 deletions(-)
include/linux/fs.h +3 -0
diff -uNrp linux-2.6.9-rc1-mm2/include/linux/fs.h linux-2.6.9-rc1-mm2-cachefs/include/linux/fs.h
@@ -328,6 +328,9 @@ struct address_space_operations {
 	int (*releasepage) (struct page *, int);
 	ssize_t (*direct_IO)(int, struct kiocb *, const struct iovec *iov,
 			loff_t offset, unsigned long nr_segs);
+
+	/* notification that a page is about to become writable */
+	int (*page_mkwrite)(struct page *page);
 };
 
 struct backing_dev_info;
include/linux/mm.h +4 -0
diff -uNrp linux-2.6.9-rc1-mm2/include/linux/mm.h linux-2.6.9-rc1-mm2-cachefs/include/linux/mm.h
@@ -175,6 +175,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,
mm/filemap.c +18 -1
diff -uNrp linux-2.6.9-rc1-mm2/mm/filemap.c linux-2.6.9-rc1-mm2-cachefs/mm/filemap.c
@@ -1445,11 +1445,25 @@ repeat:
 	return 0;
 }
 
+/*
+ * pass notification that a page is becoming writable up to the filesystem
+ */
+static int filemap_page_mkwrite(struct vm_area_struct *vma, struct page *page)
+{
+	return page->mapping->a_ops->page_mkwrite(page);
+}
+
 struct vm_operations_struct generic_file_vm_ops = {
 	.nopage		= filemap_nopage,
 	.populate	= filemap_populate,
 };
 
+struct vm_operations_struct generic_file_vm_mkwr_ops = {
+	.nopage		= filemap_nopage,
+	.populate	= filemap_populate,
+	.page_mkwrite	= filemap_page_mkwrite,
+};
+
 /* This is used for a general mmap of a disk file */
 
 int generic_file_mmap(struct file * file, struct vm_area_struct * vma)

@@ -1459,7 +1473,10 @@ int generic_file_mmap(struct file * file
 	if (!mapping->a_ops->readpage)
 		return -ENOEXEC;
 	file_accessed(file);
-	vma->vm_ops = &generic_file_vm_ops;
+	if (!mapping->a_ops->page_mkwrite)
+		vma->vm_ops = &generic_file_vm_ops;
+	else
+		vma->vm_ops = &generic_file_vm_mkwr_ops;
 	return 0;
 }
mm/memory.c +74 -22

121 lines not on this page · raw message

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

#1 lore raw

mm/memory.c static inline void break_cow(struct vm_a · line 1040

Quoted 6 lines · David Howells
mm/memory.c +6 -0
+static inline int do_wp_page_mk_pte_writable(struct mm_struct *mm,
+					     struct vm_area_struct *vma,
+					     unsigned long address,
+					     pte_t *page_table,
+					     struct page *old_page,
+					     pte_t pte)

I protest. There are at least 3 vowels and 2 non-acronyms in this function name. Also, 6 arguments is clearly too few. Can we not also pass a struct urb, an ethtool_wolinfo and a Scsi_Cmnd?

Signature · 11 lines
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw
In reply to David Howells

include/linux/fs.h struct address_space_operations { · line 333

Quoted 3 lines
include/linux/fs.h +3 -0
+
+	/* notification that a page is about to become writable */
+	int (*page_mkwrite)(struct page *page);

This doesn't fit into address_space operations at all. The vm_operation below is enough.

Quoted 4 lines
mm/memory.c +0 -0
@@ -1030,6 +1030,54 @@ static inline void break_cow(struct vm_a
 }

mm/memory.c static inline void break_cow(struct vm_a · line 1040

Quoted 10 lines
mm/memory.c +8 -0
 
 /*
+ * Make a PTE writeable for do_wp_page() on a shared-writable page
+ */
+static inline int do_wp_page_mk_pte_writable(struct mm_struct *mm,
+					     struct vm_area_struct *vma,
+					     unsigned long address,
+					     pte_t *page_table,
+					     struct page *old_page,
+					     pte_t pte)

This prototype shows pretty much that splitting it out doesn't make much sense. Why not add a goto reuse_page; where you call it currently and append it at the end of do_wp_page?

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

author
#3 lore raw
Quoted 5 lines

include/linux/fs.h struct address_space_operations { · line 333

Quoted 2 lines
include/linux/fs.h +2 -0
+	/* notification that a page is about to become writable */
+	int (*page_mkwrite)(struct page *page);

This doesn't fit into address_space operations at all. The vm_operation below is enough.

Filesystems shouldn't be overloading vm_operations on ordinary files, or so I've been instructed.

Quoted 10 lines

mm/memory.c static inline void break_cow(struct vm_a · line 1040

Quoted 6 lines
mm/memory.c +6 -0
+static inline int do_wp_page_mk_pte_writable(struct mm_struct *mm,
+					     struct vm_area_struct *vma,
+					     unsigned long address,
+					     pte_t *page_table,
+					     struct page *old_page,
+					     pte_t pte)

This prototype shows pretty much that splitting it out doesn't make much sense. Why not add a goto reuse_page; where you call it currently and append it at the end of do_wp_page?

Judging by the CodingStyle doc - which you like throwing at me - it should be split into a separate inline function. I could come up with a better name, I suppose to keep Willy happy too - perhaps make_pte_writable(); it's just that I wanted to name it to show its derivation.

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

#4 lore raw
Quoted 9 lines · David Howells
Quoted 6 lines

include/linux/fs.h struct address_space_operations { · line 333

Quoted 3 lines
include/linux/fs.h +2 -0
+	/* notification that a page is about to become writable */
+	int (*page_mkwrite)(struct page *page);

This doesn't fit into address_space operations at all. The vm_operation below is enough.

Filesystems shouldn't be overloading vm_operations on ordinary files, or so I've been instructed.

huh? that doesn't make any sense. if a filesystem needs to do something special win regards to the VM it should overload vm_operations. Currently that's only ncpfs and xfs.

Quoted 8 lines
Quoted 3 lines

This prototype shows pretty much that splitting it out doesn't make much sense. Why not add a goto reuse_page; where you call it currently and append it at the end of do_wp_page?

Judging by the CodingStyle doc - which you like throwing at me - it should be split into a separate inline function. I could come up with a better name, I suppose to keep Willy happy too - perhaps make_pte_writable(); it's just that I wanted to name it to show its derivation.

Splitting in helpers makes sense if there's a sane interface. The number of arguments doesn't exactly imply that it's the case here.

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