[PATCH] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[PATCH] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Wei Yang]

Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
fail zone_reclaim() as full") changed the return value of node_reclaim().
The original return value 0 means NODE_RECLAIM_SOME after this commit.

While the return value of node_reclaim() when CONFIG_NUMA is n is not
changed. This will leads to call zone_watermark_ok() again.

This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
it is not proper to include "mm/internal.h", just hard coded it.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---

This doesn't effect the system functionally. I am not sure we need to cc to
stable tree?

---
 include/linux/swap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index d8a07a4f171d..2bd993280470 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -364,7 +364,7 @@ extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
 static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
 				unsigned int order)
 {
-	return 0;
+	return -2;	/* NODE_RECLAIM_NOSCAN */
 }
 #endif
 
-- 
2.15.1

Re: [PATCH] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Matthew Wilcox]

On Tue, Nov 13, 2018 at 12:17:50PM +0800, Wei Yang wrote:
> Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
> fail zone_reclaim() as full") changed the return value of node_reclaim().
> The original return value 0 means NODE_RECLAIM_SOME after this commit.
> 
> While the return value of node_reclaim() when CONFIG_NUMA is n is not
> changed. This will leads to call zone_watermark_ok() again.
> 
> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
> it is not proper to include "mm/internal.h", just hard coded it.

Since the return value is defined in mm/internal.h that means no code
outside mm/ can call node_reclaim (nor should it).  So let's move both
of node_reclaim's declarations to mm/internal.h instead of keeping them
in linux/swap.h.

Re: [PATCH] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Wei Yang]

On Mon, Nov 12, 2018 at 09:36:15PM -0800, Matthew Wilcox wrote:
>On Tue, Nov 13, 2018 at 12:17:50PM +0800, Wei Yang wrote:
>> Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
>> fail zone_reclaim() as full") changed the return value of node_reclaim().
>> The original return value 0 means NODE_RECLAIM_SOME after this commit.
>> 
>> While the return value of node_reclaim() when CONFIG_NUMA is n is not
>> changed. This will leads to call zone_watermark_ok() again.
>> 
>> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
>> it is not proper to include "mm/internal.h", just hard coded it.
>
>Since the return value is defined in mm/internal.h that means no code
>outside mm/ can call node_reclaim (nor should it).  So let's move both
>of node_reclaim's declarations to mm/internal.h instead of keeping them
>in linux/swap.h.

That's reasonable, thanks.

-- 
Wei Yang
Help you, Help me

[PATCH v2] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Wei Yang]

Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
fail zone_reclaim() as full") changed the return value of node_reclaim().
The original return value 0 means NODE_RECLAIM_SOME after this commit.

While the return value of node_reclaim() when CONFIG_NUMA is n is not
changed. This will leads to call zone_watermark_ok() again.

This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
node_reclaim() is only called in page_alloc.c, move it to mm/internal.h.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
v2:  move node_reclaim() to mm/internal.h
---
 include/linux/swap.h |  6 ------
 mm/internal.h        | 10 ++++++++++
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/linux/swap.h b/include/linux/swap.h
index d8a07a4f171d..065988c27373 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -358,14 +358,8 @@ extern unsigned long vm_total_pages;
 extern int node_reclaim_mode;
 extern int sysctl_min_unmapped_ratio;
 extern int sysctl_min_slab_ratio;
-extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
 #else
 #define node_reclaim_mode 0
-static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
-				unsigned int order)
-{
-	return 0;
-}
 #endif
 
 extern int page_evictable(struct page *page);
diff --git a/mm/internal.h b/mm/internal.h
index 291eb2b6d1d8..6a57811ae47d 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -444,6 +444,16 @@ static inline void mminit_validate_memmodel_limits(unsigned long *start_pfn,
 #define NODE_RECLAIM_SOME	0
 #define NODE_RECLAIM_SUCCESS	1
 
+#ifdef CONFIG_NUMA
+extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
+#else
+static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
+				unsigned int order)
+{
+	return NODE_RECLAIM_NOSCAN;
+}
+#endif
+
 extern int hwpoison_filter(struct page *p);
 
 extern u32 hwpoison_filter_dev_major;
