[PATCH RT] mm/swap: use local lock in deactivate_page()

[PATCH RT] mm/swap: use local lock in deactivate_page()

[zqiu2000]

From: Zanxiong Qiu <zqiu2000@126.com>

get_cpu_var() calls preempt_disable(), while on RT kernel,
pagevec_lru_move_fn() will call spinlock and might schedule
the context out and hence the schedule bug occurred, issue
is found on 5.4.70-rt40 and reproducable on 5.4.74-rt41.

[  306.340109] BUG: scheduling while atomic: stress-ng-vm/3361/0x00000002
...
[  306.340143] Preemption disabled at:
[  306.340143] [<ffffffffaa42c2fd>] deactivate_page+0x5d/0x110
...
[  306.340151] Call Trace:
[  306.340153]  dump_stack+0x50/0x70
[  306.340157]  ? deactivate_page+0x5d/0x110
[  306.340158]  __schedule_bug.cold+0x89/0x96
[  306.340160]  __schedule+0x576/0x860
[  306.340163]  ? _raw_spin_lock+0x13/0x30
[  306.340164]  schedule+0x43/0xd0
[  306.340166]  rt_spin_lock_slowlock_locked+0x117/0x2c0
[  306.340168]  ? __activate_page+0x2f0/0x2f0
[  306.340169]  rt_spin_lock_slowlock+0x51/0x80
[  306.340171]  pagevec_lru_move_fn+0x62/0xc0
[  306.340172]  deactivate_page+0xb3/0x110
[  306.340174]  madvise_cold_or_pageout_pte_range+0x277/0x2d0
[  306.340176]  ? free_unref_page_list+0x3ac/0x3c0
[  306.340177]  __walk_page_range+0x1f4/0x490
[  306.340181]  walk_page_range+0x89/0x110
[  306.340182]  madvise_cold+0x7e/0xc0
[  306.340183]  ? syscall_return_via_sysret+0xf/0x7f
[  306.340185]  ? __switch_to_asm+0x34/0x70
[  306.340186]  ? __switch_to_asm+0x40/0x70
[  306.340187]  ? __switch_to_asm+0x34/0x70
[  306.340188]  ? __switch_to_asm+0x40/0x70
[  306.340188]  ? _raw_spin_unlock_irq+0x17/0x50
[  306.340189]  ? find_vma+0x16/0x70
[  306.340190]  __do_sys_madvise+0x328/0x810
[  306.340193]  ? do_syscall_64+0x67/0x1f0
[  306.340195]  do_syscall_64+0x67/0x1f0
[  306.340196]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  306.340197] RIP: 0033:0x7f12e177510b

2154a0abcc9 ("mm: Revert the DEFINE_PER_CPU_PAGEVEC implementation")
reverted the lock/unlock_swap_pvec function, however, get_cpu_var()
was added back in deactivate_page(), actually, get_locked_var()
shall be used instead to avoid preempt_disable() call for RT.

Signed-off-by: Zanxiong Qiu <zqiu2000@126.com>
---
 mm/swap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index cdb4f1fa3a48..463cac334fcf 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -666,12 +666,13 @@ void deactivate_file_page(struct page *page)
 void deactivate_page(struct page *page)
 {
 	if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
-		struct pagevec *pvec = &get_cpu_var(lru_deactivate_pvecs);
+		struct pagevec *pvec = &get_locked_var(swapvec_lock,
+							lru_deactivate_pvecs);
 
 		get_page(page);
 		if (!pagevec_add(pvec, page) || PageCompound(page))
 			pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
-		put_cpu_var(lru_deactivate_pvecs);
+		put_locked_var(swapvec_lock, lru_deactivate_pvecs);
 	}
 }
 
-- 
2.17.1

Re: [PATCH RT] mm/swap: use local lock in deactivate_page()

[Sebastian Andrzej Siewior]

On 2020-11-25 22:28:58 [+0800], zqiu2000@126.com wrote:
> From: Zanxiong Qiu <zqiu2000@126.com>
> 
> get_cpu_var() calls preempt_disable(), while on RT kernel,
> pagevec_lru_move_fn() will call spinlock and might schedule
> the context out and hence the schedule bug occurred, issue
> is found on 5.4.70-rt40 and reproducable on 5.4.74-rt41.
> 
…

