[PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

page->lru bit 0 can tell whether the page is
avtive page or not.
page->lru bit 1 can tell whether the page is
file page or not.

Signed-off-by: Hui Su <sh_def@163.com>
---
 include/linux/mmzone.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..294369c652d0 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -258,12 +258,12 @@ enum lru_list {
 
 static inline bool is_file_lru(enum lru_list lru)
 {
-	return (lru == LRU_INACTIVE_FILE || lru == LRU_ACTIVE_FILE);
+	return lru & LRU_FILE;
 }
 
 static inline bool is_active_lru(enum lru_list lru)
 {
-	return (lru == LRU_ACTIVE_ANON || lru == LRU_ACTIVE_FILE);
+	return lru & LRU_ACTIVE;
 }
 
 #define ANON_AND_FILE 2
-- 
2.29.0

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

On Wed, Nov 18, 2020 at 12:49:19AM +0800, Hui Su wrote:
> page->lru bit 0 can tell whether the page is
> avtive page or not.
> page->lru bit 1 can tell whether the page is
> file page or not.
> 
> Signed-off-by: Hui Su <sh_def@163.com>
> ---
>  include/linux/mmzone.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index fb3bf696c05e..294369c652d0 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -258,12 +258,12 @@ enum lru_list {
>  
>  static inline bool is_file_lru(enum lru_list lru)
>  {
> -	return (lru == LRU_INACTIVE_FILE || lru == LRU_ACTIVE_FILE);
> +	return lru & LRU_FILE;
>  }
>  
>  static inline bool is_active_lru(enum lru_list lru)
>  {
> -	return (lru == LRU_ACTIVE_ANON || lru == LRU_ACTIVE_FILE);
> +	return lru & LRU_ACTIVE;
>  }
>  
>  #define ANON_AND_FILE 2
> -- 
> 2.29.0

The commit message is not correct, please ignore this change.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Matthew Wilcox]

On Wed, Nov 18, 2020 at 12:49:19AM +0800, Hui Su wrote:
> page->lru bit 0 can tell whether the page is
> avtive page or not.
> page->lru bit 1 can tell whether the page is
> file page or not.

This is wrong.  I'll let you figure out why.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

On Tue, Nov 17, 2020 at 05:59:00PM +0000, Matthew Wilcox wrote:
> On Wed, Nov 18, 2020 at 01:56:58AM +0800, Hui Su wrote:
> > On Tue, Nov 17, 2020 at 05:47:27PM +0000, Matthew Wilcox wrote:
> > > On Wed, Nov 18, 2020 at 01:46:33AM +0800, Hui Su wrote:
> > > > On Tue, Nov 17, 2020 at 05:41:17PM +0000, Matthew Wilcox wrote:
> > > > > On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> > > > > > lru_list lru bit 0 can tell whether the list is
> > > > > > avtive lru-list or not.
> > > > > > lru_list lru bit 1 can tell whether the list is
> > > > > > file lru-list or not.
> > > > > > 
> > > > > > And fix some define type in shrink_active_list()
> > > > > > and get_scan_count().
> > > > > > 
> > > > > > v1->v2:
> > > > > > correct the commit message, and fix the define type.
> > > > > 
> > > > > No, still incorrect.
> > > > 
> > > > I am a little confused, can you tell in detail?
> > > 
> > > Have you booted a kernel with this change?  Have you run any kind of
> > > tests on it?
> > 
> > Yes, I boot it with qemu-system-x86_64-4.1.50 on ubuntu20.04:
> > qemu-system-x86_64 -kernel /home/rlk/workspace/compile/out/arch/x86_64/boot/bzImage -hda \
> >  /home/rlk/myspace/qemu_build/rootfs.img -append "root=/dev/sda console=ttyS0" -nographic
> > 
> > using the kernel compiled with ubuntu20.04's default .config.
> > 
> > It seems nothing abnormal.
> > And i did not do other test.
> 
> Maybe you should ... how about LTP?  I think that's pretty straightforward
> to set up and run.

Hi Matthew:

Thanks for your advice, i finally setup a test platform powered by
jenkins and QEMU emulator version 4.2.1 with two evening. This is
the things that i always wanted to do but didn't do.

I apply this change based on the commit: 4d02da974ea8, and compile
it with x86_64_defconfig:
make bzImage O=/var/lib/jenkins/workspace/linux_kernel_ltp/out/

Then start qemu:
sudo qemu-system-x86_64 \
    -kernel /var/lib/jenkins/workspace/linux_kernel_ltp/out/arch/x86/boot/bzImage \
    -hda /home/ubuntu/myspace/qemu_build/ubuntu.img \
    -hdb /home/ubuntu/myspace/qemu_build/init.img       \
    -append "root=/dev/sda console=ttyS0" \
    -nographic \
    -m 1024 \
    -fsdev local,id=fs1,path=/home/ubuntu/workspace,security_model=none \
    -device virtio-9p-pci,fsdev=fs1,mount_tag=jeff-host-code

Then run the ltp test:
sudo bash /opt/ltp/runltp -f mm

The result is:
Test Start Time: Sat Nov 21 02:06:46 2020
-----------------------------------------
Testcase                                           Result     Exit Value
--------                                           ------     ----------
mm01                                               PASS       0    
mm02                                               PASS       0    
mtest01                                            PASS       0    
mtest01w                                           PASS       0    
mtest05                                            PASS       0    
mtest06                                            FAIL       2    
mtest06_2                                          PASS       0    
mtest06_3                                          PASS       0    
mem02                                              PASS       0    

mmapstress01                                       PASS       0    
mmapstress02                                       PASS       0    
mmapstress03                                       PASS       0    
mmapstress04                                       PASS       0    
mmapstress05                                       PASS       0    
mmapstress06                                       PASS       0    
mmapstress07                                       PASS       0    
mmapstress08                                       PASS       0    
mmapstress09                                       PASS       0    
mmapstress10                                       PASS       0    
mmap10                                             PASS       0    
mmap10_1                                           PASS       0    
mmap10_2                                           CONF       32   
mmap10_3                                           CONF       32   
mmap10_4                                           CONF       32   
ksm01                                              CONF       32   
ksm01_1                                            CONF       32   
ksm02                                              CONF       32   
ksm02_1                                            CONF       32   
ksm03                                              CONF       32   
ksm03_1                                            CONF       32   
ksm04                                              CONF       32   
ksm04_1                                            CONF       32   
ksm05                                              CONF       32   
ksm06                                              CONF       32   

thp02                                              CONF       32   
thp03                                              CONF       32   
thp04                                              CONF       32   
vma01                                              PASS       0    
vma02                                              CONF       32   
vma03                                              CONF       32   
vma04                                              CONF       32   
vma05                                              CONF       32   
overcommit_memory01                                PASS       0    
overcommit_memory02                                CONF       32   
overcommit_memory03                                PASS       0    
overcommit_memory04                                PASS       0    
overcommit_memory05                                PASS       0    
overcommit_memory06                                PASS       0    
max_map_count                                      PASS       0    
min_free_kbytes                                    PASS       0    

-----------------------------------------------
Total Tests: 76
Total Skipped Tests: 29
Total Failures: 1
Kernel Version: 5.10.0-rc4+
Machine Architecture: x86_64
Hostname: ubuntu

The one failure happens without my change, too.(maybe my config is
not correct? I am not sure). 

Last, i think this change have no problem.
And what's your opinions, Matthew.

Thanks.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

On Tue, Nov 17, 2020 at 05:59:00PM +0000, Matthew Wilcox wrote:
> On Wed, Nov 18, 2020 at 01:56:58AM +0800, Hui Su wrote:
> > On Tue, Nov 17, 2020 at 05:47:27PM +0000, Matthew Wilcox wrote:
> > > On Wed, Nov 18, 2020 at 01:46:33AM +0800, Hui Su wrote:
> > > > On Tue, Nov 17, 2020 at 05:41:17PM +0000, Matthew Wilcox wrote:
> > > > > On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> > > > > > lru_list lru bit 0 can tell whether the list is
> > > > > > avtive lru-list or not.
> > > > > > lru_list lru bit 1 can tell whether the list is
> > > > > > file lru-list or not.
> > > > > > 
> > > > > > And fix some define type in shrink_active_list()
> > > > > > and get_scan_count().
> > > > > > 
> > > > > > v1->v2:
> > > > > > correct the commit message, and fix the define type.
> > > > > 
> > > > > No, still incorrect.
> > > > 
> > > > I am a little confused, can you tell in detail?
> > > 
> > > Have you booted a kernel with this change?  Have you run any kind of
> > > tests on it?
> > 
> > Yes, I boot it with qemu-system-x86_64-4.1.50 on ubuntu20.04:
> > qemu-system-x86_64 -kernel /home/rlk/workspace/compile/out/arch/x86_64/boot/bzImage -hda \
> >  /home/rlk/myspace/qemu_build/rootfs.img -append "root=/dev/sda console=ttyS0" -nographic
> > 
> > using the kernel compiled with ubuntu20.04's default .config.
> > 
> > It seems nothing abnormal.
> > And i did not do other test.
> 
> Maybe you should ... how about LTP?  I think that's pretty straightforward
> to set up and run.

Thanks for your advice, Matthew.

I will set up an ltp test envirment, thanks again.

And Please ignore this change for now.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Matthew Wilcox]

On Wed, Nov 18, 2020 at 01:56:58AM +0800, Hui Su wrote:
> On Tue, Nov 17, 2020 at 05:47:27PM +0000, Matthew Wilcox wrote:
> > On Wed, Nov 18, 2020 at 01:46:33AM +0800, Hui Su wrote:
> > > On Tue, Nov 17, 2020 at 05:41:17PM +0000, Matthew Wilcox wrote:
> > > > On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> > > > > lru_list lru bit 0 can tell whether the list is
> > > > > avtive lru-list or not.
> > > > > lru_list lru bit 1 can tell whether the list is
> > > > > file lru-list or not.
> > > > > 
> > > > > And fix some define type in shrink_active_list()
> > > > > and get_scan_count().
> > > > > 
> > > > > v1->v2:
> > > > > correct the commit message, and fix the define type.
> > > > 
> > > > No, still incorrect.
> > > 
> > > I am a little confused, can you tell in detail?
> > 
> > Have you booted a kernel with this change?  Have you run any kind of
> > tests on it?
> 
> Yes, I boot it with qemu-system-x86_64-4.1.50 on ubuntu20.04:
> qemu-system-x86_64 -kernel /home/rlk/workspace/compile/out/arch/x86_64/boot/bzImage -hda \
>  /home/rlk/myspace/qemu_build/rootfs.img -append "root=/dev/sda console=ttyS0" -nographic
> 
> using the kernel compiled with ubuntu20.04's default .config.
> 
> It seems nothing abnormal.
> And i did not do other test.

Maybe you should ... how about LTP?  I think that's pretty straightforward
to set up and run.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

On Tue, Nov 17, 2020 at 05:47:27PM +0000, Matthew Wilcox wrote:
> On Wed, Nov 18, 2020 at 01:46:33AM +0800, Hui Su wrote:
> > On Tue, Nov 17, 2020 at 05:41:17PM +0000, Matthew Wilcox wrote:
> > > On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> > > > lru_list lru bit 0 can tell whether the list is
> > > > avtive lru-list or not.
> > > > lru_list lru bit 1 can tell whether the list is
> > > > file lru-list or not.
> > > > 
> > > > And fix some define type in shrink_active_list()
> > > > and get_scan_count().
> > > > 
> > > > v1->v2:
> > > > correct the commit message, and fix the define type.
> > > 
> > > No, still incorrect.
> > 
> > I am a little confused, can you tell in detail?
> 
> Have you booted a kernel with this change?  Have you run any kind of
> tests on it?

Yes, I boot it with qemu-system-x86_64-4.1.50 on ubuntu20.04:
qemu-system-x86_64 -kernel /home/rlk/workspace/compile/out/arch/x86_64/boot/bzImage -hda \
 /home/rlk/myspace/qemu_build/rootfs.img -append "root=/dev/sda console=ttyS0" -nographic

using the kernel compiled with ubuntu20.04's default .config.

It seems nothing abnormal.
And i did not do other test.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Matthew Wilcox]

