diff mbox series

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

Message ID 20201125142858.2559-1-zqiu2000@126.com
State Superseded
Headers show
Series [RT] mm/swap: use local lock in deactivate_page() | expand

Commit Message

Zanxiong Qiu Nov. 25, 2020, 2:28 p.m. UTC
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(-)

Comments

Zanxiong Qiu Nov. 27, 2020, 11:31 a.m. UTC | #1
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

>
Sebastian Andrzej Siewior Nov. 27, 2020, 11:43 a.m. UTC | #2
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
diff mbox series

Patch

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);
 	}
 }