[PATCH] vm/agp: remove private page protection map

[PATCH] vm/agp: remove private page protection map

[Dave Airlie]

AGP keeps its own copy of the protection_map, upcoming DRM changes
will also require access to this map from modules.

Signed-off-by: Dave Airlie <airlied@linux.ie>
---
  drivers/char/agp/frontend.c |    8 +-------
  include/linux/mm.h          |    1 +
  mm/mmap.c                   |    6 ++++++
  3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/char/agp/frontend.c b/drivers/char/agp/frontend.c
index d9c5a91..1842427 100644
--- a/drivers/char/agp/frontend.c
+++ b/drivers/char/agp/frontend.c
@@ -157,12 +157,6 @@ static void agp_add_seg_to_client(struct
   * some routine which does the conversion for you
   */

-static const pgprot_t my_protect_map[16] =
-{
-	__P000, __P001, __P010, __P011, __P100, __P101, __P110, __P111,
-	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
-};
-
  static pgprot_t agp_convert_mmap_flags(int prot)
  {
  #define _trans(x,bit1,bit2) \
@@ -177,7 +171,7 @@ ((bit1==bit2)?(x&bit1):(x&bit1)?bit2:0)

  	prot_bits |= VM_SHARED;

-	temp = my_protect_map[prot_bits & 0x0000000f];
+	temp = vm_get_page_prot(prot_bits & 0x0000000f);

  	return temp;
  }
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 990957e..52d6193 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1012,6 +1012,7 @@ static inline unsigned long vma_pages(st
  	return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
  }

+pgprot_t vm_get_page_prot(u8 vm_flags);
  struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
  struct page *vmalloc_to_page(void *addr);
  unsigned long vmalloc_to_pfn(void *addr);
diff --git a/mm/mmap.c b/mm/mmap.c
index c1868ec..d4c9b2a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -60,6 +60,12 @@ pgprot_t protection_map[16] = {
  	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
  };

+pgprot_t vm_get_page_prot(u8 vm_flags)
+{
+	return protection_map[vm_flags];
+}
+EXPORT_SYMBOL(vm_get_page_prot);
+
  int sysctl_overcommit_memory = OVERCOMMIT_GUESS;  /* heuristic overcommit */
  int sysctl_overcommit_ratio = 50;	/* default is 50% */
  int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
-- 
1.4.1.ga3e6

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

Re: [PATCH] vm/agp: remove private page protection map

[Dave Jones]

On Tue, Jul 18, 2006 at 07:08:12PM +0100, Dave Airlie wrote:
 > 
 > AGP keeps its own copy of the protection_map, upcoming DRM changes
 > will also require access to this map from modules.

Nice. I've always disliked having this knowledge in the agp driver.
I'll queue this up.

		Dave

-- 
http://www.codemonkey.org.uk

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

Re: [PATCH] vm/agp: remove private page protection map

[Hugh Dickins]

On Tue, 18 Jul 2006, Dave Airlie wrote:
> AGP keeps its own copy of the protection_map, upcoming DRM changes
> will also require access to this map from modules.

I'm happy with the intent of your vm_get_page_prot() patch (and would
like to extend it to other places after, minimizing references to the
protection_map[]).  But there's a few aspects which distress me - the
u8 type nowhere else in mm, the requirement that caller mask the arg,
agp_convert_mmap_flags still using its own conversion from PROT_ to VM_
while there's an inline in mm.h (though why someone thought to optimize
and so obscure that version puzzles me!).  Would you be happy to insert
your Sign-off in the replacement below?


AGP keeps its own copy of the protection_map, upcoming DRM changes will
also require access to this map from modules.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 drivers/char/agp/frontend.c |   27 ++-------------------------
 include/linux/mm.h          |    1 +
 mm/mmap.c                   |    7 +++++++
 3 files changed, 10 insertions(+), 25 deletions(-)

--- 2.6.18-rc2-git6/drivers/char/agp/frontend.c	2006-07-16 00:17:07.000000000 +0100
+++ linux/drivers/char/agp/frontend.c	2006-07-26 20:32:10.000000000 +0100
@@ -151,35 +151,12 @@ static void agp_add_seg_to_client(struct
 	client->segments = seg;
 }
 
