diff mbox series

[v3,2/4] mm/hwpoison: move definitions of num_poisoned_pages_* to memory-failure.c

Message ID 20220921091359.25889-3-naoya.horiguchi@linux.dev (mailing list archive)
State New
Headers show
Series mm, hwpoison: improve handling workload related to hugetlb and memory_hotplug | expand

Commit Message

Naoya Horiguchi Sept. 21, 2022, 9:13 a.m. UTC 
From: Naoya Horiguchi <naoya.horiguchi@nec.com>

These interfaces will be used by drivers/base/core.c by later patch, so as a
preparatory work move them to more common header file visible to the file.

Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
---
ChangeLog v2 -> v3:
- added declaration of num_poisoned_pages_inc() in #ifdef CONFIG_MEMORY_FAILURE
---
 arch/parisc/kernel/pdt.c |  3 +--
 include/linux/mm.h       |  5 +++++
 include/linux/swapops.h  | 24 ++----------------------
 mm/memory-failure.c      | 10 ++++++++++
 4 files changed, 18 insertions(+), 24 deletions(-)

Comments

Miaohe Lin Sept. 24, 2022, 11:53 a.m. UTC | #1 
On 2022/9/21 17:13, Naoya Horiguchi wrote:
> From: Naoya Horiguchi <naoya.horiguchi@nec.com>
> 
> These interfaces will be used by drivers/base/core.c by later patch, so as a
> preparatory work move them to more common header file visible to the file.
> 
> Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
> ---
> ChangeLog v2 -> v3:
> - added declaration of num_poisoned_pages_inc() in #ifdef CONFIG_MEMORY_FAILURE
> ---
>  arch/parisc/kernel/pdt.c |  3 +--
>  include/linux/mm.h       |  5 +++++
>  include/linux/swapops.h  | 24 ++----------------------
>  mm/memory-failure.c      | 10 ++++++++++
>  4 files changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/parisc/kernel/pdt.c b/arch/parisc/kernel/pdt.c
> index e391b175f5ec..fdc880e2575a 100644
> --- a/arch/parisc/kernel/pdt.c
> +++ b/arch/parisc/kernel/pdt.c
> @@ -18,8 +18,7 @@
>  #include <linux/kthread.h>
>  #include <linux/initrd.h>
>  #include <linux/pgtable.h>
> -#include <linux/swap.h>

Is header file "linux/swap.h" already unneeded before the code change? It seems there's
no code change in that file.

