This patchset only performs some small improvement to mm. First, make
several functions return bool to improve readability, and then remove
unused is_unevictable_lru function and refactor memmap_valid_within
for simplicity.
No functional change.
Yaowei Bai (7):
ipc/shm: is_file_shm_hugepages can be boolean
mm/hugetlb: is_file_hugepages can be boolean
mm/memblock: memblock_is_memory/reserved can be boolean
mm/vmscan: page_is_file_cache can be boolean
mm/lru: remove unused is_unevictable_lru function
mm/gfp: make gfp_zonelist return directly and bool
mm/mmzone: refactor memmap_valid_within
include/linux/gfp.h | 7 ++-----
include/linux/hugetlb.h | 10 ++++------
include/linux/memblock.h | 4 ++--
include/linux/mm_inline.h | 6 +++---
include/linux/mmzone.h | 11 +++--------
include/linux/shm.h | 6 +++---
ipc/shm.c | 2 +-
mm/memblock.c | 4 ++--
mm/mmzone.c | 10 ++--------
9 files changed, 22 insertions(+), 38 deletions(-)
--
1.9.1
This patch makes is_file_shm_hugepages return bool to improve
readability due to this particular function only using either
one or zero as its return value.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/shm.h | 6 +++---
ipc/shm.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/shm.h b/include/linux/shm.h
index 6fb8016..04e8818 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -52,7 +52,7 @@ struct sysv_shm {
long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr,
unsigned long shmlba);
-int is_file_shm_hugepages(struct file *file);
+bool is_file_shm_hugepages(struct file *file);
void exit_shm(struct task_struct *task);
#define shm_init_task(task) INIT_LIST_HEAD(&(task)->sysvshm.shm_clist)
#else
@@ -66,9 +66,9 @@ static inline long do_shmat(int shmid, char __user *shmaddr,
{
return -ENOSYS;
}
-static inline int is_file_shm_hugepages(struct file *file)
+static inline bool is_file_shm_hugepages(struct file *file)
{
- return 0;
+ return false;
}
static inline void exit_shm(struct task_struct *task)
{
diff --git a/ipc/shm.c b/ipc/shm.c
index 4178727..ed3027d 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -459,7 +459,7 @@ static const struct file_operations shm_file_operations_huge = {
.fallocate = shm_fallocate,
};
-int is_file_shm_hugepages(struct file *file)
+bool is_file_shm_hugepages(struct file *file)
{
return file->f_op == &shm_file_operations_huge;
}
--
1.9.1
This patch makes is_file_hugepages return bool to improve
readability due to this particular function only using either
one or zero as its return value.
This patch also removed the if condition to make is_file_hugepages
return directly.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/hugetlb.h | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 685c262..204c7f5 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -265,20 +265,18 @@ struct file *hugetlb_file_setup(const char *name, size_t size, vm_flags_t acct,
struct user_struct **user, int creat_flags,
int page_size_log);
-static inline int is_file_hugepages(struct file *file)
+static inline bool is_file_hugepages(struct file *file)
{
if (file->f_op == &hugetlbfs_file_operations)
- return 1;
- if (is_file_shm_hugepages(file))
- return 1;
+ return true;
- return 0;
+ return is_file_shm_hugepages(file);
}
#else /* !CONFIG_HUGETLBFS */
-#define is_file_hugepages(file) 0
+#define is_file_hugepages(file) false
static inline struct file *
hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
struct user_struct **user, int creat_flags,
--
1.9.1
This patch makes memblock_is_memory/reserved return bool to improve
readability due to this particular function only using either
one or zero as its return value.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/memblock.h | 4 ++--
mm/memblock.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 24daf8f..a25cce94 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -318,9 +318,9 @@ phys_addr_t memblock_mem_size(unsigned long limit_pfn);
phys_addr_t memblock_start_of_DRAM(void);
phys_addr_t memblock_end_of_DRAM(void);
void memblock_enforce_memory_limit(phys_addr_t memory_limit);
-int memblock_is_memory(phys_addr_t addr);
+bool memblock_is_memory(phys_addr_t addr);
int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
-int memblock_is_reserved(phys_addr_t addr);
+bool memblock_is_reserved(phys_addr_t addr);
bool memblock_is_region_reserved(phys_addr_t base, phys_addr_t size);
extern void __memblock_dump_all(void);
diff --git a/mm/memblock.c b/mm/memblock.c
index d300f13..1ab7b9e 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1509,12 +1509,12 @@ static int __init_memblock memblock_search(struct memblock_type *type, phys_addr
return -1;
}
-int __init memblock_is_reserved(phys_addr_t addr)
+bool __init memblock_is_reserved(phys_addr_t addr)
{
return memblock_search(&memblock.reserved, addr) != -1;
}
-int __init_memblock memblock_is_memory(phys_addr_t addr)
+bool __init_memblock memblock_is_memory(phys_addr_t addr)
{
return memblock_search(&memblock.memory, addr) != -1;
}
--
1.9.1
This patch makes page_is_file_cache return bool to improve
readability due to this particular function only using either
one or zero as its return value.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/mm_inline.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index cf55945..af73135 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -8,8 +8,8 @@
* page_is_file_cache - should the page be on a file LRU or anon LRU?
* @page: the page to test
*
- * Returns 1 if @page is page cache page backed by a regular filesystem,
- * or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed.
+ * Returns true if @page is page cache page backed by a regular filesystem,
+ * or false if @page is anonymous, tmpfs or otherwise ram or swap backed.
* Used by functions that manipulate the LRU lists, to sort a page
* onto the right LRU list.
*
@@ -17,7 +17,7 @@
* needs to survive until the page is last deleted from the LRU, which
* could be as far down as __page_cache_release.
*/
-static inline int page_is_file_cache(struct page *page)
+static inline bool page_is_file_cache(struct page *page)
{
return !PageSwapBacked(page);
}
--
1.9.1
Since commit a0b8cab3 ("mm: remove lru parameter from __pagevec_lru_add
and remove parts of pagevec API") there's no user of this function anymore,
so remove it.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/mmzone.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index e23a9e7..9963846 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -195,11 +195,6 @@ static inline int is_active_lru(enum lru_list lru)
return (lru == LRU_ACTIVE_ANON || lru == LRU_ACTIVE_FILE);
}
-static inline int is_unevictable_lru(enum lru_list lru)
-{
- return (lru == LRU_UNEVICTABLE);
-}
-
struct zone_reclaim_stat {
/*
* The pageout code in vmscan.c keeps track of how many of the
--
1.9.1
This patch makes gfp_zonelist return bool due to this
particular function only using either one or zero as its return
value.
This patch also makes gfp_zonelist return directly by removing if.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/gfp.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 6523109..1da03f5 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -375,12 +375,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
* virtual kernel addresses to the allocated page(s).
*/
-static inline int gfp_zonelist(gfp_t flags)
+static inline bool gfp_zonelist(gfp_t flags)
{
- if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
- return 1;
-
- return 0;
+ return IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE);
}
/*
--
1.9.1
This patch makes memmap_valid_within return bool due to this
particular function only using either one or zero as its return
value.
This patch also refactors memmap_valid_within for simplicity.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/mmzone.h | 6 +++---
mm/mmzone.c | 10 ++--------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 9963846..b9b59bb8 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1202,13 +1202,13 @@ unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
* the zone and PFN linkages are still valid. This is expensive, but walkers
* of the full memmap are extremely rare.
*/
-int memmap_valid_within(unsigned long pfn,
+bool memmap_valid_within(unsigned long pfn,
struct page *page, struct zone *zone);
#else
-static inline int memmap_valid_within(unsigned long pfn,
+static inline bool memmap_valid_within(unsigned long pfn,
struct page *page, struct zone *zone)
{
- return 1;
+ return true;
}
#endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
diff --git a/mm/mmzone.c b/mm/mmzone.c
index 7d87ebb..de0824e 100644
--- a/mm/mmzone.c
+++ b/mm/mmzone.c
@@ -72,16 +72,10 @@ struct zoneref *next_zones_zonelist(struct zoneref *z,
}
#ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
-int memmap_valid_within(unsigned long pfn,
+bool memmap_valid_within(unsigned long pfn,
struct page *page, struct zone *zone)
{
- if (page_to_pfn(page) != pfn)
- return 0;
-
- if (page_zone(page) != zone)
- return 0;
-
- return 1;
+ return page_to_pfn(page) == pfn && page_zone(page) == zone;
}
#endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
--
1.9.1
On Mon, 16 Nov 2015, Yaowei Bai wrote:
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 6523109..1da03f5 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -375,12 +375,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> * virtual kernel addresses to the allocated page(s).
> */
>
> -static inline int gfp_zonelist(gfp_t flags)
> +static inline bool gfp_zonelist(gfp_t flags)
> {
> - if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> - return 1;
> -
> - return 0;
> + return IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE);
> }
>
> /*
This function is used to index into a pgdat's node_zonelists[] array, bool
makes no sense.
On Mon, 16 Nov 2015, Yaowei Bai wrote:
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index cf55945..af73135 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -8,8 +8,8 @@
> * page_is_file_cache - should the page be on a file LRU or anon LRU?
> * @page: the page to test
> *
> - * Returns 1 if @page is page cache page backed by a regular filesystem,
> - * or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed.
> + * Returns true if @page is page cache page backed by a regular filesystem,
> + * or false if @page is anonymous, tmpfs or otherwise ram or swap backed.
> * Used by functions that manipulate the LRU lists, to sort a page
> * onto the right LRU list.
> *
> @@ -17,7 +17,7 @@
> * needs to survive until the page is last deleted from the LRU, which
> * could be as far down as __page_cache_release.
> */
> -static inline int page_is_file_cache(struct page *page)
> +static inline bool page_is_file_cache(struct page *page)
> {
> return !PageSwapBacked(page);
> }
Since page_is_file_cache() is often used to determine which zlc to
increment or decrement (usage such as
NR_ISOLATED_ANON + page_is_file_cache(page)), I don't think this style is
helpful.
On Mon 16-11-15 14:51:20, Yaowei Bai wrote:
> This patch makes is_file_shm_hugepages return bool to improve
> readability due to this particular function only using either
> one or zero as its return value.
yes it makes sense here.
> No functional change.
>
> Signed-off-by: Yaowei Bai <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> include/linux/shm.h | 6 +++---
> ipc/shm.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/shm.h b/include/linux/shm.h
> index 6fb8016..04e8818 100644
> --- a/include/linux/shm.h
> +++ b/include/linux/shm.h
> @@ -52,7 +52,7 @@ struct sysv_shm {
>
> long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr,
> unsigned long shmlba);
> -int is_file_shm_hugepages(struct file *file);
> +bool is_file_shm_hugepages(struct file *file);
> void exit_shm(struct task_struct *task);
> #define shm_init_task(task) INIT_LIST_HEAD(&(task)->sysvshm.shm_clist)
> #else
> @@ -66,9 +66,9 @@ static inline long do_shmat(int shmid, char __user *shmaddr,
> {
> return -ENOSYS;
> }
> -static inline int is_file_shm_hugepages(struct file *file)
> +static inline bool is_file_shm_hugepages(struct file *file)
> {
> - return 0;
> + return false;
> }
> static inline void exit_shm(struct task_struct *task)
> {
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 4178727..ed3027d 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -459,7 +459,7 @@ static const struct file_operations shm_file_operations_huge = {
> .fallocate = shm_fallocate,
> };
>
> -int is_file_shm_hugepages(struct file *file)
> +bool is_file_shm_hugepages(struct file *file)
> {
> return file->f_op == &shm_file_operations_huge;
> }
> --
> 1.9.1
>
>
--
Michal Hocko
SUSE Labs
On Mon 16-11-15 14:51:21, Yaowei Bai wrote:
> This patch makes is_file_hugepages return bool to improve
> readability due to this particular function only using either
> one or zero as its return value.
>
> This patch also removed the if condition to make is_file_hugepages
> return directly.
>
> No functional change.
>
> Signed-off-by: Yaowei Bai <[email protected]>
I think this could be squashed into the previous patch.
Acked-by: Michal Hocko <[email protected]>
> ---
> include/linux/hugetlb.h | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 685c262..204c7f5 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -265,20 +265,18 @@ struct file *hugetlb_file_setup(const char *name, size_t size, vm_flags_t acct,
> struct user_struct **user, int creat_flags,
> int page_size_log);
>
> -static inline int is_file_hugepages(struct file *file)
> +static inline bool is_file_hugepages(struct file *file)
> {
> if (file->f_op == &hugetlbfs_file_operations)
> - return 1;
> - if (is_file_shm_hugepages(file))
> - return 1;
> + return true;
>
> - return 0;
> + return is_file_shm_hugepages(file);
> }
>
>
> #else /* !CONFIG_HUGETLBFS */
>
> -#define is_file_hugepages(file) 0
> +#define is_file_hugepages(file) false
> static inline struct file *
> hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
> struct user_struct **user, int creat_flags,
> --
> 1.9.1
>
>
--
Michal Hocko
SUSE Labs
On Mon 16-11-15 14:51:22, Yaowei Bai wrote:
> This patch makes memblock_is_memory/reserved return bool to improve
> readability due to this particular function only using either
> one or zero as its return value.
>
> No functional change.
>
> Signed-off-by: Yaowei Bai <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> include/linux/memblock.h | 4 ++--
> mm/memblock.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 24daf8f..a25cce94 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -318,9 +318,9 @@ phys_addr_t memblock_mem_size(unsigned long limit_pfn);
> phys_addr_t memblock_start_of_DRAM(void);
> phys_addr_t memblock_end_of_DRAM(void);
> void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> -int memblock_is_memory(phys_addr_t addr);
> +bool memblock_is_memory(phys_addr_t addr);
> int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> -int memblock_is_reserved(phys_addr_t addr);
> +bool memblock_is_reserved(phys_addr_t addr);
> bool memblock_is_region_reserved(phys_addr_t base, phys_addr_t size);
>
> extern void __memblock_dump_all(void);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index d300f13..1ab7b9e 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1509,12 +1509,12 @@ static int __init_memblock memblock_search(struct memblock_type *type, phys_addr
> return -1;
> }
>
> -int __init memblock_is_reserved(phys_addr_t addr)
> +bool __init memblock_is_reserved(phys_addr_t addr)
> {
> return memblock_search(&memblock.reserved, addr) != -1;
> }
>
> -int __init_memblock memblock_is_memory(phys_addr_t addr)
> +bool __init_memblock memblock_is_memory(phys_addr_t addr)
> {
> return memblock_search(&memblock.memory, addr) != -1;
> }
> --
> 1.9.1
>
>
--
Michal Hocko
SUSE Labs
On Mon 16-11-15 14:51:24, Yaowei Bai wrote:
> Since commit a0b8cab3 ("mm: remove lru parameter from __pagevec_lru_add
> and remove parts of pagevec API") there's no user of this function anymore,
> so remove it.
>
> Signed-off-by: Yaowei Bai <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> include/linux/mmzone.h | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index e23a9e7..9963846 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -195,11 +195,6 @@ static inline int is_active_lru(enum lru_list lru)
> return (lru == LRU_ACTIVE_ANON || lru == LRU_ACTIVE_FILE);
> }
>
> -static inline int is_unevictable_lru(enum lru_list lru)
> -{
> - return (lru == LRU_UNEVICTABLE);
> -}
> -
> struct zone_reclaim_stat {
> /*
> * The pageout code in vmscan.c keeps track of how many of the
> --
> 1.9.1
>
>
--
Michal Hocko
SUSE Labs
On Mon 16-11-15 14:51:26, Yaowei Bai wrote:
[...]
> @@ -72,16 +72,10 @@ struct zoneref *next_zones_zonelist(struct zoneref *z,
> }
>
> #ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
> -int memmap_valid_within(unsigned long pfn,
> +bool memmap_valid_within(unsigned long pfn,
> struct page *page, struct zone *zone)
> {
> - if (page_to_pfn(page) != pfn)
> - return 0;
> -
> - if (page_zone(page) != zone)
> - return 0;
> -
> - return 1;
> + return page_to_pfn(page) == pfn && page_zone(page) == zone;
I do not think this is easier to read. Quite contrary
> }
> #endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
>
> --
> 1.9.1
>
>
--
Michal Hocko
SUSE Labs
On Mon, Nov 16, 2015 at 02:05:46AM -0800, David Rientjes wrote:
> On Mon, 16 Nov 2015, Yaowei Bai wrote:
>
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 6523109..1da03f5 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -375,12 +375,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> > * virtual kernel addresses to the allocated page(s).
> > */
> >
> > -static inline int gfp_zonelist(gfp_t flags)
> > +static inline bool gfp_zonelist(gfp_t flags)
> > {
> > - if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> > - return 1;
> > -
> > - return 0;
> > + return IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE);
> > }
> >
> > /*
>
> This function is used to index into a pgdat's node_zonelists[] array, bool
> makes no sense.
Yes, you'r right, but i think hardcoding the index here is not a good idea.
How about this:
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 6523109..14a6249 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
static inline int gfp_zonelist(gfp_t flags)
{
if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
- return 1;
+ return ZONELIST_NOFALLBACK;
- return 0;
+ return ZONELIST_FALLBACK;
}
/*
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index e23a9e7..9664d6c 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -576,8 +576,6 @@ static inline bool zone_is_empty(struct zone *zone)
/* Maximum number of zones on a zonelist */
#define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
-#ifdef CONFIG_NUMA
-
/*
* The NUMA zonelists are doubled because we need zonelists that restrict the
* allocations to a single node for __GFP_THISNODE.
@@ -585,10 +583,13 @@ static inline bool zone_is_empty(struct zone *zone)
* [0] : Zonelist with fallback
* [1] : No fallback (__GFP_THISNODE)
*/
-#define MAX_ZONELISTS 2
-#else
-#define MAX_ZONELISTS 1
+enum {
+ ZONELIST_FALLBACK,
+#ifdef CONFIG_NUMA
+ ZONELIST_NOFALLBACK,
#endif
+ MAX_ZONELISTS
+};
/*
* This struct contains information about a zone in a zonelist. It is stored
On Mon, Nov 16, 2015 at 02:10:10AM -0800, David Rientjes wrote:
> On Mon, 16 Nov 2015, Yaowei Bai wrote:
>
> > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> > index cf55945..af73135 100644
> > --- a/include/linux/mm_inline.h
> > +++ b/include/linux/mm_inline.h
> > @@ -8,8 +8,8 @@
> > * page_is_file_cache - should the page be on a file LRU or anon LRU?
> > * @page: the page to test
> > *
> > - * Returns 1 if @page is page cache page backed by a regular filesystem,
> > - * or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed.
> > + * Returns true if @page is page cache page backed by a regular filesystem,
> > + * or false if @page is anonymous, tmpfs or otherwise ram or swap backed.
> > * Used by functions that manipulate the LRU lists, to sort a page
> > * onto the right LRU list.
> > *
> > @@ -17,7 +17,7 @@
> > * needs to survive until the page is last deleted from the LRU, which
> > * could be as far down as __page_cache_release.
> > */
> > -static inline int page_is_file_cache(struct page *page)
> > +static inline bool page_is_file_cache(struct page *page)
> > {
> > return !PageSwapBacked(page);
> > }
>
> Since page_is_file_cache() is often used to determine which zlc to
> increment or decrement (usage such as
> NR_ISOLATED_ANON + page_is_file_cache(page)), I don't think this style is
> helpful.
Yes, you'r right, we can drop this one.
On Mon, Nov 16, 2015 at 01:45:01PM +0100, Michal Hocko wrote:
> On Mon 16-11-15 14:51:26, Yaowei Bai wrote:
> [...]
> > @@ -72,16 +72,10 @@ struct zoneref *next_zones_zonelist(struct zoneref *z,
> > }
> >
> > #ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
> > -int memmap_valid_within(unsigned long pfn,
> > +bool memmap_valid_within(unsigned long pfn,
> > struct page *page, struct zone *zone)
> > {
> > - if (page_to_pfn(page) != pfn)
> > - return 0;
> > -
> > - if (page_zone(page) != zone)
> > - return 0;
> > -
> > - return 1;
> > + return page_to_pfn(page) == pfn && page_zone(page) == zone;
>
> I do not think this is easier to read. Quite contrary
OK, so we can just make it return ture/false without refactoring it.
>
> > }
> > #endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
> >
> > --
> > 1.9.1
> >
> >
>
> --
> Michal Hocko
> SUSE Labs
>
> Since commit a0b8cab3 ("mm: remove lru parameter from __pagevec_lru_add
> and remove parts of pagevec API") there's no user of this function anymore,
> so remove it.
>
> Signed-off-by: Yaowei Bai <[email protected]>
> ---
Acked-by: Hillf Danton <[email protected]>
> include/linux/mmzone.h | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index e23a9e7..9963846 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -195,11 +195,6 @@ static inline int is_active_lru(enum lru_list lru)
> return (lru == LRU_ACTIVE_ANON || lru == LRU_ACTIVE_FILE);
> }
>
> -static inline int is_unevictable_lru(enum lru_list lru)
> -{
> - return (lru == LRU_UNEVICTABLE);
> -}
> -
> struct zone_reclaim_stat {
> /*
> * The pageout code in vmscan.c keeps track of how many of the
> --
> 1.9.1
>
On Tue, 17 Nov 2015, Yaowei Bai wrote:
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 6523109..14a6249 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> static inline int gfp_zonelist(gfp_t flags)
> {
> if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> - return 1;
> + return ZONELIST_NOFALLBACK;
>
> - return 0;
> + return ZONELIST_FALLBACK;
> }
>
> /*
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index e23a9e7..9664d6c 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -576,8 +576,6 @@ static inline bool zone_is_empty(struct zone *zone)
> /* Maximum number of zones on a zonelist */
> #define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
>
> -#ifdef CONFIG_NUMA
> -
> /*
> * The NUMA zonelists are doubled because we need zonelists that restrict the
> * allocations to a single node for __GFP_THISNODE.
> @@ -585,10 +583,13 @@ static inline bool zone_is_empty(struct zone *zone)
> * [0] : Zonelist with fallback
> * [1] : No fallback (__GFP_THISNODE)
> */
> -#define MAX_ZONELISTS 2
> -#else
> -#define MAX_ZONELISTS 1
> +enum {
> + ZONELIST_FALLBACK,
> +#ifdef CONFIG_NUMA
> + ZONELIST_NOFALLBACK,
> #endif
> + MAX_ZONELISTS
> +};
>
> /*
> * This struct contains information about a zone in a zonelist. It is stored
This is a different change than the original. I don't see a benefit from
it, but I have no strong feelings on it. If someone else finds value in
this, please update the comment when defining the enum as well.
On Mon, Nov 16, 2015 at 09:44:11PM -0800, David Rientjes wrote:
> On Tue, 17 Nov 2015, Yaowei Bai wrote:
>
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 6523109..14a6249 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
> > static inline int gfp_zonelist(gfp_t flags)
> > {
> > if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
> > - return 1;
> > + return ZONELIST_NOFALLBACK;
> >
> > - return 0;
> > + return ZONELIST_FALLBACK;
> > }
> >
> > /*
> > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > index e23a9e7..9664d6c 100644
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -576,8 +576,6 @@ static inline bool zone_is_empty(struct zone *zone)
> > /* Maximum number of zones on a zonelist */
> > #define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
> >
> > -#ifdef CONFIG_NUMA
> > -
> > /*
> > * The NUMA zonelists are doubled because we need zonelists that restrict the
> > * allocations to a single node for __GFP_THISNODE.
> > @@ -585,10 +583,13 @@ static inline bool zone_is_empty(struct zone *zone)
> > * [0] : Zonelist with fallback
> > * [1] : No fallback (__GFP_THISNODE)
> > */
> > -#define MAX_ZONELISTS 2
> > -#else
> > -#define MAX_ZONELISTS 1
> > +enum {
> > + ZONELIST_FALLBACK,
> > +#ifdef CONFIG_NUMA
> > + ZONELIST_NOFALLBACK,
> > #endif
> > + MAX_ZONELISTS
> > +};
> >
> > /*
> > * This struct contains information about a zone in a zonelist. It is stored
>
> This is a different change than the original.
The original patch doesn't make sense as you said so let's drop it.
> I don't see a benefit from
> it, but I have no strong feelings on it. If someone else finds value in
> this, please update the comment when defining the enum as well.
OK, i'll send a update patch to review first and if nobody disagrees with it i will
resend this patchset.
Hardcoding index to zonelists array in gfp_zonelist() is not a good idea,
let's enumerate it to improve readability.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/gfp.h | 4 ++--
include/linux/mmzone.h | 20 +++++++++-----------
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 6523109..14a6249 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -378,9 +378,9 @@ static inline enum zone_type gfp_zone(gfp_t flags)
static inline int gfp_zonelist(gfp_t flags)
{
if (IS_ENABLED(CONFIG_NUMA) && unlikely(flags & __GFP_THISNODE))
- return 1;
+ return ZONELIST_NOFALLBACK;
- return 0;
+ return ZONELIST_FALLBACK;
}
/*
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index e23a9e7..bb31301 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -576,19 +576,17 @@ static inline bool zone_is_empty(struct zone *zone)
/* Maximum number of zones on a zonelist */
#define MAX_ZONES_PER_ZONELIST (MAX_NUMNODES * MAX_NR_ZONES)
+enum {
+ ZONELIST_FALLBACK, /* zonelist with fallback */
#ifdef CONFIG_NUMA
-
-/*
- * The NUMA zonelists are doubled because we need zonelists that restrict the
- * allocations to a single node for __GFP_THISNODE.
- *
- * [0] : Zonelist with fallback
- * [1] : No fallback (__GFP_THISNODE)
- */
-#define MAX_ZONELISTS 2
-#else
-#define MAX_ZONELISTS 1
+ /*
+ * The NUMA zonelists are doubled because we need zonelists that restrict
+ * the allocations to a single node for __GFP_THISNODE.
+ */
+ ZONELIST_NOFALLBACK, /* zonelist without fallback (__GFP_THISNODE) */
#endif
+ MAX_ZONELISTS
+};
/*
* This struct contains information about a zone in a zonelist. It is stored
--
1.9.1
This patch makes memmap_valid_within return bool due to this
particular function only using either one or zero as its return
value.
No functional change.
Signed-off-by: Yaowei Bai <[email protected]>
---
include/linux/mmzone.h | 6 +++---
mm/mmzone.c | 8 ++++----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 9963846..b9b59bb8 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1202,13 +1202,13 @@ unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
* the zone and PFN linkages are still valid. This is expensive, but walkers
* of the full memmap are extremely rare.
*/
-int memmap_valid_within(unsigned long pfn,
+bool memmap_valid_within(unsigned long pfn,
struct page *page, struct zone *zone);
#else
-static inline int memmap_valid_within(unsigned long pfn,
+static inline bool memmap_valid_within(unsigned long pfn,
struct page *page, struct zone *zone)
{
- return 1;
+ return true;
}
#endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
diff --git a/mm/mmzone.c b/mm/mmzone.c
index 7d87ebb..52687fb 100644
--- a/mm/mmzone.c
+++ b/mm/mmzone.c
@@ -72,16 +72,16 @@ struct zoneref *next_zones_zonelist(struct zoneref *z,
}
#ifdef CONFIG_ARCH_HAS_HOLES_MEMORYMODEL
-int memmap_valid_within(unsigned long pfn,
+bool memmap_valid_within(unsigned long pfn,
struct page *page, struct zone *zone)
{
if (page_to_pfn(page) != pfn)
- return 0;
+ return false;
if (page_zone(page) != zone)
- return 0;
+ return false;
- return 1;
+ return true;
}
#endif /* CONFIG_ARCH_HAS_HOLES_MEMORYMODEL */
--
1.9.1