linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic
@ 2021-03-17  6:47 Nicholas Piggin
  2021-03-17  6:47 ` [PATCH v2 2/3] mm: add page_cpupid_reset_last atomic variant Nicholas Piggin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-03-17  6:47 UTC (permalink / raw)
  To: linux-mm; +Cc: Nicholas Piggin, Peter Zijlstra

Add a __ prefix to indicate that it should be used in non-atomic
situations (where the page is not subject to concurrent flags
access, following the pattern of the other page flags modifications).

This prepares for an atomic version.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Rebase only. Any comments on this idea?

Thanks,
Nick

 include/linux/mm.h | 6 +++---
 mm/page_alloc.c    | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 64a71bf20536..086ab710383f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1396,7 +1396,7 @@ static inline int page_cpupid_last(struct page *page)
 {
 	return page->_last_cpupid;
 }
-static inline void page_cpupid_reset_last(struct page *page)
+static inline void __page_cpupid_reset_last(struct page *page)
 {
 	page->_last_cpupid = -1 & LAST_CPUPID_MASK;
 }
@@ -1408,7 +1408,7 @@ static inline int page_cpupid_last(struct page *page)
 
 extern int page_cpupid_xchg_last(struct page *page, int cpupid);
 
-static inline void page_cpupid_reset_last(struct page *page)
+static inline void __page_cpupid_reset_last(struct page *page)
 {
 	page->flags |= LAST_CPUPID_MASK << LAST_CPUPID_PGSHIFT;
 }
@@ -1449,7 +1449,7 @@ static inline bool cpupid_pid_unset(int cpupid)
 	return true;
 }
 
-static inline void page_cpupid_reset_last(struct page *page)
+static inline void __page_cpupid_reset_last(struct page *page)
 {
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cfc72873961d..172368357e73 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1266,7 +1266,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
 	if (bad)
 		return false;
 
-	page_cpupid_reset_last(page);
+	__page_cpupid_reset_last(page);
 	page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
 	reset_page_owner(page, order);
 
@@ -1463,7 +1463,7 @@ static void __meminit __init_single_page(struct page *page, unsigned long pfn,
 	set_page_links(page, zone, nid, pfn);
 	init_page_count(page);
 	page_mapcount_reset(page);
-	page_cpupid_reset_last(page);
+	__page_cpupid_reset_last(page);
 	page_kasan_tag_reset(page);
 
 	INIT_LIST_HEAD(&page->lru);
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] mm: add page_cpupid_reset_last atomic variant
  2021-03-17  6:47 [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
@ 2021-03-17  6:47 ` Nicholas Piggin
  2021-03-17  6:47 ` [PATCH v2 3/3] mm: skip page_cpupid_reset_last atomic update if the value is already reset Nicholas Piggin
  2021-06-01  5:06 ` [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
  2 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-03-17  6:47 UTC (permalink / raw)
  To: linux-mm; +Cc: Nicholas Piggin, Peter Zijlstra

This can be used when there is concurrency on the struct page. If cpupid
is not in page flags, it can use a store.

Use this to replace the open-coded reset in wp_page_reuse.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/linux/mm.h | 15 +++++++++++++++
 mm/memory.c        |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 086ab710383f..2524553ea898 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1400,6 +1400,11 @@ static inline void __page_cpupid_reset_last(struct page *page)
 {
 	page->_last_cpupid = -1 & LAST_CPUPID_MASK;
 }
+
+static inline void page_cpupid_reset_last(struct page *page)
+{
+	WRITE_ONCE(page->_last_cpupid, -1 & LAST_CPUPID_MASK);
+}
 #else
 static inline int page_cpupid_last(struct page *page)
 {
@@ -1412,6 +1417,12 @@ static inline void __page_cpupid_reset_last(struct page *page)
 {
 	page->flags |= LAST_CPUPID_MASK << LAST_CPUPID_PGSHIFT;
 }
+
+static inline void page_cpupid_reset_last(struct page *page)
+{
+	page_cpupid_xchg_last(page, (1 << LAST_CPUPID_SHIFT) - 1);
+}
+
 #endif /* LAST_CPUPID_NOT_IN_PAGE_FLAGS */
 #else /* !CONFIG_NUMA_BALANCING */
 static inline int page_cpupid_xchg_last(struct page *page, int cpupid)
@@ -1453,6 +1464,10 @@ static inline void __page_cpupid_reset_last(struct page *page)
 {
 }
 
+static inline void page_cpupid_reset_last(struct page *page)
+{
+}
+
 static inline bool cpupid_match_pid(struct task_struct *task, int cpupid)
 {
 	return false;
diff --git a/mm/memory.c b/mm/memory.c
index 5efa07fb6cdc..bb37dc0f4dac 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2804,7 +2804,7 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
 	 * unrelated process.
 	 */
 	if (page)
-		page_cpupid_xchg_last(page, (1 << LAST_CPUPID_SHIFT) - 1);
+		page_cpupid_reset_last(page);
 
 	flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
 	entry = pte_mkyoung(vmf->orig_pte);
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] mm: skip page_cpupid_reset_last atomic update if the value is already reset
  2021-03-17  6:47 [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
  2021-03-17  6:47 ` [PATCH v2 2/3] mm: add page_cpupid_reset_last atomic variant Nicholas Piggin
@ 2021-03-17  6:47 ` Nicholas Piggin
  2021-06-01  5:06 ` [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
  2 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-03-17  6:47 UTC (permalink / raw)
  To: linux-mm; +Cc: Nicholas Piggin, Peter Zijlstra

Improves performance of a PROT_READ -> PROT_READ|PROT_WRITE -> store
microbenchmark (which goes via the wp_page_reuse fault path) by about
10% on a POWER9.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/linux/mm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2524553ea898..1e1471235055 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1420,6 +1420,9 @@ static inline void __page_cpupid_reset_last(struct page *page)
 
 static inline void page_cpupid_reset_last(struct page *page)
 {
+	if (page_cpupid_last(page) == (-1 & LAST_CPUPID_MASK))
+		return;
+
 	page_cpupid_xchg_last(page, (1 << LAST_CPUPID_SHIFT) - 1);
 }
 
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic
  2021-03-17  6:47 [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
  2021-03-17  6:47 ` [PATCH v2 2/3] mm: add page_cpupid_reset_last atomic variant Nicholas Piggin
  2021-03-17  6:47 ` [PATCH v2 3/3] mm: skip page_cpupid_reset_last atomic update if the value is already reset Nicholas Piggin
@ 2021-06-01  5:06 ` Nicholas Piggin
  2021-06-01  6:42   ` Peter Zijlstra
  2 siblings, 1 reply; 6+ messages in thread
From: Nicholas Piggin @ 2021-06-01  5:06 UTC (permalink / raw)
  To: linux-mm; +Cc: Peter Zijlstra

Ping? Any concerns about these patches?

Thanks,
Nick

Excerpts from Nicholas Piggin's message of March 17, 2021 4:47 pm:
> Add a __ prefix to indicate that it should be used in non-atomic
> situations (where the page is not subject to concurrent flags
> access, following the pattern of the other page flags modifications).
> 
> This prepares for an atomic version.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Rebase only. Any comments on this idea?
> 
> Thanks,
> Nick
> 
>  include/linux/mm.h | 6 +++---
>  mm/page_alloc.c    | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 64a71bf20536..086ab710383f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1396,7 +1396,7 @@ static inline int page_cpupid_last(struct page *page)
>  {
>  	return page->_last_cpupid;
>  }
> -static inline void page_cpupid_reset_last(struct page *page)
> +static inline void __page_cpupid_reset_last(struct page *page)
>  {
>  	page->_last_cpupid = -1 & LAST_CPUPID_MASK;
>  }
> @@ -1408,7 +1408,7 @@ static inline int page_cpupid_last(struct page *page)
>  
>  extern int page_cpupid_xchg_last(struct page *page, int cpupid);
>  
> -static inline void page_cpupid_reset_last(struct page *page)
> +static inline void __page_cpupid_reset_last(struct page *page)
>  {
>  	page->flags |= LAST_CPUPID_MASK << LAST_CPUPID_PGSHIFT;
>  }
> @@ -1449,7 +1449,7 @@ static inline bool cpupid_pid_unset(int cpupid)
>  	return true;
>  }
>  
> -static inline void page_cpupid_reset_last(struct page *page)
> +static inline void __page_cpupid_reset_last(struct page *page)
>  {
>  }
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index cfc72873961d..172368357e73 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1266,7 +1266,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
>  	if (bad)
>  		return false;
>  
> -	page_cpupid_reset_last(page);
> +	__page_cpupid_reset_last(page);
>  	page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
>  	reset_page_owner(page, order);
>  
> @@ -1463,7 +1463,7 @@ static void __meminit __init_single_page(struct page *page, unsigned long pfn,
>  	set_page_links(page, zone, nid, pfn);
>  	init_page_count(page);
>  	page_mapcount_reset(page);
> -	page_cpupid_reset_last(page);
> +	__page_cpupid_reset_last(page);
>  	page_kasan_tag_reset(page);
>  
>  	INIT_LIST_HEAD(&page->lru);
> -- 
> 2.23.0
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic
  2021-06-01  5:06 ` [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
@ 2021-06-01  6:42   ` Peter Zijlstra
  2021-06-08  2:18     ` Nicholas Piggin
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2021-06-01  6:42 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linux-mm

On Tue, Jun 01, 2021 at 03:06:01PM +1000, Nicholas Piggin wrote:
> Ping? Any concerns about these patches?

For all three:

Acked-by: Peter Zijlstra (Intel) <peterz@infradead>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic
  2021-06-01  6:42   ` Peter Zijlstra
@ 2021-06-08  2:18     ` Nicholas Piggin
  0 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-06-08  2:18 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-mm

Excerpts from Peter Zijlstra's message of June 1, 2021 4:42 pm:
> On Tue, Jun 01, 2021 at 03:06:01PM +1000, Nicholas Piggin wrote:
>> Ping? Any concerns about these patches?
> 
> For all three:
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead>
> 

Great, thanks for taking a look.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-08  2:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17  6:47 [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
2021-03-17  6:47 ` [PATCH v2 2/3] mm: add page_cpupid_reset_last atomic variant Nicholas Piggin
2021-03-17  6:47 ` [PATCH v2 3/3] mm: skip page_cpupid_reset_last atomic update if the value is already reset Nicholas Piggin
2021-06-01  5:06 ` [PATCH v2 1/3] mm: add __ prefix to page_cpupid_reset_last because it is non-atomic Nicholas Piggin
2021-06-01  6:42   ` Peter Zijlstra
2021-06-08  2:18     ` Nicholas Piggin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).