remap_file_pages: kernel-doc corrections

PATCH Superseded Randy Dunlap 2 participants 4 messages Oct 8, 2007 – Oct 9, 2007 Memory management

lore

Share & tools

lore

Randy Dunlap

author

2007-10-09 00:03 UTC #0 lore raw

PATCH 1/1 remap_file_pages: kernel-doc corrections

Fix kernel-doc for sys_remap_file_pages() and add info to the __prot NOTE.

Signed-off-by Randy Dunlap <[email protected]>

---
 mm/fremap.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

mm/fremap.c +11 -9

@@ -97,23 +97,25 @@ static int populate_range(struct mm_stru
 
 }
 
-/***
- * sys_remap_file_pages - remap arbitrary pages of a shared backing store
- *                        file within an existing vma.
+/**
+ * sys_remap_file_pages - remap arbitrary pages of a shared backing store file
  * @start: start of the remapped virtual memory range
  * @size: size of the remapped virtual memory range
- * @prot: new protection bits of the range
- * @pgoff: to be mapped page of the backing store file
+ * @__prot: new protection bits of the range (see NOTE)
+ * @pgoff: to-be-mapped page of the backing store file
  * @flags: 0 or MAP_NONBLOCKED - the later will cause no IO.
  *
- * this syscall works purely via pagetables, so it's the most efficient
+ * sys_remap_file_pages remaps arbitrary pages of a shared backing store file
+ * within an existing vma.
+ *
+ * This syscall works purely via pagetables, so it's the most efficient
  * way to map the same (large) file into a given virtual window. Unlike
  * mmap()/mremap() it does not create any new vmas. The new mappings are
  * also safe across swapout.
  *
- * NOTE: the 'prot' parameter right now is ignored, and the vma's default
- * protection is used. Arbitrary protections might be implemented in the
- * future.
+ * NOTE: the '__prot' parameter right now is ignored (but must be zero),
+ * and the vma's default protection is used. Arbitrary protections
+ * might be implemented in the future.
  */
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
 	unsigned long __prot, unsigned long pgoff, unsigned long flags)

--
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>

Nick Piggin

2007-10-08 08:44 UTC #1 lore raw

Quoted 3 lines · Randy Dunlap

From: Randy Dunlap <[email protected]>
Fix kernel-doc for sys_remap_file_pages() and add info to the __prot NOTE.

Why not just get rid of the double underscore, I wonder?

