handle_mm_fault() calling convention cleanup..

handle_mm_fault() calling convention cleanup..

[Linus Torvalds]

Just a heads up that I committed the patches that I sent out two months 
ago to make the fault handling routines use the finer-grained fault flags 
(FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".

That was originally for the NOPAGE_RETRY patches, but it's a general 
cleanup too. I have this suspicion that we should extend this to 
"get_user_pages()" too, instead of having those boolean "write" and 
"force" flags (and GUP_FLAGS_xyzzy as opposed to FAULT_FLAGS_yyzzy).

We should probably also get rid of the insane FOLL_xyz flags too. Right 
now the code in fact depends on FOLL_WRITE being the same as 
FAULT_FLAGS_WRITE, and while that is a simple dependency, it's just crazy 
how we have all these different flags for what ends up often boiling down 
to the same fundamental issue in the end (even if not all versions of the 
flags are necessarily always valid for all uses).

I fixed up all architectures that I noticed (at least microblaze had been 
added since the original patches in April), but arch maintainers should 
double-check. Arch maintainers might also want to check whether the 
mindless conversion of

	'is_write' => 'is_write ? FAULT_FLAGS_WRITE : 0'

might perhaps be written in some more natural way (for example, maybe 
you'd like to get rid of 'iswrite' as a variable entirely, and replace it 
with a 'fault_flags' variable).

It's pushed out and tested on x86-64, but it really was such a mindless 
conversion that I hope it works on all architectures. But I thought I'd 
better give people a shout-out regardless.

		Linus

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

handle_mm_fault() calling convention cleanup..

[Linus Torvalds]

Just a heads up that I committed the patches that I sent out two months 
ago to make the fault handling routines use the finer-grained fault flags 
(FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".

That was originally for the NOPAGE_RETRY patches, but it's a general 
cleanup too. I have this suspicion that we should extend this to 
"get_user_pages()" too, instead of having those boolean "write" and 
"force" flags (and GUP_FLAGS_xyzzy as opposed to FAULT_FLAGS_yyzzy).

We should probably also get rid of the insane FOLL_xyz flags too. Right 
now the code in fact depends on FOLL_WRITE being the same as 
FAULT_FLAGS_WRITE, and while that is a simple dependency, it's just crazy 
how we have all these different flags for what ends up often boiling down 
to the same fundamental issue in the end (even if not all versions of the 
flags are necessarily always valid for all uses).

I fixed up all architectures that I noticed (at least microblaze had been 
added since the original patches in April), but arch maintainers should 
double-check. Arch maintainers might also want to check whether the 
mindless conversion of

	'is_write' => 'is_write ? FAULT_FLAGS_WRITE : 0'

might perhaps be written in some more natural way (for example, maybe 
you'd like to get rid of 'iswrite' as a variable entirely, and replace it 
with a 'fault_flags' variable).

It's pushed out and tested on x86-64, but it really was such a mindless 
conversion that I hope it works on all architectures. But I thought I'd 
better give people a shout-out regardless.

		Linus

Re: handle_mm_fault() calling convention cleanup..

[David Miller]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 21 Jun 2009 13:42:35 -0700 (PDT)

> I fixed up all architectures that I noticed (at least microblaze had been 
> added since the original patches in April), but arch maintainers should 
> double-check. Arch maintainers might also want to check whether the 
> mindless conversion of
> 
> 	'is_write' => 'is_write ? FAULT_FLAGS_WRITE : 0'
> 
> might perhaps be written in some more natural way (for example, maybe 
> you'd like to get rid of 'iswrite' as a variable entirely, and replace it 
> with a 'fault_flags' variable).
> 
> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Sparc looks good, and sparc64 seems to work fine.

--
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: handle_mm_fault() calling convention cleanup..

[David Miller]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 21 Jun 2009 13:42:35 -0700 (PDT)

> I fixed up all architectures that I noticed (at least microblaze had been 
> added since the original patches in April), but arch maintainers should 
> double-check. Arch maintainers might also want to check whether the 
> mindless conversion of
> 
> 	'is_write' => 'is_write ? FAULT_FLAGS_WRITE : 0'
> 
> might perhaps be written in some more natural way (for example, maybe 
> you'd like to get rid of 'iswrite' as a variable entirely, and replace it 
> with a 'fault_flags' variable).
> 
> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Sparc looks good, and sparc64 seems to work fine.

Re: handle_mm_fault() calling convention cleanup..

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Just a heads up that I committed the patches that I sent out two 
> months ago to make the fault handling routines use the 
> finer-grained fault flags (FAULT_FLAG_xyzzy) rather than passing 
> in a boolean for "write".

All is fine on x86.

	Ingo

--
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: handle_mm_fault() calling convention cleanup..

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Just a heads up that I committed the patches that I sent out two 
> months ago to make the fault handling routines use the 
> finer-grained fault flags (FAULT_FLAG_xyzzy) rather than passing 
> in a boolean for "write".

All is fine on x86.

	Ingo

Re: handle_mm_fault() calling convention cleanup..

[Martin Schwidefsky]

On Sun, 21 Jun 2009 13:42:35 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Just a heads up that I committed the patches that I sent out two months 
> ago to make the fault handling routines use the finer-grained fault flags 
> (FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".

Todays git tree still works on s390.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: handle_mm_fault() calling convention cleanup..

[Martin Schwidefsky]

On Sun, 21 Jun 2009 13:42:35 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> Just a heads up that I committed the patches that I sent out two months 
> ago to make the fault handling routines use the finer-grained fault flags 
> (FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".

Todays git tree still works on s390.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
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: handle_mm_fault() calling convention cleanup..

[David Howells]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Works on FRV and MN10300.

David

--
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: handle_mm_fault() calling convention cleanup..

[David Howells]

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Works on FRV and MN10300.

David

Re: handle_mm_fault() calling convention cleanup..

[James Bottomley]

On Sun, 2009-06-21 at 13:42 -0700, Linus Torvalds wrote:
> I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Works on parisc.

James


--
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: handle_mm_fault() calling convention cleanup..

[James Bottomley]

On Sun, 2009-06-21 at 13:42 -0700, Linus Torvalds wrote:
> I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Works on parisc.

James

Re: handle_mm_fault() calling convention cleanup..

[Russell King]

On Sun, Jun 21, 2009 at 01:42:35PM -0700, Linus Torvalds wrote:
> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Works fine on ARM, thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: handle_mm_fault() calling convention cleanup..

[Russell King]

On Sun, Jun 21, 2009 at 01:42:35PM -0700, Linus Torvalds wrote:
> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.

Works fine on ARM, thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

--
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: handle_mm_fault() calling convention cleanup..

[Nick Piggin]

On Sun, Jun 21, 2009 at 01:42:35PM -0700, Linus Torvalds wrote:
> 
> Just a heads up that I committed the patches that I sent out two months 
> ago to make the fault handling routines use the finer-grained fault flags 
> (FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".

While you've got everyone's attention, may I just remind arch
maintainers to consider using pagefault_out_of_memory() rather
than unconditional kill current in the pagefault OOM case. See
x86.

Thanks,
Nick

--
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: handle_mm_fault() calling convention cleanup..

[Nick Piggin]

On Sun, Jun 21, 2009 at 01:42:35PM -0700, Linus Torvalds wrote:
> 
> Just a heads up that I committed the patches that I sent out two months 
> ago to make the fault handling routines use the finer-grained fault flags 
> (FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".

While you've got everyone's attention, may I just remind arch
maintainers to consider using pagefault_out_of_memory() rather
than unconditional kill current in the pagefault OOM case. See
x86.

Thanks,
Nick

[PATCH] hugetlb: fault flags instead of write_access

[Hugh Dickins]

handle_mm_fault() is now passing fault flags rather than write_access
down to hugetlb_fault(), so better recognize that in hugetlb_fault(),
and in hugetlb_no_page().

Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---

 include/linux/hugetlb.h |    4 ++--
 mm/hugetlb.c            |   17 +++++++++--------
 2 files changed, 11 insertions(+), 10 deletions(-)

--- 2.6.30-git20/include/linux/hugetlb.h	2009-06-23 11:06:22.000000000 +0100
+++ linux/include/linux/hugetlb.h	2009-06-23 13:07:57.000000000 +0100
@@ -33,7 +33,7 @@ void hugetlb_report_meminfo(struct seq_f
 int hugetlb_report_node_meminfo(int, char *);
 unsigned long hugetlb_total_pages(void);
 int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
-			unsigned long address, int write_access);
+			unsigned long address, unsigned int flags);
 int hugetlb_reserve_pages(struct inode *inode, long from, long to,
 						struct vm_area_struct *vma,
 						int acctflags);
@@ -98,7 +98,7 @@ static inline void hugetlb_report_meminf
 #define pud_huge(x)	0
 #define is_hugepage_only_range(mm, addr, len)	0
 #define hugetlb_free_pgd_range(tlb, addr, end, floor, ceiling) ({BUG(); 0; })
-#define hugetlb_fault(mm, vma, addr, write)	({ BUG(); 0; })
+#define hugetlb_fault(mm, vma, addr, flags)	({ BUG(); 0; })
 
 #define hugetlb_change_protection(vma, address, end, newprot)
 
--- 2.6.30-git20/mm/hugetlb.c	2009-06-23 11:06:25.000000000 +0100
+++ linux/mm/hugetlb.c	2009-06-23 13:07:57.000000000 +0100
@@ -1985,7 +1985,7 @@ static struct page *hugetlbfs_pagecache_
 }
 
 static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
-			unsigned long address, pte_t *ptep, int write_access)
+			unsigned long address, pte_t *ptep, unsigned int flags)
 {
 	struct hstate *h = hstate_vma(vma);
 	int ret = VM_FAULT_SIGBUS;
@@ -2053,7 +2053,7 @@ retry:
 	 * any allocations necessary to record that reservation occur outside
 	 * the spinlock.
 	 */
-	if (write_access && !(vma->vm_flags & VM_SHARED))
+	if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED))
 		if (vma_needs_reservation(h, vma, address) < 0) {
 			ret = VM_FAULT_OOM;
 			goto backout_unlocked;
@@ -2072,7 +2072,7 @@ retry:
 				&& (vma->vm_flags & VM_SHARED)));
 	set_huge_pte_at(mm, address, ptep, new_pte);
 