> 2154a0abcc9 ("mm: Revert the DEFINE_PER_CPU_PAGEVEC implementation")
> reverted the lock/unlock_swap_pvec function, however, get_cpu_var()
> was added back in deactivate_page(), actually, get_locked_var()
> shall be used instead to avoid preempt_disable() call for RT.

The commit is
    32154a0abcc97 ("mm: Revert the DEFINE_PER_CPU_PAGEVEC implementation")

the reasoning is correct. deactivate_page() was added in v5.4 and I
missed that part when I added back the old patches which did not handle
it :/

Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> Signed-off-by: Zanxiong Qiu <zqiu2000@126.com>
> ---
>  mm/swap.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index cdb4f1fa3a48..463cac334fcf 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -666,12 +666,13 @@ void deactivate_file_page(struct page *page)
>  void deactivate_page(struct page *page)
>  {
>  	if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
> -		struct pagevec *pvec = &get_cpu_var(lru_deactivate_pvecs);
> +		struct pagevec *pvec = &get_locked_var(swapvec_lock,
> +							lru_deactivate_pvecs);
>  
>  		get_page(page);
>  		if (!pagevec_add(pvec, page) || PageCompound(page))
>  			pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
> -		put_cpu_var(lru_deactivate_pvecs);
> +		put_locked_var(swapvec_lock, lru_deactivate_pvecs);
>  	}
>  }
>  

Sebastian

Re: [PATCH RT] mm/swap: use local lock in deactivate_page()

[Zanxiong Qiu]

On 2020/11/27 上午12:35, Sebastian Andrzej Siewior wrote:
> On 2020-11-25 22:28:58 [+0800], zqiu2000@126.com wrote:
>> From: Zanxiong Qiu <zqiu2000@126.com>
>>
>> get_cpu_var() calls preempt_disable(), while on RT kernel,
>> pagevec_lru_move_fn() will call spinlock and might schedule
>> the context out and hence the schedule bug occurred, issue
>> is found on 5.4.70-rt40 and reproducable on 5.4.74-rt41.
>>
> …
> 
>> 2154a0abcc9 ("mm: Revert the DEFINE_PER_CPU_PAGEVEC implementation")
>> reverted the lock/unlock_swap_pvec function, however, get_cpu_var()
>> was added back in deactivate_page(), actually, get_locked_var()
>> shall be used instead to avoid preempt_disable() call for RT.
> 
> The commit is
>      32154a0abcc97 ("mm: Revert the DEFINE_PER_CPU_PAGEVEC implementation")
> 
> the reasoning is correct. deactivate_page() was added in v5.4 and I
> missed that part when I added back the old patches which did not handle
> it :/
> 
> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 

My bad, will correct the commit msg and resend, thanks.

>> Signed-off-by: Zanxiong Qiu <zqiu2000@126.com>
>> ---
>>   mm/swap.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/swap.c b/mm/swap.c
>> index cdb4f1fa3a48..463cac334fcf 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -666,12 +666,13 @@ void deactivate_file_page(struct page *page)
>>   void deactivate_page(struct page *page)
>>   {
>>   	if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
>> -		struct pagevec *pvec = &get_cpu_var(lru_deactivate_pvecs);
>> +		struct pagevec *pvec = &get_locked_var(swapvec_lock,
>> +							lru_deactivate_pvecs);
>>   
>>   		get_page(page);
>>   		if (!pagevec_add(pvec, page) || PageCompound(page))
>>   			pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
>> -		put_cpu_var(lru_deactivate_pvecs);
>> +		put_locked_var(swapvec_lock, lru_deactivate_pvecs);
>>   	}
>>   }
>>   
> 
> Sebastian
> 

Re: [PATCH RT] mm/swap: use local lock in deactivate_page()

[Sebastian Andrzej Siewior]

On 2020-11-27 19:31:47 [+0800], Zanxiong Qiu wrote:
> My bad, will correct the commit msg and resend, thanks.

Okay, thanks. Please also omit the stack trace from the commit message
as it does not add any additional value. The error is quite obvious if
you look at it.

Sebastian