-- 
2.15.1

Re: [PATCH v2] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Michal Hocko]

On Tue 13-11-18 16:04:36, Wei Yang wrote:
> Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
> fail zone_reclaim() as full") changed the return value of node_reclaim().
> The original return value 0 means NODE_RECLAIM_SOME after this commit.
> 
> While the return value of node_reclaim() when CONFIG_NUMA is n is not
> changed. This will leads to call zone_watermark_ok() again.
> 
> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
> node_reclaim() is only called in page_alloc.c, move it to mm/internal.h.

The issue should be cosmetic but the code consistency is definitely an
improvement. Moving this from swap.h makes a lot of sense as well.

> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
> v2:  move node_reclaim() to mm/internal.h
> ---
>  include/linux/swap.h |  6 ------
>  mm/internal.h        | 10 ++++++++++
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index d8a07a4f171d..065988c27373 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -358,14 +358,8 @@ extern unsigned long vm_total_pages;
>  extern int node_reclaim_mode;
>  extern int sysctl_min_unmapped_ratio;
>  extern int sysctl_min_slab_ratio;
> -extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
>  #else
>  #define node_reclaim_mode 0
> -static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
> -				unsigned int order)
> -{
> -	return 0;
> -}
>  #endif
>  
>  extern int page_evictable(struct page *page);
> diff --git a/mm/internal.h b/mm/internal.h
> index 291eb2b6d1d8..6a57811ae47d 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -444,6 +444,16 @@ static inline void mminit_validate_memmodel_limits(unsigned long *start_pfn,
>  #define NODE_RECLAIM_SOME	0
>  #define NODE_RECLAIM_SUCCESS	1
>  
> +#ifdef CONFIG_NUMA
> +extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
> +#else
> +static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
> +				unsigned int order)
> +{
> +	return NODE_RECLAIM_NOSCAN;
> +}
> +#endif
> +
>  extern int hwpoison_filter(struct page *p);
>  
>  extern u32 hwpoison_filter_dev_major;
> -- 
> 2.15.1

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Wei Yang]

On Tue, Nov 13, 2018 at 01:56:11PM +0100, Michal Hocko wrote:
>On Tue 13-11-18 16:04:36, Wei Yang wrote:
>> Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
>> fail zone_reclaim() as full") changed the return value of node_reclaim().
>> The original return value 0 means NODE_RECLAIM_SOME after this commit.
>> 
>> While the return value of node_reclaim() when CONFIG_NUMA is n is not
>> changed. This will leads to call zone_watermark_ok() again.
>> 
>> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
>> node_reclaim() is only called in page_alloc.c, move it to mm/internal.h.
>
>The issue should be cosmetic but the code consistency is definitely an
>improvement. Moving this from swap.h makes a lot of sense as well.
>
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>Acked-by: Michal Hocko <mhocko@suse.com>

Thanks.

>
>> ---
>> v2:  move node_reclaim() to mm/internal.h
>> ---
>>  include/linux/swap.h |  6 ------
>>  mm/internal.h        | 10 ++++++++++
>>  2 files changed, 10 insertions(+), 6 deletions(-)
>> 
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index d8a07a4f171d..065988c27373 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -358,14 +358,8 @@ extern unsigned long vm_total_pages;
>>  extern int node_reclaim_mode;
>>  extern int sysctl_min_unmapped_ratio;
>>  extern int sysctl_min_slab_ratio;
>> -extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
>>  #else
>>  #define node_reclaim_mode 0
>> -static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
>> -				unsigned int order)
>> -{
>> -	return 0;
>> -}
>>  #endif
>>  
>>  extern int page_evictable(struct page *page);
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 291eb2b6d1d8..6a57811ae47d 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -444,6 +444,16 @@ static inline void mminit_validate_memmodel_limits(unsigned long *start_pfn,
>>  #define NODE_RECLAIM_SOME	0
>>  #define NODE_RECLAIM_SUCCESS	1
>>  
>> +#ifdef CONFIG_NUMA
>> +extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
>> +#else
>> +static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
>> +				unsigned int order)
>> +{
>> +	return NODE_RECLAIM_NOSCAN;
>> +}
>> +#endif
>> +
>>  extern int hwpoison_filter(struct page *p);
>>  
>>  extern u32 hwpoison_filter_dev_major;
>> -- 
>> 2.15.1
>
>-- 
>Michal Hocko
>SUSE Labs