-/* Originally taken from linux/mm/mmap.c from the array
- * protection_map.
- * The original really should be exported to modules, or
- * some routine which does the conversion for you
- */
-
-static const pgprot_t my_protect_map[16] =
-{
-	__P000, __P001, __P010, __P011, __P100, __P101, __P110, __P111,
-	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
-};
-
 static pgprot_t agp_convert_mmap_flags(int prot)
 {
-#define _trans(x,bit1,bit2) \
-((bit1==bit2)?(x&bit1):(x&bit1)?bit2:0)
-
 	unsigned long prot_bits;
-	pgprot_t temp;
-
-	prot_bits = _trans(prot, PROT_READ, VM_READ) |
-	    _trans(prot, PROT_WRITE, VM_WRITE) |
-	    _trans(prot, PROT_EXEC, VM_EXEC);
-
-	prot_bits |= VM_SHARED;
 
-	temp = my_protect_map[prot_bits & 0x0000000f];
-
-	return temp;
+	prot_bits = calc_vm_prot_bits(prot) | VM_SHARED;
+	return vm_get_page_prot(prot_bits);
 }
 
 static int agp_create_segment(struct agp_client *client, struct agp_region *region)
--- 2.6.18-rc2-git6/include/linux/mm.h	2006-07-16 00:17:31.000000000 +0100
+++ linux/include/linux/mm.h	2006-07-26 20:33:59.000000000 +0100
@@ -1012,6 +1012,7 @@ static inline unsigned long vma_pages(st
 	return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
 }
 
+pgprot_t vm_get_page_prot(unsigned long vm_flags);
 struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
 struct page *vmalloc_to_page(void *addr);
 unsigned long vmalloc_to_pfn(void *addr);
--- 2.6.18-rc2-git6/mm/mmap.c	2006-07-16 00:17:39.000000000 +0100
+++ linux/mm/mmap.c	2006-07-26 20:40:12.000000000 +0100
@@ -60,6 +60,13 @@ pgprot_t protection_map[16] = {
 	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
 };
 
+pgprot_t vm_get_page_prot(unsigned long vm_flags)
+{
+	return protection_map[vm_flags &
+				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
+}
+EXPORT_SYMBOL(vm_get_page_prot);
+
 int sysctl_overcommit_memory = OVERCOMMIT_GUESS;  /* heuristic overcommit */
 int sysctl_overcommit_ratio = 50;	/* default is 50% */
 int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;

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

Re: [PATCH] vm/agp: remove private page protection map

[Dave Airlie]

>
> I'm happy with the intent of your vm_get_page_prot() patch (and would
> like to extend it to other places after, minimizing references to the
> protection_map[]).  But there's a few aspects which distress me - the
> u8 type nowhere else in mm, the requirement that caller mask the arg,
> agp_convert_mmap_flags still using its own conversion from PROT_ to VM_
> while there's an inline in mm.h (though why someone thought to optimize
> and so obscure that version puzzles me!).  Would you be happy to insert
> your Sign-off in the replacement below?

No worries, I think davej can drop my one from his tree as well and take 
this..

Thanks for looking at this..

Dave.

>
>
> AGP keeps its own copy of the protection_map, upcoming DRM changes will
> also require access to this map from modules.
>
> Signed-off-by: Hugh Dickins <hugh@veritas.com>

Signed-of-by: Dave Airlie <airlied@linux.ie>

