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 <[email protected]>
---
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
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.
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
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 <[email protected]>
---
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
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 <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> 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
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 <[email protected]>
Reviewed-by: Matthew Wilcox <[email protected]>
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 <[email protected]>
>
>Acked-by: Michal Hocko <[email protected]>
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
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 <[email protected]>
>
>Reviewed-by: Matthew Wilcox <[email protected]>
Thanks
--
Wei Yang
Help you, Help me
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.