[v2,40/42] example for future extension: mm:gup/writeback: add callbacks for inaccessible pages: source indication

Commit Message

Christian Borntraeger Feb. 14, 2020, 10:26 p.m. UTC

From: Claudio Imbrenda <[e-mail]>

We might want to do different things depending on where we are coming
from.

Cc: Will Deacon <[e-mail]>
Cc: Sean Christopherson <[e-mail]>
Signed-off-by: Claudio Imbrenda <[e-mail]>
[[e-mail]: patch description]
Signed-off-by: Christian Borntraeger <[e-mail]>
---
 include/linux/gfp.h | 8 +++++++-
 mm/gup.c            | 4 ++--
 mm/page-writeback.c | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

Comments

Ulrich Weigand Feb. 17, 2020, 2:15 p.m. UTC | #1

On Fri, Feb 14, 2020 at 05:26:56PM -0500, Christian Borntraeger wrote:
> +enum access_type {
> +	MAKE_ACCESSIBLE_GENERIC,
> +	MAKE_ACCESSIBLE_GET,
> +	MAKE_ACCESSIBLE_GET_FAST,
> +	MAKE_ACCESSIBLE_WRITEBACK
> +};
>  #ifndef HAVE_ARCH_MAKE_PAGE_ACCESSIBLE
> -static inline int arch_make_page_accessible(struct page *page)
> +static inline int arch_make_page_accessible(struct page *page, int where)

If we want to make this distinction, wouldn't it be simpler to just
use different function names, like
  arch_make_page_accessible_for_writeback
  arch_make_page_accessible_for_gup
etc.

Bye,
Ulrich

Christian Borntraeger Feb. 17, 2020, 2:38 p.m. UTC | #2

On 17.02.20 15:15, Ulrich Weigand wrote:
> On Fri, Feb 14, 2020 at 05:26:56PM -0500, Christian Borntraeger wrote:
>> +enum access_type {
>> +	MAKE_ACCESSIBLE_GENERIC,
>> +	MAKE_ACCESSIBLE_GET,
>> +	MAKE_ACCESSIBLE_GET_FAST,
>> +	MAKE_ACCESSIBLE_WRITEBACK
>> +};
>>  #ifndef HAVE_ARCH_MAKE_PAGE_ACCESSIBLE
>> -static inline int arch_make_page_accessible(struct page *page)
>> +static inline int arch_make_page_accessible(struct page *page, int where)
> 
> If we want to make this distinction, wouldn't it be simpler to just
> use different function names, like
>   arch_make_page_accessible_for_writeback
>   arch_make_page_accessible_for_gup
> etc.

Agreed.
I would suggest to do these changes when somebody needs them, though.
On the other hand, Patch 39 (the error handling) is something that we
could merge now.

Patch

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index be2754841369..a15fcb361e7c 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -485,8 +485,14 @@  static inline void arch_free_page(struct page *page, int order) { }
 #ifndef HAVE_ARCH_ALLOC_PAGE
 static inline void arch_alloc_page(struct page *page, int order) { }
 #endif
+enum access_type {
+	MAKE_ACCESSIBLE_GENERIC,
+	MAKE_ACCESSIBLE_GET,
+	MAKE_ACCESSIBLE_GET_FAST,
+	MAKE_ACCESSIBLE_WRITEBACK
+};
 #ifndef HAVE_ARCH_MAKE_PAGE_ACCESSIBLE
-static inline int arch_make_page_accessible(struct page *page)
+static inline int arch_make_page_accessible(struct page *page, int where)
 {
 	return 0;
 }
diff --git a/mm/gup.c b/mm/gup.c
index 354bcfbd844b..ce962c155724 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -274,7 +274,7 @@  static struct page *follow_page_pte(struct vm_area_struct *vma,
 			page = ERR_PTR(-ENOMEM);
 			goto out;
 		}
-		ret = arch_make_page_accessible(page);
+		ret = arch_make_page_accessible(page, MAKE_ACCESSIBLE_GET);
 		if (ret) {
 			put_page(page);
 			page = ERR_PTR(ret);
@@ -1923,7 +1923,7 @@  static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
 
 		VM_BUG_ON_PAGE(compound_head(page) != head, page);
 
-		ret = arch_make_page_accessible(page);
+		ret = arch_make_page_accessible(page, MAKE_ACCESSIBLE_GET_FAST);
 		if (ret) {
 			put_page(head);
 			goto pte_unmap;
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 558d7063c117..f85148e59800 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2811,7 +2811,7 @@  int __test_set_page_writeback(struct page *page, bool keep_write)
 	 * If writeback has been triggered on a page that cannot be made
 	 * accessible, it is too late.
 	 */
-	WARN_ON(arch_make_page_accessible(page));
+	WARN_ON(arch_make_page_accessible(page, MAKE_ACCESSIBLE_WRITEBACK));
 	return ret;
 
 }