On Wed, Nov 18, 2020 at 01:46:33AM +0800, Hui Su wrote:
> On Tue, Nov 17, 2020 at 05:41:17PM +0000, Matthew Wilcox wrote:
> > On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> > > lru_list lru bit 0 can tell whether the list is
> > > avtive lru-list or not.
> > > lru_list lru bit 1 can tell whether the list is
> > > file lru-list or not.
> > > 
> > > And fix some define type in shrink_active_list()
> > > and get_scan_count().
> > > 
> > > v1->v2:
> > > correct the commit message, and fix the define type.
> > 
> > No, still incorrect.
> 
> I am a little confused, can you tell in detail?

Have you booted a kernel with this change?  Have you run any kind of
tests on it?

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

On Tue, Nov 17, 2020 at 05:41:17PM +0000, Matthew Wilcox wrote:
> On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> > lru_list lru bit 0 can tell whether the list is
> > avtive lru-list or not.
> > lru_list lru bit 1 can tell whether the list is
> > file lru-list or not.
> > 
> > And fix some define type in shrink_active_list()
> > and get_scan_count().
> > 
> > v1->v2:
> > correct the commit message, and fix the define type.
> 
> No, still incorrect.

I am a little confused, can you tell in detail?

Thanks.

Re: [PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Matthew Wilcox]