-	if (write_access && !(vma->vm_flags & VM_SHARED)) {
+	if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
 		/* Optimization, do the COW without a second fault */
 		ret = hugetlb_cow(mm, vma, address, ptep, new_pte, page);
 	}
@@ -2091,7 +2091,7 @@ backout_unlocked:
 }
 
 int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
-			unsigned long address, int write_access)
+			unsigned long address, unsigned int flags)
 {
 	pte_t *ptep;
 	pte_t entry;
@@ -2112,7 +2112,7 @@ int hugetlb_fault(struct mm_struct *mm,
 	mutex_lock(&hugetlb_instantiation_mutex);
 	entry = huge_ptep_get(ptep);
 	if (huge_pte_none(entry)) {
-		ret = hugetlb_no_page(mm, vma, address, ptep, write_access);
+		ret = hugetlb_no_page(mm, vma, address, ptep, flags);
 		goto out_mutex;
 	}
 
@@ -2126,7 +2126,7 @@ int hugetlb_fault(struct mm_struct *mm,
 	 * page now as it is used to determine if a reservation has been
 	 * consumed.
 	 */
-	if (write_access && !pte_write(entry)) {
+	if ((flags & FAULT_FLAG_WRITE) && !pte_write(entry)) {
 		if (vma_needs_reservation(h, vma, address) < 0) {
 			ret = VM_FAULT_OOM;
 			goto out_mutex;
@@ -2143,7 +2143,7 @@ int hugetlb_fault(struct mm_struct *mm,
 		goto out_page_table_lock;
 
 
-	if (write_access) {
+	if (flags & FAULT_FLAG_WRITE) {
 		if (!pte_write(entry)) {
 			ret = hugetlb_cow(mm, vma, address, ptep, entry,
 							pagecache_page);
@@ -2152,7 +2152,8 @@ int hugetlb_fault(struct mm_struct *mm,
 		entry = pte_mkdirty(entry);
 	}
 	entry = pte_mkyoung(entry);
-	if (huge_ptep_set_access_flags(vma, address, ptep, entry, write_access))
+	if (huge_ptep_set_access_flags(vma, address, ptep, entry,
+						flags & FAULT_FLAG_WRITE))
 		update_mmu_cache(vma, address, entry);
 
 out_page_table_lock:

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

[PATCH] mm: don't rely on flags coincidence

[Hugh Dickins]

Indeed FOLL_WRITE matches FAULT_FLAG_WRITE, matches GUP_FLAGS_WRITE,
and it's tempting to devise a set of Grand Unified Paging flags;
but not today.  So until then, let's rely upon the compiler to spot
the coincidence, "rather than have that subtle dependency and a
comment for it" - as you remarked in another context yesterday.

Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---

 mm/memory.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- 2.6.30-git20/mm/memory.c	2009-06-23 11:06:25.000000000 +0100
+++ linux/mm/memory.c	2009-06-23 13:07:57.000000000 +0100
@@ -1311,8 +1311,10 @@ int __get_user_pages(struct task_struct
 			while (!(page = follow_page(vma, start, foll_flags))) {
 				int ret;
 
-				/* FOLL_WRITE matches FAULT_FLAG_WRITE! */
-				ret = handle_mm_fault(mm, vma, start, foll_flags & FOLL_WRITE);
+				ret = handle_mm_fault(mm, vma, start,
+					(foll_flags & FOLL_WRITE) ?
+					FAULT_FLAG_WRITE : 0);
+
 				if (ret & VM_FAULT_ERROR) {
 					if (ret & VM_FAULT_OOM)
 						return i ? i : -ENOMEM;

--
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] hugetlb: fault flags instead of write_access

[Wu Fengguang]

On Tue, Jun 23, 2009 at 08:49:05PM +0800, Hugh Dickins wrote:
> handle_mm_fault() is now passing fault flags rather than write_access
> down to hugetlb_fault(), so better recognize that in hugetlb_fault(),
> and in hugetlb_no_page().
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Looks OK and compiles OK.

Thanks,
Fengguang

--
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] mm: don't rely on flags coincidence

[Wu Fengguang]

On Tue, Jun 23, 2009 at 08:52:49PM +0800, Hugh Dickins wrote:
> Indeed FOLL_WRITE matches FAULT_FLAG_WRITE, matches GUP_FLAGS_WRITE,
> and it's tempting to devise a set of Grand Unified Paging flags;
> but not today.  So until then, let's rely upon the compiler to spot
> the coincidence, "rather than have that subtle dependency and a
> comment for it" - as you remarked in another context yesterday.
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Acked-by: Wu Fengguang <fengguang.wu@intel.com>

> ---
> 
>  mm/memory.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- 2.6.30-git20/mm/memory.c	2009-06-23 11:06:25.000000000 +0100
> +++ linux/mm/memory.c	2009-06-23 13:07:57.000000000 +0100
> @@ -1311,8 +1311,10 @@ int __get_user_pages(struct task_struct
>  			while (!(page = follow_page(vma, start, foll_flags))) {
>  				int ret;
>  
> -				/* FOLL_WRITE matches FAULT_FLAG_WRITE! */
> -				ret = handle_mm_fault(mm, vma, start, foll_flags & FOLL_WRITE);
> +				ret = handle_mm_fault(mm, vma, start,
> +					(foll_flags & FOLL_WRITE) ?
> +					FAULT_FLAG_WRITE : 0);
> +
>  				if (ret & VM_FAULT_ERROR) {
>  					if (ret & VM_FAULT_OOM)
>  						return i ? i : -ENOMEM;

--
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] hugetlb: fault flags instead of write_access

[Rik van Riel]

Hugh Dickins wrote:
> handle_mm_fault() is now passing fault flags rather than write_access
> down to hugetlb_fault(), so better recognize that in hugetlb_fault(),
> and in hugetlb_no_page().
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed.

--
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] mm: don't rely on flags coincidence

[Rik van Riel]

Hugh Dickins wrote:
> Indeed FOLL_WRITE matches FAULT_FLAG_WRITE, matches GUP_FLAGS_WRITE,
> and it's tempting to devise a set of Grand Unified Paging flags;
> but not today.  So until then, let's rely upon the compiler to spot
> the coincidence, "rather than have that subtle dependency and a
> comment for it" - as you remarked in another context yesterday.
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Good catch.

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed.

--
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] hugetlb: fault flags instead of write_access

[Mel Gorman]

On Tue, Jun 23, 2009 at 01:49:05PM +0100, Hugh Dickins wrote:
> handle_mm_fault() is now passing fault flags rather than write_access
> down to hugetlb_fault(), so better recognize that in hugetlb_fault(),
> and in hugetlb_no_page().
> 
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Patch looks good and passes libhugetlbfs regression tests. It passes
with or without the tests but without this patch, it's only a
co-incidence it passes as opposed to designed.

Reviewed-by: Mel Gorman <mel@csn.ul.ie>

> ---
> 
>  include/linux/hugetlb.h |    4 ++--
>  mm/hugetlb.c            |   17 +++++++++--------
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> --- 2.6.30-git20/include/linux/hugetlb.h	2009-06-23 11:06:22.000000000 +0100
> +++ linux/include/linux/hugetlb.h	2009-06-23 13:07:57.000000000 +0100
> @@ -33,7 +33,7 @@ void hugetlb_report_meminfo(struct seq_f
>  int hugetlb_report_node_meminfo(int, char *);
>  unsigned long hugetlb_total_pages(void);
>  int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> -			unsigned long address, int write_access);
> +			unsigned long address, unsigned int flags);
>  int hugetlb_reserve_pages(struct inode *inode, long from, long to,
>  						struct vm_area_struct *vma,
>  						int acctflags);
> @@ -98,7 +98,7 @@ static inline void hugetlb_report_meminf
>  #define pud_huge(x)	0
>  #define is_hugepage_only_range(mm, addr, len)	0
>  #define hugetlb_free_pgd_range(tlb, addr, end, floor, ceiling) ({BUG(); 0; })
> -#define hugetlb_fault(mm, vma, addr, write)	({ BUG(); 0; })
> +#define hugetlb_fault(mm, vma, addr, flags)	({ BUG(); 0; })
>  
>  #define hugetlb_change_protection(vma, address, end, newprot)
>  
> --- 2.6.30-git20/mm/hugetlb.c	2009-06-23 11:06:25.000000000 +0100
> +++ linux/mm/hugetlb.c	2009-06-23 13:07:57.000000000 +0100
> @@ -1985,7 +1985,7 @@ static struct page *hugetlbfs_pagecache_
>  }
>  
>  static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
> -			unsigned long address, pte_t *ptep, int write_access)
> +			unsigned long address, pte_t *ptep, unsigned int flags)
>  {
>  	struct hstate *h = hstate_vma(vma);
>  	int ret = VM_FAULT_SIGBUS;
> @@ -2053,7 +2053,7 @@ retry:
>  	 * any allocations necessary to record that reservation occur outside
>  	 * the spinlock.
>  	 */
> -	if (write_access && !(vma->vm_flags & VM_SHARED))
> +	if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED))
>  		if (vma_needs_reservation(h, vma, address) < 0) {
>  			ret = VM_FAULT_OOM;
>  			goto backout_unlocked;
> @@ -2072,7 +2072,7 @@ retry:
>  				&& (vma->vm_flags & VM_SHARED)));
>  	set_huge_pte_at(mm, address, ptep, new_pte);
>  
> -	if (write_access && !(vma->vm_flags & VM_SHARED)) {
> +	if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
>  		/* Optimization, do the COW without a second fault */
>  		ret = hugetlb_cow(mm, vma, address, ptep, new_pte, page);
>  	}
> @@ -2091,7 +2091,7 @@ backout_unlocked:
>  }
>  
>  int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> -			unsigned long address, int write_access)
> +			unsigned long address, unsigned int flags)
>  {
>  	pte_t *ptep;
>  	pte_t entry;
> @@ -2112,7 +2112,7 @@ int hugetlb_fault(struct mm_struct *mm,
>  	mutex_lock(&hugetlb_instantiation_mutex);
>  	entry = huge_ptep_get(ptep);
>  	if (huge_pte_none(entry)) {
> -		ret = hugetlb_no_page(mm, vma, address, ptep, write_access);
> +		ret = hugetlb_no_page(mm, vma, address, ptep, flags);
>  		goto out_mutex;
>  	}
>  
> @@ -2126,7 +2126,7 @@ int hugetlb_fault(struct mm_struct *mm,
>  	 * page now as it is used to determine if a reservation has been
>  	 * consumed.
>  	 */
> -	if (write_access && !pte_write(entry)) {
> +	if ((flags & FAULT_FLAG_WRITE) && !pte_write(entry)) {
>  		if (vma_needs_reservation(h, vma, address) < 0) {
>  			ret = VM_FAULT_OOM;
>  			goto out_mutex;
> @@ -2143,7 +2143,7 @@ int hugetlb_fault(struct mm_struct *mm,
>  		goto out_page_table_lock;
>  
>  
> -	if (write_access) {
> +	if (flags & FAULT_FLAG_WRITE) {
>  		if (!pte_write(entry)) {
>  			ret = hugetlb_cow(mm, vma, address, ptep, entry,
>  							pagecache_page);
> @@ -2152,7 +2152,8 @@ int hugetlb_fault(struct mm_struct *mm,
>  		entry = pte_mkdirty(entry);
>  	}
>  	entry = pte_mkyoung(entry);
> -	if (huge_ptep_set_access_flags(vma, address, ptep, entry, write_access))
> +	if (huge_ptep_set_access_flags(vma, address, ptep, entry,
> +						flags & FAULT_FLAG_WRITE))
>  		update_mmu_cache(vma, address, entry);
>  
>  out_page_table_lock:
> 
> --
> 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>
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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: handle_mm_fault() calling convention cleanup..

[Benjamin Herrenschmidt]

On Sun, 2009-06-21 at 13:42 -0700, Linus Torvalds wrote:
> Just a heads up that I committed the patches that I sent out two months 
> ago to make the fault handling routines use the finer-grained fault flags 
> (FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".
> 
> That was originally for the NOPAGE_RETRY patches, but it's a general 
> cleanup too. I have this suspicion that we should extend this to 
> "get_user_pages()" too, instead of having those boolean "write" and 
> "force" flags (and GUP_FLAGS_xyzzy as opposed to FAULT_FLAGS_yyzzy).

BTW. I'd like to extend these if there's no objection one of these days
to also pass whether it was an exec fault, and pass the full flags to
ptep_set_access_flags().

That would (finally) give us a better hook for architectures that need
to do it to handle i$/d$ coherency. Right now, I go dig for the current
fault type inside the current pt_regs from ptep_set_access_flags() which
is positively ugly.

Ben.

> We should probably also get rid of the insane FOLL_xyz flags too. Right 
> now the code in fact depends on FOLL_WRITE being the same as 
> FAULT_FLAGS_WRITE, and while that is a simple dependency, it's just crazy 
> how we have all these different flags for what ends up often boiling down 
> to the same fundamental issue in the end (even if not all versions of the 
> flags are necessarily always valid for all uses).
> 
> I fixed up all architectures that I noticed (at least microblaze had been 
> added since the original patches in April), but arch maintainers should 
> double-check. Arch maintainers might also want to check whether the 
> mindless conversion of
> 
> 	'is_write' => 'is_write ? FAULT_FLAGS_WRITE : 0'
> 
> might perhaps be written in some more natural way (for example, maybe 
> you'd like to get rid of 'iswrite' as a variable entirely, and replace it 
> with a 'fault_flags' variable).
> 
> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.
> 
> 		Linus
> 
> --
> 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>

--
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: handle_mm_fault() calling convention cleanup..

[Benjamin Herrenschmidt]

On Sun, 2009-06-21 at 13:42 -0700, Linus Torvalds wrote:
> Just a heads up that I committed the patches that I sent out two months 
> ago to make the fault handling routines use the finer-grained fault flags 
> (FAULT_FLAG_xyzzy) rather than passing in a boolean for "write".
> 
> That was originally for the NOPAGE_RETRY patches, but it's a general 
> cleanup too. I have this suspicion that we should extend this to 
> "get_user_pages()" too, instead of having those boolean "write" and 
> "force" flags (and GUP_FLAGS_xyzzy as opposed to FAULT_FLAGS_yyzzy).

BTW. I'd like to extend these if there's no objection one of these days
to also pass whether it was an exec fault, and pass the full flags to
ptep_set_access_flags().

That would (finally) give us a better hook for architectures that need
to do it to handle i$/d$ coherency. Right now, I go dig for the current
fault type inside the current pt_regs from ptep_set_access_flags() which
is positively ugly.

Ben.

> We should probably also get rid of the insane FOLL_xyz flags too. Right 
> now the code in fact depends on FOLL_WRITE being the same as 
> FAULT_FLAGS_WRITE, and while that is a simple dependency, it's just crazy 
> how we have all these different flags for what ends up often boiling down 
> to the same fundamental issue in the end (even if not all versions of the 
> flags are necessarily always valid for all uses).
> 
> I fixed up all architectures that I noticed (at least microblaze had been 
> added since the original patches in April), but arch maintainers should 
> double-check. Arch maintainers might also want to check whether the 
> mindless conversion of
> 
> 	'is_write' => 'is_write ? FAULT_FLAGS_WRITE : 0'
> 
> might perhaps be written in some more natural way (for example, maybe 
> you'd like to get rid of 'iswrite' as a variable entirely, and replace it 
> with a 'fault_flags' variable).
> 
> It's pushed out and tested on x86-64, but it really was such a mindless 
> conversion that I hope it works on all architectures. But I thought I'd 
> better give people a shout-out regardless.
> 
> 		Linus
> 
> --
> 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: handle_mm_fault() calling convention cleanup..

[Linus Torvalds]

On Sat, 4 Jul 2009, Benjamin Herrenschmidt wrote:
> 
> BTW. I'd like to extend these if there's no objection one of these days
> to also pass whether it was an exec fault, and pass the full flags to
> ptep_set_access_flags().

Sure. No problem, and sounds sane.

Just a tiny word of warning: right now, the conversion I did pretty much 
depended on the fact that even if I missed a spot, it wouldn't actually 
make any difference. If somebody used "flags" as a binary value (ie like 
the old "write_access" kind of semantics), things would still all work, 
because it was still a "zero-vs-nonzero" issue wrt writes.

And there were cases in the hugepage handling that I had missed, that 
Hugh picked up. Maybe he picked them all - but be careful.

I didn't add any flags (like the FAULT_FLAG_RETRY thing that started it 
all) that would actually _require_ everybody to always treat it as a 
bitmask. And some places still pass the flags down as basically just the 
"write or not" thing. ptep_set_access_flags() stands out as one of them 
(and I think your suggestion would actually clean things up), but there 
are probably others.

		Linus

--
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: handle_mm_fault() calling convention cleanup..

[Linus Torvalds]

On Sat, 4 Jul 2009, Benjamin Herrenschmidt wrote:
> 
> BTW. I'd like to extend these if there's no objection one of these days
> to also pass whether it was an exec fault, and pass the full flags to
> ptep_set_access_flags().

Sure. No problem, and sounds sane.

Just a tiny word of warning: right now, the conversion I did pretty much 
depended on the fact that even if I missed a spot, it wouldn't actually 
make any difference. If somebody used "flags" as a binary value (ie like 
the old "write_access" kind of semantics), things would still all work, 
because it was still a "zero-vs-nonzero" issue wrt writes.

And there were cases in the hugepage handling that I had missed, that 
Hugh picked up. Maybe he picked them all - but be careful.

I didn't add any flags (like the FAULT_FLAG_RETRY thing that started it 
all) that would actually _require_ everybody to always treat it as a 
bitmask. And some places still pass the flags down as basically just the 
"write or not" thing. ptep_set_access_flags() stands out as one of them 
(and I think your suggestion would actually clean things up), but there 
are probably others.

		Linus

Re: handle_mm_fault() calling convention cleanup..

[Benjamin Herrenschmidt]

On Sat, 2009-07-04 at 09:44 -0700, Linus Torvalds wrote:

> Just a tiny word of warning: right now, the conversion I did pretty much 
> depended on the fact that even if I missed a spot, it wouldn't actually 
> make any difference. If somebody used "flags" as a binary value (ie like 
> the old "write_access" kind of semantics), things would still all work, 
> because it was still a "zero-vs-nonzero" issue wrt writes.

 .../...

Right. Oh well.. we'll see when I get to it. I have a few higher
priority things on my pile at the moment.

Cheers,
Ben.

--
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: handle_mm_fault() calling convention cleanup..

[Benjamin Herrenschmidt]

On Sat, 2009-07-04 at 09:44 -0700, Linus Torvalds wrote:

> Just a tiny word of warning: right now, the conversion I did pretty much 
> depended on the fact that even if I missed a spot, it wouldn't actually 
> make any difference. If somebody used "flags" as a binary value (ie like 
> the old "write_access" kind of semantics), things would still all work, 
> because it was still a "zero-vs-nonzero" issue wrt writes.

 .../...

Right. Oh well.. we'll see when I get to it. I have a few higher
priority things on my pile at the moment.

Cheers,
Ben.

Re: handle_mm_fault() calling convention cleanup..

[Nick Piggin]

On Sun, Jul 05, 2009 at 07:08:38AM +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2009-07-04 at 09:44 -0700, Linus Torvalds wrote:
> 
> > Just a tiny word of warning: right now, the conversion I did pretty much 
> > depended on the fact that even if I missed a spot, it wouldn't actually 
> > make any difference. If somebody used "flags" as a binary value (ie like 
> > the old "write_access" kind of semantics), things would still all work, 
> > because it was still a "zero-vs-nonzero" issue wrt writes.
> 
>  .../...
> 
> Right. Oh well.. we'll see when I get to it. I have a few higher
> priority things on my pile at the moment.

I have no problems with that. I'd always intended to have flags
go further up the call chain like Linus did (since we'd discussed
perhaps making faults interruptible and requiring an extra flag
to distinguish get_user_pages callers that were not interruptible).

So yes adding more flags to improve code or make things simpler
is fine by me :)

Thanks,
Nick

Re: handle_mm_fault() calling convention cleanup..

[Nick Piggin]

On Sun, Jul 05, 2009 at 07:08:38AM +1000, Benjamin Herrenschmidt wrote:
> On Sat, 2009-07-04 at 09:44 -0700, Linus Torvalds wrote:
> 
> > Just a tiny word of warning: right now, the conversion I did pretty much 
> > depended on the fact that even if I missed a spot, it wouldn't actually 
> > make any difference. If somebody used "flags" as a binary value (ie like 
> > the old "write_access" kind of semantics), things would still all work, 
> > because it was still a "zero-vs-nonzero" issue wrt writes.
> 
>  .../...
> 
> Right. Oh well.. we'll see when I get to it. I have a few higher
> priority things on my pile at the moment.

I have no problems with that. I'd always intended to have flags
go further up the call chain like Linus did (since we'd discussed
perhaps making faults interruptible and requiring an extra flag
to distinguish get_user_pages callers that were not interruptible).

So yes adding more flags to improve code or make things simpler
is fine by me :)

Thanks,
Nick

--
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: handle_mm_fault() calling convention cleanup..

[Benjamin Herrenschmidt]

On Mon, 2009-07-06 at 09:31 +0200, Nick Piggin wrote:
> I have no problems with that. I'd always intended to have flags
> go further up the call chain like Linus did (since we'd discussed
> perhaps making faults interruptible and requiring an extra flag
> to distinguish get_user_pages callers that were not interruptible).
> 
> So yes adding more flags to improve code or make things simpler
> is fine by me :)
> 
That's before you see my evil plan of bringing the flags all the way
down to set_pte_at() :-)

Cheers,
Ben.


--
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: handle_mm_fault() calling convention cleanup..

[Benjamin Herrenschmidt]

On Mon, 2009-07-06 at 09:31 +0200, Nick Piggin wrote:
> I have no problems with that. I'd always intended to have flags
> go further up the call chain like Linus did (since we'd discussed
> perhaps making faults interruptible and requiring an extra flag
> to distinguish get_user_pages callers that were not interruptible).
> 
> So yes adding more flags to improve code or make things simpler
> is fine by me :)
> 
That's before you see my evil plan of bringing the flags all the way
down to set_pte_at() :-)

Cheers,
Ben.

Re: handle_mm_fault() calling convention cleanup..

[Nick Piggin]

On Mon, Jul 06, 2009 at 08:56:16PM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2009-07-06 at 09:31 +0200, Nick Piggin wrote:
> > I have no problems with that. I'd always intended to have flags
> > go further up the call chain like Linus did (since we'd discussed
> > perhaps making faults interruptible and requiring an extra flag
> > to distinguish get_user_pages callers that were not interruptible).
> > 
> > So yes adding more flags to improve code or make things simpler
> > is fine by me :)
> > 
> That's before you see my evil plan of bringing the flags all the way
> down to set_pte_at() :-)

So long as it can be nooped out of x86 I don't see it being
a problem.

One problem x86 has with the mm/memory.c code is that it
runs out of registers (especially in fork/exit iirc). So
I wouldn't like to add unnecessary arguments to functions
if they cannot be optimised away.


--
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: handle_mm_fault() calling convention cleanup..

[Nick Piggin]

On Mon, Jul 06, 2009 at 08:56:16PM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2009-07-06 at 09:31 +0200, Nick Piggin wrote:
> > I have no problems with that. I'd always intended to have flags
> > go further up the call chain like Linus did (since we'd discussed
> > perhaps making faults interruptible and requiring an extra flag
> > to distinguish get_user_pages callers that were not interruptible).
> > 
> > So yes adding more flags to improve code or make things simpler
> > is fine by me :)
> > 
> That's before you see my evil plan of bringing the flags all the way
> down to set_pte_at() :-)

So long as it can be nooped out of x86 I don't see it being
a problem.

One problem x86 has with the mm/memory.c code is that it
runs out of registers (especially in fork/exit iirc). So
I wouldn't like to add unnecessary arguments to functions
if they cannot be optimised away.