2008-08-16 09:34:55

by Ulrich Drepper

[permalink] [raw]
Subject: [PATCH] VMA comment fixes

These seem to be two tiny problems in comments related to VMA data
structures. Am I correct?


Signed-off-by: Ulrich Drepper <[email protected]>

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 386edbe..5ceb830 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -113,7 +113,7 @@ struct vm_area_struct {
struct vm_area_struct *vm_next;

pgprot_t vm_page_prot; /* Access permissions of this VMA. */
- unsigned long vm_flags; /* Flags, listed below. */
+ unsigned long vm_flags; /* Flags, see vmalloc.h. */

struct rb_node vm_rb;

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 364789a..39fc8d7 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -6,7 +6,7 @@

struct vm_area_struct;

-/* bits in vm_struct->flags */
+/* bits in vm_area_struct->flags */
#define VM_IOREMAP 0x00000001 /* ioremap() and friends */
#define VM_ALLOC 0x00000002 /* vmalloc() */
#define VM_MAP 0x00000004 /* vmap()ed pages */


2008-08-16 10:08:16

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] VMA comment fixes

On Sat, 16 Aug 2008, Ulrich Drepper wrote:

> These seem to be two tiny problems in comments related to VMA data
> structures. Am I correct?

Not on this occasion - fixed patch at the bottom.
I suspect you of having a wicked sense of humour.

(Once upon a time some types got separated out from mm.h to mm_types.h:
probably mm.h's VM_flags should have travelled with vm_area_struct,
even though they're not types. And on another occasion, a forward
declaration of vm_area_struct was put in vmalloc.h, home of vm_struct.)

>
> Signed-off-by: Ulrich Drepper <[email protected]>
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 386edbe..5ceb830 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -113,7 +113,7 @@ struct vm_area_struct {
> struct vm_area_struct *vm_next;
>
> pgprot_t vm_page_prot; /* Access permissions of this VMA. */
> - unsigned long vm_flags; /* Flags, listed below. */
> + unsigned long vm_flags; /* Flags, see vmalloc.h. */
>
> struct rb_node vm_rb;
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 364789a..39fc8d7 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -6,7 +6,7 @@
>
> struct vm_area_struct;
>
> -/* bits in vm_struct->flags */
> +/* bits in vm_area_struct->flags */
> #define VM_IOREMAP 0x00000001 /* ioremap() and friends */
> #define VM_ALLOC 0x00000002 /* vmalloc() */
> #define VM_MAP 0x00000004 /* vmap()ed pages */

[PATCH] mm: VM_flags comment fixes

Try to comment away a little of the confusion between mm's vm_area_struct
vm_flags and vmalloc's vm_struct flags: based on an idea by Ulrich Drepper.

Signed-off-by: Hugh Dickins <[email protected]>
---

include/linux/mm.h | 2 +-
include/linux/mm_types.h | 2 +-
include/linux/vmalloc.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)

--- 2.6.27-rc3/include/linux/mm.h 2008-08-13 04:14:50.000000000 +0100
+++ linux/include/linux/mm.h 2008-08-16 10:51:58.000000000 +0100
@@ -73,7 +73,7 @@ extern unsigned int kobjsize(const void
#endif

/*
- * vm_flags..
+ * vm_flags in vm_area_struct, see mm_types.h.
*/
#define VM_READ 0x00000001 /* currently active flags */
#define VM_WRITE 0x00000002
--- 2.6.27-rc3/include/linux/mm_types.h 2008-07-29 04:24:55.000000000 +0100
+++ linux/include/linux/mm_types.h 2008-08-16 10:44:34.000000000 +0100
@@ -113,7 +113,7 @@ struct vm_area_struct {
struct vm_area_struct *vm_next;

pgprot_t vm_page_prot; /* Access permissions of this VMA. */
- unsigned long vm_flags; /* Flags, listed below. */
+ unsigned long vm_flags; /* Flags, see mm.h. */

struct rb_node vm_rb;

--- 2.6.27-rc3/include/linux/vmalloc.h 2008-07-13 22:51:29.000000000 +0100
+++ linux/include/linux/vmalloc.h 2008-08-16 10:51:58.000000000 +0100
@@ -4,9 +4,9 @@
#include <linux/spinlock.h>
#include <asm/page.h> /* pgprot_t */

-struct vm_area_struct;
+struct vm_area_struct; /* vma defining user mapping in mm_types.h */

-/* bits in vm_struct->flags */
+/* bits in flags of vmalloc's vm_struct below */
#define VM_IOREMAP 0x00000001 /* ioremap() and friends */
#define VM_ALLOC 0x00000002 /* vmalloc() */
#define VM_MAP 0x00000004 /* vmap()ed pages */

2008-08-16 17:45:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] VMA comment fixes



On Sat, 16 Aug 2008, Ulrich Drepper wrote:
>
> These seem to be two tiny problems in comments related to VMA data
> structures. Am I correct?

No.

> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> - unsigned long vm_flags; /* Flags, listed below. */
> + unsigned long vm_flags; /* Flags, see vmalloc.h. */

It's "see mm.h" for the primary one. The vmalloc.h ones are only used for
vmalloc'ed areas, no for regular _user_ mappings.

That said, I agree that it's damn confusing that we have two different
sets of flags, both called VM_xyz. We use the "vm_area_struct" for the
normal user virtual mappings (mmap) tracking, and we use the "vm_struct"
for the kernel virtual mappings (vmalloc).

Both have a flags field, and both use VM_xyz for their field names.

> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> -/* bits in vm_struct->flags */
> +/* bits in vm_area_struct->flags */

See above. The vmalloc ones really are in "vm_struct->flags".

The confusion comes at least partly because

- I think historically we _used_ to use vm_area_struct for both, if I
recall correctly

and because

- we actually have some mixing of the two (for example
"remap_vmalloc_range()" will remap a vmalloc mapping _into_ a user
mapping, so it takes a _user_ mapping (a "vm_area_struct") _and_ a
vmalloc address (internally using a "struct vm_struct") and copies from
one to the other.

it might be a good idea to call the vmalloc flags somethign else than
VM_xyz. But I'm not sure it's worth the churn. I don't remember there
having ever been any _actual_ confusion by the VM people who have to deal
with both (because you really very seldom can confuse them).

Linus