-- 
Wei Yang
Help you, Help me

Re: [PATCH v2] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Matthew Wilcox]

On Tue, Nov 13, 2018 at 04:04:36PM +0800, Wei Yang wrote:
> Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
> fail zone_reclaim() as full") changed the return value of node_reclaim().
> The original return value 0 means NODE_RECLAIM_SOME after this commit.
> 
> While the return value of node_reclaim() when CONFIG_NUMA is n is not
> changed. This will leads to call zone_watermark_ok() again.
> 
> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
> node_reclaim() is only called in page_alloc.c, move it to mm/internal.h.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Reviewed-by: Matthew Wilcox <willy@infradead.org>

Re: [PATCH v2] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Wei Yang]

On Tue, Nov 13, 2018 at 05:04:20AM -0800, Matthew Wilcox wrote:
>On Tue, Nov 13, 2018 at 04:04:36PM +0800, Wei Yang wrote:
>> Commit fa5e084e43eb ("vmscan: do not unconditionally treat zones that
>> fail zone_reclaim() as full") changed the return value of node_reclaim().
>> The original return value 0 means NODE_RECLAIM_SOME after this commit.
>> 
>> While the return value of node_reclaim() when CONFIG_NUMA is n is not
>> changed. This will leads to call zone_watermark_ok() again.
>> 
>> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
>> node_reclaim() is only called in page_alloc.c, move it to mm/internal.h.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>Reviewed-by: Matthew Wilcox <willy@infradead.org>

Thanks

-- 
Wei Yang
Help you, Help me

Re: [PATCH v2] vmscan: return NODE_RECLAIM_NOSCAN in node_reclaim() when CONFIG_NUMA is n

[Matthew Wilcox]

On Tue, Nov 13, 2018 at 04:04:36PM +0800, Wei Yang wrote:
> This patch fix the return value by adjusting to NODE_RECLAIM_NOSCAN. Since
> node_reclaim() is only called in page_alloc.c, move it to mm/internal.h.

linux/swap.h is included in quite a few places in the kernel, but let's
see what's really used from it outside mm/

SWAP_FLAG* -- only used in mm/swapfile.c.  Move to swapfile.c?
current_is_kswapd() -- used by some drivers.
MAX_SWAPFILES* -- used by arch code.
union swap_header -- used by mtdswap.
struct reclaim_state -- used by fs/inode.c.
struct swap_extent -- embedded in swap_info_struct, which is used widely.
struct swap_cluster_info -- ditto
struct vma_swap_readahead -- only used in swap_state.c.  Move it there?
nr_free_pages() -- used in fs/ and kernel/power/swap.c
totalram_pages -- used widely
totalreserve_pages -- used widely
vm_swappiness -- used by sysctl
vm_total_pages -- only used in mm -- move to mm/internal.h?
node_reclaim_mode -- used by sysctl
kswapd_run -- only used in mm
kswapd_stop -- ditto
swap_address_space -- only used in mm
swapper_spaces -- likewise
SWAP_ADDRESS_SPACE* --likewise

I haven't covered all of the file, but there's definitely opportunity
for some followup patches to shrink linux/swap.h.