On Wed, Nov 18, 2020 at 01:12:42AM +0800, Hui Su wrote:
> lru_list lru bit 0 can tell whether the list is
> avtive lru-list or not.
> lru_list lru bit 1 can tell whether the list is
> file lru-list or not.
> 
> And fix some define type in shrink_active_list()
> and get_scan_count().
> 
> v1->v2:
> correct the commit message, and fix the define type.

No, still incorrect.

[PATCH] mm/lru: simplify is_file_lru() and is_active_lru()

[Hui Su]

lru_list lru bit 0 can tell whether the list is
avtive lru-list or not.
lru_list lru bit 1 can tell whether the list is
file lru-list or not.

And fix some define type in shrink_active_list()
and get_scan_count().

v1->v2:
correct the commit message, and fix the define type.

Signed-off-by: Hui Su <sh_def@163.com>
---
 include/linux/mmzone.h | 4 ++--
 mm/vmscan.c            | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..294369c652d0 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -258,12 +258,12 @@ enum lru_list {
 
 static inline bool is_file_lru(enum lru_list lru)
 {
-	return (lru == LRU_INACTIVE_FILE || lru == LRU_ACTIVE_FILE);
+	return lru & LRU_FILE;
 }
 
 static inline bool is_active_lru(enum lru_list lru)
 {
-	return (lru == LRU_ACTIVE_ANON || lru == LRU_ACTIVE_FILE);
+	return lru & LRU_ACTIVE;
 }
 
 #define ANON_AND_FILE 2
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7b4e31eac2cf..7d8ea2fc87fc 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2020,7 +2020,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
 	struct page *page;
 	unsigned nr_deactivate, nr_activate;
 	unsigned nr_rotated = 0;
-	int file = is_file_lru(lru);
+	bool file = is_file_lru(lru);
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 
 	lru_add_drain();
@@ -2323,7 +2323,7 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 	denominator = ap + fp;
 out:
 	for_each_evictable_lru(lru) {
-		int file = is_file_lru(lru);
+		bool file = is_file_lru(lru);
 		unsigned long lruvec_size;
 		unsigned long scan;
 		unsigned long protection;
-- 
2.29.0