Pity that prot always has to be PROT_NONE on most architectures (rather than the current vma's prot)... but that's something which had to have been picked up years ago.

Quoted 9 lines

Signed-off-by Randy Dunlap <[email protected]>
---
 mm/fremap.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

mm/fremap.c static int populate_range(struct mm_stru · line 101

Quoted 8 lines

mm/fremap.c +2 -3

@@ -97,23 +97,25 @@ static int populate_range(struct mm_stru
 
 }
 
-/***
- * sys_remap_file_pages - remap arbitrary pages of a shared backing store
- *                        file within an existing vma.
+/**
+ * sys_remap_file_pages - remap arbitrary pages of a shared backing store

I think the "vma" part is kind of important, and probably not emphasised enough in the original, and "shared backing store file" is a bit vague. It's actually a VM_SHARED _vma_.

(which is a bit funny eg. it must be a PROT_WRITE && MAP_SHARED vma -- doesn't seem to be documented in the man page, and odd that you aren't allowed to have readonly nonlinear mappings, but it's not mine to wonder why).

Otherwise, looks OK.

Quoted 18 lines

file * @start: start of the remapped virtual memory range
* @size: size of the remapped virtual memory range
- * @prot: new protection bits of the range
- * @pgoff: to be mapped page of the backing store file
+ * @__prot: new protection bits of the range (see NOTE)
+ * @pgoff: to-be-mapped page of the backing store file
* @flags: 0 or MAP_NONBLOCKED - the later will cause no IO. *
- * this syscall works purely via pagetables, so it's the most efficient
+ * sys_remap_file_pages remaps arbitrary pages of a shared backing store file + * within an existing vma.
+ *
+ * This syscall works purely via pagetables, so it's the most efficient
* way to map the same (large) file into a given virtual window. Unlike
* mmap()/mremap() it does not create any new vmas. The new mappings are
* also safe across swapout.
*
- * NOTE: the 'prot' parameter right now is ignored, and the vma's default

Quoted 8 lines

- * protection is used. Arbitrary protections might be implemented in the
- * future.
+ * NOTE: the '__prot' parameter right now is ignored (but must be zero),
+ * and the vma's default protection is used. Arbitrary protections
+ * might be implemented in the future.
*/
asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long
size, unsigned long __prot, unsigned long pgoff, unsigned long flags)

Randy Dunlap

author

2007-10-09 04:29 UTC #2 lore raw

PATCH v2 1/1 remap_file_pages: kernel-doc corrections

Quoted 3 lines · On Mon, 8 Oct 2007 18:44:42 +1000 Nick Piggin wrote:

Quoted 1 line
Fix kernel-doc for sys_remap_file_pages() and add info to the __prot NOTE.
Why not just get rid of the double underscore, I wonder?

ok, did that.

Quoted 9 lines

Review
Quoted 5 lines
(quoted diff) +2 -3
-/***
- * sys_remap_file_pages - remap arbitrary pages of a shared backing store
- *                        file within an existing vma.
+/**
+ * sys_remap_file_pages - remap arbitrary pages of a shared backing store
I think the "vma" part is kind of important, and probably not emphasised enough in the original, and "shared backing store file" is a bit vague. It's actually a VM_SHARED _vma_.

I modified that comment based on your comments.
See if it's better now, please.

---

From: Randy Dunlap <[email protected]>

Fix kernel-doc for sys_remap_file_pages() and add info to the 'prot' NOTE. Rename __prot parameter to prot.

Signed-off-by Randy Dunlap <[email protected]>

---
 mm/fremap.c |   24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

mm/fremap.c +13 ~~-11~~

@@ -97,26 +97,28 @@ static int populate_range(struct mm_stru
 
 }
 
-/***
- * sys_remap_file_pages - remap arbitrary pages of a shared backing store
- *                        file within an existing vma.
+/**
+ * sys_remap_file_pages - remap arbitrary pages of an existing VM_SHARED vma
  * @start: start of the remapped virtual memory range
  * @size: size of the remapped virtual memory range
- * @prot: new protection bits of the range
- * @pgoff: to be mapped page of the backing store file
+ * @prot: new protection bits of the range (see NOTE)
+ * @pgoff: to-be-mapped page of the backing store file
  * @flags: 0 or MAP_NONBLOCKED - the later will cause no IO.
  *
- * this syscall works purely via pagetables, so it's the most efficient
+ * sys_remap_file_pages remaps arbitrary pages of an existing VM_SHARED vma
+ * (shared backing store file).
+ *
+ * This syscall works purely via pagetables, so it's the most efficient
  * way to map the same (large) file into a given virtual window. Unlike
  * mmap()/mremap() it does not create any new vmas. The new mappings are
  * also safe across swapout.
  *
- * NOTE: the 'prot' parameter right now is ignored, and the vma's default
- * protection is used. Arbitrary protections might be implemented in the
- * future.
+ * NOTE: the 'prot' parameter right now is ignored (but must be zero),
+ * and the vma's default protection is used. Arbitrary protections
+ * might be implemented in the future.
  */
 asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
-	unsigned long __prot, unsigned long pgoff, unsigned long flags)
+	unsigned long prot, unsigned long pgoff, unsigned long flags)
 {
 	struct mm_struct *mm = current->mm;
 	struct address_space *mapping;

@@ -125,7 +127,7 @@ asmlinkage long sys_remap_file_pages(uns
 	int err = -EINVAL;
 	int has_write_lock = 0;
 
-	if (__prot)
+	if (prot)
 		return err;
 	/*
 	 * Sanitize the syscall parameters:

Nick Piggin

2007-10-08 12:39 UTC #3 lore raw

Quoted 2 lines · Randy Dunlap

I modified that comment based on your comments.
See if it's better now, please.

Thanks, looks good.