1. Patchew
  2. linux
  3. View series

[PATCH] mm/codetag: clear tags before swap

David Wang posted 1 patch 1 year ago
There is a newer version of this series
include/linux/alloc_tag.h | 2 +-
lib/alloc_tag.c           | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
[PATCH] mm/codetag: clear tags before swap
Posted by David Wang 1 year ago 
When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
triggered when calling __alloc_tag_ref_set() during swap:

	alloc_tag was not cleared (got tag for mm/filemap.c:1951)
	WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...

Clear code tags before swap can fix the warning. And this patch also fix
a potential invalid address dereference in alloc_tag_add_check() when
CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
which is defined as ((void *)1).

Signed-off-by: David Wang <00107082@163.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
---
 include/linux/alloc_tag.h | 2 +-
 lib/alloc_tag.c           | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 7c0786bdf9af..cba024bf2db3 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -135,7 +135,7 @@ static inline struct alloc_tag_counters alloc_tag_read(struct alloc_tag *tag)
 #ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
 static inline void alloc_tag_add_check(union codetag_ref *ref, struct alloc_tag *tag)
 {
-	WARN_ONCE(ref && ref->ct,
+	WARN_ONCE(ref && ref->ct && !is_codetag_empty(ref),
 		  "alloc_tag was not cleared (got tag for %s:%u)\n",
 		  ref->ct->filename, ref->ct->lineno);
 
diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
index 35f7560a309a..cc5fda9901c2 100644
--- a/lib/alloc_tag.c
+++ b/lib/alloc_tag.c
@@ -209,6 +209,10 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
 		return;
 	}
 
+	/* clear tags before swap */
+	set_codetag_empty(&ref_old);
+	set_codetag_empty(&ref_new);
+
 	/* swap tags */
 	__alloc_tag_ref_set(&ref_old, tag_new);
 	update_page_tag_ref(handle_old, &ref_old);
-- 
2.39.2
Re: [PATCH] mm/codetag: clear tags before swap
Posted by Suren Baghdasaryan 1 year ago 
On Wed, Dec 11, 2024 at 8:03 PM David Wang <00107082@163.com> wrote:
>
> When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
> triggered when calling __alloc_tag_ref_set() during swap:
>
>         alloc_tag was not cleared (got tag for mm/filemap.c:1951)
>         WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...
>
> Clear code tags before swap can fix the warning. And this patch also fix
> a potential invalid address dereference in alloc_tag_add_check() when
> CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
> which is defined as ((void *)1).
^^^
Good catch!

>
> Signed-off-by: David Wang <00107082@163.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
> ---
>  include/linux/alloc_tag.h | 2 +-
>  lib/alloc_tag.c           | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
> index 7c0786bdf9af..cba024bf2db3 100644
> --- a/include/linux/alloc_tag.h
> +++ b/include/linux/alloc_tag.h
> @@ -135,7 +135,7 @@ static inline struct alloc_tag_counters alloc_tag_read(struct alloc_tag *tag)
>  #ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
>  static inline void alloc_tag_add_check(union codetag_ref *ref, struct alloc_tag *tag)
>  {
> -       WARN_ONCE(ref && ref->ct,
> +       WARN_ONCE(ref && ref->ct && !is_codetag_empty(ref),
>                   "alloc_tag was not cleared (got tag for %s:%u)\n",
>                   ref->ct->filename, ref->ct->lineno);
>
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 35f7560a309a..cc5fda9901c2 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -209,6 +209,10 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
>                 return;
>         }
>
> +       /* clear tags before swap */

The above comment states what we already know from the code but does
not explain why we do this. Better to describe the reason and not what
we do. Something like:

/*
 * Clear tag references to avoid debug warning when using
 *  __alloc_tag_ref_set() with non-empty reference.
 */

> +       set_codetag_empty(&ref_old);
> +       set_codetag_empty(&ref_new);
> +
>         /* swap tags */
>         __alloc_tag_ref_set(&ref_old, tag_new);
>         update_page_tag_ref(handle_old, &ref_old);
> --
> 2.39.2
>
>
Re: [PATCH] mm/codetag: clear tags before swap
Posted by David Wang 1 year ago 
At 2024-12-12 15:09:59, "Suren Baghdasaryan" <surenb@google.com> wrote:
>On Wed, Dec 11, 2024 at 8:03 PM David Wang <00107082@163.com> wrote:
>>
>> When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
>> triggered when calling __alloc_tag_ref_set() during swap:
>>
>>         alloc_tag was not cleared (got tag for mm/filemap.c:1951)
>>         WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...
>>
>> Clear code tags before swap can fix the warning. And this patch also fix
>> a potential invalid address dereference in alloc_tag_add_check() when
>> CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
>> which is defined as ((void *)1).
>^^^
>Good catch!
>
>>
>> Signed-off-by: David Wang <00107082@163.com>
>> Reported-by: kernel test robot <oliver.sang@intel.com>
>> Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
>> ---
>>  include/linux/alloc_tag.h | 2 +-
>>  lib/alloc_tag.c           | 4 ++++
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
>> index 7c0786bdf9af..cba024bf2db3 100644
>> --- a/include/linux/alloc_tag.h
>> +++ b/include/linux/alloc_tag.h
>> @@ -135,7 +135,7 @@ static inline struct alloc_tag_counters alloc_tag_read(struct alloc_tag *tag)
>>  #ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
>>  static inline void alloc_tag_add_check(union codetag_ref *ref, struct alloc_tag *tag)
>>  {
>> -       WARN_ONCE(ref && ref->ct,
>> +       WARN_ONCE(ref && ref->ct && !is_codetag_empty(ref),
>>                   "alloc_tag was not cleared (got tag for %s:%u)\n",
>>                   ref->ct->filename, ref->ct->lineno);
>>
>> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
>> index 35f7560a309a..cc5fda9901c2 100644
>> --- a/lib/alloc_tag.c
>> +++ b/lib/alloc_tag.c
>> @@ -209,6 +209,10 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
>>                 return;
>>         }
>>
>> +       /* clear tags before swap */
>
>The above comment states what we already know from the code but does
>not explain why we do this. Better to describe the reason and not what
>we do. Something like:
>
>/*
> * Clear tag references to avoid debug warning when using
> *  __alloc_tag_ref_set() with non-empty reference.
> */
>

Copy that~!


Thanks!
David
>> +       set_codetag_empty(&ref_old);
>> +       set_codetag_empty(&ref_new);
>> +
>>         /* swap tags */
>>         __alloc_tag_ref_set(&ref_old, tag_new);
>>         update_page_tag_ref(handle_old, &ref_old);
>> --
>> 2.39.2
>>
>>
[PATCH v2] mm/codetag: clear tags before swap
Posted by David Wang 1 year ago 
When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
triggered when calling __alloc_tag_ref_set() during swap:

	alloc_tag was not cleared (got tag for mm/filemap.c:1951)
	WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...

Clear code tags before swap can fix the warning. And this patch also fix
a potential invalid address dereference in alloc_tag_add_check() when
CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
which is defined as ((void *)1).

Signed-off-by: David Wang <00107082@163.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
Suggested-by: Suren Baghdasaryan <surenb@google.com>
---
 include/linux/alloc_tag.h | 2 +-
 lib/alloc_tag.c           | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 7c0786bdf9af..cba024bf2db3 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -135,7 +135,7 @@ static inline struct alloc_tag_counters alloc_tag_read(struct alloc_tag *tag)
 #ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
 static inline void alloc_tag_add_check(union codetag_ref *ref, struct alloc_tag *tag)
 {
-	WARN_ONCE(ref && ref->ct,
+	WARN_ONCE(ref && ref->ct && !is_codetag_empty(ref),
 		  "alloc_tag was not cleared (got tag for %s:%u)\n",
 		  ref->ct->filename, ref->ct->lineno);
 
diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
index 35f7560a309a..3a0413462e9f 100644
--- a/lib/alloc_tag.c
+++ b/lib/alloc_tag.c
@@ -209,6 +209,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
 		return;
 	}
 
+	/*
+	 * Clear tag references to avoid debug warning when using
+	 * __alloc_tag_ref_set() with non-empty reference.
+	 */
+	set_codetag_empty(&ref_old);
+	set_codetag_empty(&ref_new);
+
 	/* swap tags */
 	__alloc_tag_ref_set(&ref_old, tag_new);
 	update_page_tag_ref(handle_old, &ref_old);
-- 
2.39.2
Re: [PATCH v2] mm/codetag: clear tags before swap
Posted by Suren Baghdasaryan 1 year ago 
On Thu, Dec 12, 2024 at 12:29 AM David Wang <00107082@163.com> wrote:
>
> When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
> triggered when calling __alloc_tag_ref_set() during swap:
>
>         alloc_tag was not cleared (got tag for mm/filemap.c:1951)
>         WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...
>
> Clear code tags before swap can fix the warning. And this patch also fix
> a potential invalid address dereference in alloc_tag_add_check() when
> CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
> which is defined as ((void *)1).
>
> Signed-off-by: David Wang <00107082@163.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
> Suggested-by: Suren Baghdasaryan <surenb@google.com>

I didn't really suggest much in this patch, so please replace above
Suggested-by with:

Acked-by: Suren Baghdasaryan <surenb@google.com>

Thanks for fixing this!

> ---
>  include/linux/alloc_tag.h | 2 +-
>  lib/alloc_tag.c           | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
> index 7c0786bdf9af..cba024bf2db3 100644
> --- a/include/linux/alloc_tag.h
> +++ b/include/linux/alloc_tag.h
> @@ -135,7 +135,7 @@ static inline struct alloc_tag_counters alloc_tag_read(struct alloc_tag *tag)
>  #ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
>  static inline void alloc_tag_add_check(union codetag_ref *ref, struct alloc_tag *tag)
>  {
> -       WARN_ONCE(ref && ref->ct,
> +       WARN_ONCE(ref && ref->ct && !is_codetag_empty(ref),
>                   "alloc_tag was not cleared (got tag for %s:%u)\n",
>                   ref->ct->filename, ref->ct->lineno);
>
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 35f7560a309a..3a0413462e9f 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -209,6 +209,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
>                 return;
>         }
>
> +       /*
> +        * Clear tag references to avoid debug warning when using
> +        * __alloc_tag_ref_set() with non-empty reference.
> +        */
> +       set_codetag_empty(&ref_old);
> +       set_codetag_empty(&ref_new);
> +
>         /* swap tags */
>         __alloc_tag_ref_set(&ref_old, tag_new);
>         update_page_tag_ref(handle_old, &ref_old);
> --
> 2.39.2
>
[PATCH v3] mm/codetag: clear tags before swap
Posted by David Wang 1 year ago 
When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
triggered when calling __alloc_tag_ref_set() during swap:

	alloc_tag was not cleared (got tag for mm/filemap.c:1951)
	WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...

Clear code tags before swap can fix the warning. And this patch also fix
a potential invalid address dereference in alloc_tag_add_check() when
CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
which is defined as ((void *)1).

Signed-off-by: David Wang <00107082@163.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
Acked-by: Suren Baghdasaryan <surenb@google.com>
---
 include/linux/alloc_tag.h | 2 +-
 lib/alloc_tag.c           | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 7c0786bdf9af..cba024bf2db3 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -135,7 +135,7 @@ static inline struct alloc_tag_counters alloc_tag_read(struct alloc_tag *tag)
 #ifdef CONFIG_MEM_ALLOC_PROFILING_DEBUG
 static inline void alloc_tag_add_check(union codetag_ref *ref, struct alloc_tag *tag)
 {
-	WARN_ONCE(ref && ref->ct,
+	WARN_ONCE(ref && ref->ct && !is_codetag_empty(ref),
 		  "alloc_tag was not cleared (got tag for %s:%u)\n",
 		  ref->ct->filename, ref->ct->lineno);
 
diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
index 35f7560a309a..3a0413462e9f 100644
--- a/lib/alloc_tag.c
+++ b/lib/alloc_tag.c
@@ -209,6 +209,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
 		return;
 	}
 
+	/*
+	 * Clear tag references to avoid debug warning when using
+	 * __alloc_tag_ref_set() with non-empty reference.
+	 */
+	set_codetag_empty(&ref_old);
+	set_codetag_empty(&ref_new);
+
 	/* swap tags */
 	__alloc_tag_ref_set(&ref_old, tag_new);
 	update_page_tag_ref(handle_old, &ref_old);
-- 
2.39.2
Re: [PATCH v3] mm/codetag: clear tags before swap
Posted by Andrew Morton 1 year ago 
On Fri, 13 Dec 2024 09:33:32 +0800 David Wang <00107082@163.com> wrote:

> When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
> triggered when calling __alloc_tag_ref_set() during swap:
> 
> 	alloc_tag was not cleared (got tag for mm/filemap.c:1951)
> 	WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...
> 
> Clear code tags before swap can fix the warning. And this patch also fix
> a potential invalid address dereference in alloc_tag_add_check() when
> CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
> which is defined as ((void *)1).
> 
> Signed-off-by: David Wang <00107082@163.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com

This points at 

51f43d5d82ed ("mm/codetag: swap tags when migrate pages"), which had
	Fixes: e0a955bf7f61 ("mm/codetag: add pgalloc_tag_copy()")

e0a955bf7f61 ("mm/codetag: add pgalloc_tag_copy()") had
	Fixes: dcfe378c81f7 ("lib: introduce support for page allocation tagging")
	Cc: <stable@vger.kernel.org>

And I'm thinking that this fix should have
	Fixes: 51f43d5d82ed ("mm/codetag: swap tags when migrate pages")
	Cc: <stable@vger.kernel.org>
Re: [PATCH v3] mm/codetag: clear tags before swap
Posted by Suren Baghdasaryan 1 year ago 
On Thu, Dec 12, 2024 at 8:12 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 13 Dec 2024 09:33:32 +0800 David Wang <00107082@163.com> wrote:
>
> > When CONFIG_MEM_ALLOC_PROFILING_DEBUG is set, kernel WARN would be
> > triggered when calling __alloc_tag_ref_set() during swap:
> >
> >       alloc_tag was not cleared (got tag for mm/filemap.c:1951)
> >       WARNING: CPU: 0 PID: 816 at ./include/linux/alloc_tag.h...
> >
> > Clear code tags before swap can fix the warning. And this patch also fix
> > a potential invalid address dereference in alloc_tag_add_check() when
> > CONFIG_MEM_ALLOC_PROFILING_DEBUG is set and ref->ct is CODETAG_EMPTY,
> > which is defined as ((void *)1).
> >
> > Signed-off-by: David Wang <00107082@163.com>
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Closes: https://lore.kernel.org/oe-lkp/202412112227.df61ebb-lkp@intel.com
>
> This points at
>
> 51f43d5d82ed ("mm/codetag: swap tags when migrate pages"), which had
>         Fixes: e0a955bf7f61 ("mm/codetag: add pgalloc_tag_copy()")
>
> e0a955bf7f61 ("mm/codetag: add pgalloc_tag_copy()") had
>         Fixes: dcfe378c81f7 ("lib: introduce support for page allocation tagging")
>         Cc: <stable@vger.kernel.org>
>
> And I'm thinking that this fix should have
>         Fixes: 51f43d5d82ed ("mm/codetag: swap tags when migrate pages")
>         Cc: <stable@vger.kernel.org>

Yes, that is correct. Sorry for missing that.

>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.