> ---
>
> drivers/char/agp/frontend.c |   27 ++-------------------------
> include/linux/mm.h          |    1 +
> mm/mmap.c                   |    7 +++++++
> 3 files changed, 10 insertions(+), 25 deletions(-)
>
> --- 2.6.18-rc2-git6/drivers/char/agp/frontend.c	2006-07-16 00:17:07.000000000 +0100
> +++ linux/drivers/char/agp/frontend.c	2006-07-26 20:32:10.000000000 +0100
> @@ -151,35 +151,12 @@ static void agp_add_seg_to_client(struct
> 	client->segments = seg;
> }
>
> -/* Originally taken from linux/mm/mmap.c from the array
> - * protection_map.
> - * The original really should be exported to modules, or
> - * some routine which does the conversion for you
> - */
> -
> -static const pgprot_t my_protect_map[16] =
> -{
> -	__P000, __P001, __P010, __P011, __P100, __P101, __P110, __P111,
> -	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
> -};
> -
> static pgprot_t agp_convert_mmap_flags(int prot)
> {
> -#define _trans(x,bit1,bit2) \
> -((bit1==bit2)?(x&bit1):(x&bit1)?bit2:0)
> -
> 	unsigned long prot_bits;
> -	pgprot_t temp;
> -
> -	prot_bits = _trans(prot, PROT_READ, VM_READ) |
> -	    _trans(prot, PROT_WRITE, VM_WRITE) |
> -	    _trans(prot, PROT_EXEC, VM_EXEC);
> -
> -	prot_bits |= VM_SHARED;
>
> -	temp = my_protect_map[prot_bits & 0x0000000f];
> -
> -	return temp;
> +	prot_bits = calc_vm_prot_bits(prot) | VM_SHARED;
> +	return vm_get_page_prot(prot_bits);
> }
>
> static int agp_create_segment(struct agp_client *client, struct agp_region *region)
> --- 2.6.18-rc2-git6/include/linux/mm.h	2006-07-16 00:17:31.000000000 +0100
> +++ linux/include/linux/mm.h	2006-07-26 20:33:59.000000000 +0100
> @@ -1012,6 +1012,7 @@ static inline unsigned long vma_pages(st
> 	return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> }
>
> +pgprot_t vm_get_page_prot(unsigned long vm_flags);
> struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
> struct page *vmalloc_to_page(void *addr);
> unsigned long vmalloc_to_pfn(void *addr);
> --- 2.6.18-rc2-git6/mm/mmap.c	2006-07-16 00:17:39.000000000 +0100
> +++ linux/mm/mmap.c	2006-07-26 20:40:12.000000000 +0100
> @@ -60,6 +60,13 @@ pgprot_t protection_map[16] = {
> 	__S000, __S001, __S010, __S011, __S100, __S101, __S110, __S111
> };
>
> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +{
> +	return protection_map[vm_flags &
> +				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)];
> +}
> +EXPORT_SYMBOL(vm_get_page_prot);
> +
> int sysctl_overcommit_memory = OVERCOMMIT_GUESS;  /* heuristic overcommit */
> int sysctl_overcommit_ratio = 50;	/* default is 50% */
> int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
>

-- 
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG

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

Re: [PATCH] vm/agp: remove private page protection map

[Dave Jones]

On Thu, Jul 27, 2006 at 12:24:27AM +0100, Dave Airlie wrote:
 > >
 > >I'm happy with the intent of your vm_get_page_prot() patch (and would
 > >like to extend it to other places after, minimizing references to the
 > >protection_map[]).  But there's a few aspects which distress me - the
 > >u8 type nowhere else in mm, the requirement that caller mask the arg,
 > >agp_convert_mmap_flags still using its own conversion from PROT_ to VM_
 > >while there's an inline in mm.h (though why someone thought to optimize
 > >and so obscure that version puzzles me!).  Would you be happy to insert
 > >your Sign-off in the replacement below?
 > 
 > No worries, I think davej can drop my one from his tree as well and take 
 > this..

Done, and pushed out to agpgart.git

Thanks,

		Dave

-- 
http://www.codemonkey.org.uk

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

Re: [PATCH] vm/agp: remove private page protection map

[Hugh Dickins]

On Thu, 27 Jul 2006, Dave Airlie wrote:
> > agp_convert_mmap_flags still using its own conversion from PROT_ to VM_
> > while there's an inline in mm.h (though why someone thought to optimize

My mistake: calc_vm_prot_bits() is actually in include/linux/mman.h
(which you are already #including, so no problem).

> > AGP keeps its own copy of the protection_map, upcoming DRM changes will
> > also require access to this map from modules.
> >
> > Signed-off-by: Hugh Dickins <hugh@veritas.com>
> 
> Signed-of-by: Dave Airlie <airlied@linux.ie>

Thanks.  By the way, I hope you noticed that some architectures
(arm, m68k, sparc, sparc64) may adjust protection_map[] at startup:
so the old agp_convert_mmap_flags would supply the compiled in prot,
whereas the new agp_convert_mmap_flags supplies the adjusted prot.

I assume this is either irrelevant to you (no AGP on some arches?)
or an improvement (the adjusted prot more appropriate); but if you
weren't aware of it, please do check that those do what you want.

Hugh

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

Re: [PATCH] vm/agp: remove private page protection map

[Dave Jones]

On Thu, Jul 27, 2006 at 12:59:44AM +0100, Hugh Dickins wrote:

 > Thanks.  By the way, I hope you noticed that some architectures
 > (arm, m68k, sparc, sparc64) may adjust protection_map[] at startup:
 > so the old agp_convert_mmap_flags would supply the compiled in prot,
 > whereas the new agp_convert_mmap_flags supplies the adjusted prot.
 > 
 > I assume this is either irrelevant to you (no AGP on some arches?)

AGP doesn't exist on any of the above architectures.

		Dave

-- 
http://www.codemonkey.org.uk

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