> -#include <linux/swapops.h>
> +#include <linux/mm.h>
>  
>  #include <asm/pdc.h>
>  #include <asm/pdcpat.h>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c2277f5aba9e..80a2d800f272 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3279,11 +3279,16 @@ extern atomic_long_t num_poisoned_pages __read_mostly;
>  extern int soft_offline_page(unsigned long pfn, int flags);
>  #ifdef CONFIG_MEMORY_FAILURE
>  extern int __get_huge_page_for_hwpoison(unsigned long pfn, int flags);
> +extern void num_poisoned_pages_inc(void);
>  #else
>  static inline int __get_huge_page_for_hwpoison(unsigned long pfn, int flags)
>  {
>  	return 0;
>  }
> +
> +static inline void num_poisoned_pages_inc(void)
> +{
> +}
>  #endif
>  
>  #ifndef arch_memory_failure
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index a91dd08e107b..3e58a812399a 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -581,8 +581,6 @@ static inline int is_pmd_migration_entry(pmd_t pmd)
>  
>  #ifdef CONFIG_MEMORY_FAILURE
>  
> -extern atomic_long_t num_poisoned_pages __read_mostly;
> -
>  /*
>   * Support for hardware poisoned pages
>   */
> @@ -610,17 +608,7 @@ static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
>  	return p;
>  }
>  
> -static inline void num_poisoned_pages_inc(void)
> -{
> -	atomic_long_inc(&num_poisoned_pages);
> -}
> -
> -static inline void num_poisoned_pages_sub(long i)
> -{
> -	atomic_long_sub(i, &num_poisoned_pages);
> -}
> -
> -#else  /* CONFIG_MEMORY_FAILURE */
> +#else
>  
>  static inline swp_entry_t make_hwpoison_entry(struct page *page)
>  {
> @@ -636,15 +624,7 @@ static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
>  {
>  	return NULL;
>  }
> -
> -static inline void num_poisoned_pages_inc(void)
> -{
> -}
> -
> -static inline void num_poisoned_pages_sub(long i)
> -{
> -}
> -#endif  /* CONFIG_MEMORY_FAILURE */
> +#endif
>  
>  static inline int non_swap_entry(swp_entry_t entry)
>  {
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 5942e1c0407e..aa6ce685b863 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -74,6 +74,16 @@ atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
>  
>  static bool hw_memory_failure __read_mostly = false;
>  
> +static inline void num_poisoned_pages_inc(void)

This function is defined as "static inline" while it's "extern void num_poisoned_pages_inc(void)"
in the header file. Is this expected?

Thanks,
Miaohe Lin

> +{
> +	atomic_long_inc(&num_poisoned_pages);
> +}
> +
> +static inline void num_poisoned_pages_sub(long i)
> +{
> +	atomic_long_sub(i, &num_poisoned_pages);
> +}
> +
>  /*
>   * Return values:
>   *   1:   the page is dissolved (if needed) and taken off from buddy,
>
Naoya Horiguchi Sept. 28, 2022, 2:05 a.m. UTC | #2 
On Sat, Sep 24, 2022 at 07:53:15PM +0800, Miaohe Lin wrote:
> On 2022/9/21 17:13, Naoya Horiguchi wrote:
> > From: Naoya Horiguchi <naoya.horiguchi@nec.com>
> > 
> > These interfaces will be used by drivers/base/core.c by later patch, so as a
> > preparatory work move them to more common header file visible to the file.
> > 
> > Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
> > ---
> > ChangeLog v2 -> v3:
> > - added declaration of num_poisoned_pages_inc() in #ifdef CONFIG_MEMORY_FAILURE
> > ---
> >  arch/parisc/kernel/pdt.c |  3 +--
> >  include/linux/mm.h       |  5 +++++
> >  include/linux/swapops.h  | 24 ++----------------------
> >  mm/memory-failure.c      | 10 ++++++++++
> >  4 files changed, 18 insertions(+), 24 deletions(-)
> > 
> > diff --git a/arch/parisc/kernel/pdt.c b/arch/parisc/kernel/pdt.c
> > index e391b175f5ec..fdc880e2575a 100644
> > --- a/arch/parisc/kernel/pdt.c
> > +++ b/arch/parisc/kernel/pdt.c
> > @@ -18,8 +18,7 @@
> >  #include <linux/kthread.h>
> >  #include <linux/initrd.h>
> >  #include <linux/pgtable.h>
> > -#include <linux/swap.h>
> 
> Is header file "linux/swap.h" already unneeded before the code change? It seems there's
> no code change in that file.

Maybe yes.  I updated this line too because it's introduced together
swapops.h by the following commit.

  commit 0e5a7ff6e36ad58933d076ddcac36ff14d014692
  Author: Helge Deller <deller@gmx.de>
  Date:   Fri Jul 24 19:17:52 2020 +0200
  
      parisc: Report bad pages as HardwareCorrupted

> 
> > -#include <linux/swapops.h>
> > +#include <linux/mm.h>
> >  
> >  #include <asm/pdc.h>
> >  #include <asm/pdcpat.h>
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index c2277f5aba9e..80a2d800f272 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -3279,11 +3279,16 @@ extern atomic_long_t num_poisoned_pages __read_mostly;
> >  extern int soft_offline_page(unsigned long pfn, int flags);
> >  #ifdef CONFIG_MEMORY_FAILURE
> >  extern int __get_huge_page_for_hwpoison(unsigned long pfn, int flags);
> > +extern void num_poisoned_pages_inc(void);
> >  #else
> >  static inline int __get_huge_page_for_hwpoison(unsigned long pfn, int flags)
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline void num_poisoned_pages_inc(void)
> > +{
> > +}
> >  #endif
> >  
> >  #ifndef arch_memory_failure
> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > index a91dd08e107b..3e58a812399a 100644
> > --- a/include/linux/swapops.h
> > +++ b/include/linux/swapops.h
> > @@ -581,8 +581,6 @@ static inline int is_pmd_migration_entry(pmd_t pmd)
> >  
> >  #ifdef CONFIG_MEMORY_FAILURE
> >  
> > -extern atomic_long_t num_poisoned_pages __read_mostly;
> > -
> >  /*
> >   * Support for hardware poisoned pages
> >   */
> > @@ -610,17 +608,7 @@ static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
> >  	return p;
> >  }
> >  
> > -static inline void num_poisoned_pages_inc(void)
> > -{
> > -	atomic_long_inc(&num_poisoned_pages);
> > -}
> > -
> > -static inline void num_poisoned_pages_sub(long i)
> > -{
> > -	atomic_long_sub(i, &num_poisoned_pages);
> > -}
> > -
> > -#else  /* CONFIG_MEMORY_FAILURE */
> > +#else
> >  
> >  static inline swp_entry_t make_hwpoison_entry(struct page *page)
> >  {
> > @@ -636,15 +624,7 @@ static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
> >  {
> >  	return NULL;
> >  }
> > -
> > -static inline void num_poisoned_pages_inc(void)
> > -{
> > -}
> > -
> > -static inline void num_poisoned_pages_sub(long i)
> > -{
> > -}
> > -#endif  /* CONFIG_MEMORY_FAILURE */
> > +#endif
> >  
> >  static inline int non_swap_entry(swp_entry_t entry)
> >  {
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index 5942e1c0407e..aa6ce685b863 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -74,6 +74,16 @@ atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
> >  
> >  static bool hw_memory_failure __read_mostly = false;
> >  
> > +static inline void num_poisoned_pages_inc(void)
> 
> This function is defined as "static inline" while it's "extern void num_poisoned_pages_inc(void)"
> in the header file. Is this expected?

No. 4/4 effectively fixes it, but I should've done this in this patch.
Thank you,
- Naoya Horiguchi

> 
> Thanks,
> Miaohe Lin
> 
> > +{
> > +	atomic_long_inc(&num_poisoned_pages);
> > +}
> > +
> > +static inline void num_poisoned_pages_sub(long i)
> > +{
> > +	atomic_long_sub(i, &num_poisoned_pages);
> > +}
> > +
> >  /*
> >   * Return values:
> >   *   1:   the page is dissolved (if needed) and taken off from buddy,
> > 
>
Miaohe Lin Sept. 28, 2022, 7:56 a.m. UTC | #3 
On 2022/9/28 10:05, Naoya Horiguchi wrote:
> On Sat, Sep 24, 2022 at 07:53:15PM +0800, Miaohe Lin wrote:
>> On 2022/9/21 17:13, Naoya Horiguchi wrote:
>>> From: Naoya Horiguchi <naoya.horiguchi@nec.com>
>>>
>>> These interfaces will be used by drivers/base/core.c by later patch, so as a
>>> preparatory work move them to more common header file visible to the file.
>>>
>>> Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
>>> ---
>>> ChangeLog v2 -> v3:
>>> - added declaration of num_poisoned_pages_inc() in #ifdef CONFIG_MEMORY_FAILURE
>>> ---
>>>  arch/parisc/kernel/pdt.c |  3 +--
>>>  include/linux/mm.h       |  5 +++++
>>>  include/linux/swapops.h  | 24 ++----------------------
>>>  mm/memory-failure.c      | 10 ++++++++++
>>>  4 files changed, 18 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/arch/parisc/kernel/pdt.c b/arch/parisc/kernel/pdt.c
>>> index e391b175f5ec..fdc880e2575a 100644
>>> --- a/arch/parisc/kernel/pdt.c
>>> +++ b/arch/parisc/kernel/pdt.c
>>> @@ -18,8 +18,7 @@
>>>  #include <linux/kthread.h>
>>>  #include <linux/initrd.h>
>>>  #include <linux/pgtable.h>
>>> -#include <linux/swap.h>
>>
>> Is header file "linux/swap.h" already unneeded before the code change? It seems there's
>> no code change in that file.
> 
> Maybe yes.  I updated this line too because it's introduced together
> swapops.h by the following commit.
> 
>   commit 0e5a7ff6e36ad58933d076ddcac36ff14d014692
>   Author: Helge Deller <deller@gmx.de>
>   Date:   Fri Jul 24 19:17:52 2020 +0200
>   
>       parisc: Report bad pages as HardwareCorrupted
> 

I see. Many thanks for your explanation.

>>
>>> -#include <linux/swapops.h>
>>> +#include <linux/mm.h>
>>>  
>>>  #include <asm/pdc.h>
>>>  #include <asm/pdcpat.h>
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index c2277f5aba9e..80a2d800f272 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -3279,11 +3279,16 @@ extern atomic_long_t num_poisoned_pages __read_mostly;
>>>  extern int soft_offline_page(unsigned long pfn, int flags);
>>>  #ifdef CONFIG_MEMORY_FAILURE
>>>  extern int __get_huge_page_for_hwpoison(unsigned long pfn, int flags);
>>> +extern void num_poisoned_pages_inc(void);
>>>  #else
>>>  static inline int __get_huge_page_for_hwpoison(unsigned long pfn, int flags)
>>>  {
>>>  	return 0;
>>>  }
>>> +
>>> +static inline void num_poisoned_pages_inc(void)
>>> +{
>>> +}
>>>  #endif
>>>  
>>>  #ifndef arch_memory_failure
>>> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
>>> index a91dd08e107b..3e58a812399a 100644
>>> --- a/include/linux/swapops.h
>>> +++ b/include/linux/swapops.h
>>> @@ -581,8 +581,6 @@ static inline int is_pmd_migration_entry(pmd_t pmd)
>>>  
>>>  #ifdef CONFIG_MEMORY_FAILURE
>>>  
>>> -extern atomic_long_t num_poisoned_pages __read_mostly;
>>> -
>>>  /*
>>>   * Support for hardware poisoned pages
>>>   */
>>> @@ -610,17 +608,7 @@ static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
>>>  	return p;
>>>  }
>>>  
>>> -static inline void num_poisoned_pages_inc(void)
>>> -{
>>> -	atomic_long_inc(&num_poisoned_pages);
>>> -}
>>> -
>>> -static inline void num_poisoned_pages_sub(long i)
>>> -{
>>> -	atomic_long_sub(i, &num_poisoned_pages);
>>> -}
>>> -
>>> -#else  /* CONFIG_MEMORY_FAILURE */
>>> +#else
>>>  
>>>  static inline swp_entry_t make_hwpoison_entry(struct page *page)
>>>  {
>>> @@ -636,15 +624,7 @@ static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
>>>  {
>>>  	return NULL;
>>>  }
>>> -
>>> -static inline void num_poisoned_pages_inc(void)
>>> -{
>>> -}
>>> -
>>> -static inline void num_poisoned_pages_sub(long i)
>>> -{
>>> -}
>>> -#endif  /* CONFIG_MEMORY_FAILURE */
>>> +#endif
>>>  
>>>  static inline int non_swap_entry(swp_entry_t entry)
>>>  {
>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>> index 5942e1c0407e..aa6ce685b863 100644
>>> --- a/mm/memory-failure.c
>>> +++ b/mm/memory-failure.c
>>> @@ -74,6 +74,16 @@ atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
>>>  
>>>  static bool hw_memory_failure __read_mostly = false;
>>>  
>>> +static inline void num_poisoned_pages_inc(void)
>>
>> This function is defined as "static inline" while it's "extern void num_poisoned_pages_inc(void)"
>> in the header file. Is this expected?
> 
> No. 4/4 effectively fixes it, but I should've done this in this patch.
> Thank you,
> - Naoya Horiguchi

Then this patch looks good to me. Thanks.

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>

Thanks,
Miaohe Lin
diff mbox series

Patch

diff --git a/arch/parisc/kernel/pdt.c b/arch/parisc/kernel/pdt.c
index e391b175f5ec..fdc880e2575a 100644
--- a/arch/parisc/kernel/pdt.c
+++ b/arch/parisc/kernel/pdt.c
@@ -18,8 +18,7 @@ 
 #include <linux/kthread.h>
 #include <linux/initrd.h>
 #include <linux/pgtable.h>
-#include <linux/swap.h>
-#include <linux/swapops.h>
+#include <linux/mm.h>
 
 #include <asm/pdc.h>
 #include <asm/pdcpat.h>
diff --git a/include/linux/mm.h b/include/linux/mm.h
index c2277f5aba9e..80a2d800f272 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3279,11 +3279,16 @@  extern atomic_long_t num_poisoned_pages __read_mostly;
 extern int soft_offline_page(unsigned long pfn, int flags);
 #ifdef CONFIG_MEMORY_FAILURE
 extern int __get_huge_page_for_hwpoison(unsigned long pfn, int flags);
+extern void num_poisoned_pages_inc(void);
 #else
 static inline int __get_huge_page_for_hwpoison(unsigned long pfn, int flags)
 {
 	return 0;
 }
+
+static inline void num_poisoned_pages_inc(void)
+{
+}
 #endif
 
 #ifndef arch_memory_failure
diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index a91dd08e107b..3e58a812399a 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -581,8 +581,6 @@  static inline int is_pmd_migration_entry(pmd_t pmd)
 
 #ifdef CONFIG_MEMORY_FAILURE
 
-extern atomic_long_t num_poisoned_pages __read_mostly;
-
 /*
  * Support for hardware poisoned pages
  */
@@ -610,17 +608,7 @@  static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
 	return p;
 }
 
-static inline void num_poisoned_pages_inc(void)
-{
-	atomic_long_inc(&num_poisoned_pages);
-}
-
-static inline void num_poisoned_pages_sub(long i)
-{
-	atomic_long_sub(i, &num_poisoned_pages);
-}
-
-#else  /* CONFIG_MEMORY_FAILURE */
+#else
 
 static inline swp_entry_t make_hwpoison_entry(struct page *page)
 {
@@ -636,15 +624,7 @@  static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
 {
 	return NULL;
 }
-
-static inline void num_poisoned_pages_inc(void)
-{
-}
-
-static inline void num_poisoned_pages_sub(long i)
-{
-}
-#endif  /* CONFIG_MEMORY_FAILURE */
+#endif
 
 static inline int non_swap_entry(swp_entry_t entry)
 {
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 5942e1c0407e..aa6ce685b863 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -74,6 +74,16 @@  atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
 
 static bool hw_memory_failure __read_mostly = false;
 
+static inline void num_poisoned_pages_inc(void)
+{
+	atomic_long_inc(&num_poisoned_pages);
+}
+
+static inline void num_poisoned_pages_sub(long i)
+{
+	atomic_long_sub(i, &num_poisoned_pages);
+}
+
 /*
  * Return values:
  *   1:   the page is dissolved (if needed) and taken off from buddy,