linux-kernel - Re: [RFC][PATCH] lru_add_drain_all() don't use schedule_on_each

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20081029172157.080de70b.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Wed, 29 Oct 2008 17:21:57 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	"KOSAKI Motohiro" <kosaki.motohiro@...fujitsu.com>
Cc:	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Christoph Lameter" <cl@...ux-foundation.org>,
	heiko.carstens@...ibm.com, npiggin@...e.de,
	linux-kernel@...r.kernel.org, hugh@...itas.com,
	torvalds@...ux-foundation.org, riel@...hat.com,
	lee.schermerhorn@...com, linux-mm@...ck.org
Subject: Re: [RFC][PATCH] lru_add_drain_all() don't use
 schedule_on_each_cpu()

On Wed, 29 Oct 2008 16:20:24 +0900
"KOSAKI Motohiro" <kosaki.motohiro@...fujitsu.com> wrote:

> > I guess we should document our newly discovered schedule_on_each_cpu()
> > problems before we forget about it and later rediscover it.
> 
> Now, schedule_on_each_cpu() is only used by lru_add_drain_all().
> and smp_call_function() is better way for cross call.
> 
> So I propose
>    1. lru_add_drain_all() use smp_call_function()
IMHO, smp_call_function() is not good, either.

The real problem in this lru_add_drain_all() around mlock() is handling of
pagevec. How about attached one ?(not tested at all..just an idea.)

>    2. remove schedule_on_each_cpu()
> 
I'm using schedule_on_each_cpu() from not dangerous context (in new memcg patch..)

Thanks,
-Kame

==
pagevec is used for avoidning lru_lock contention for add/remove pages to/from
LRU. But under split-lru/unevictable lru world, this delay in pagevec can
cause unexpected behavior.
  * A page scheduled to add to Unevictable lru is unlocked
    while it's in pagevec.
Because a page wrongly linked to Unevictable lru cannot come back to usual
lru, this is a problem. To avoid this kind of situation, lru_add_drain_all()
is called from mlock() path.


This patch remove "delay" of pagevec for Unevictable pages and remove
lru_add_drain_all(), which is a burtal function should not be called from
deep under the kernel.




---
 mm/mlock.c |   13 ++-----------
 mm/swap.c  |   17 +++++++++++++----
 2 files changed, 15 insertions(+), 15 deletions(-)

Index: mmotm-2.6.27+/mm/mlock.c
===================================================================
--- mmotm-2.6.27+.orig/mm/mlock.c
+++ mmotm-2.6.27+/mm/mlock.c
@@ -66,14 +66,9 @@ void __clear_page_mlock(struct page *pag
 		putback_lru_page(page);
 	} else {
 		/*
-		 * Page not on the LRU yet.  Flush all pagevecs and retry.
+		 * Page not on the LRU yet.
+		 * pagevec will handle this in proper way.
 		 */
-		lru_add_drain_all();
-		if (!isolate_lru_page(page))
-			putback_lru_page(page);
-		else if (PageUnevictable(page))
-			count_vm_event(UNEVICTABLE_PGSTRANDED);
-
 	}
 }
 
@@ -187,8 +182,6 @@ static long __mlock_vma_pages_range(stru
 	if (vma->vm_flags & VM_WRITE)
 		gup_flags |= GUP_FLAGS_WRITE;
 
-	lru_add_drain_all();	/* push cached pages to LRU */
-
 	while (nr_pages > 0) {
 		int i;
 
@@ -251,8 +244,6 @@ static long __mlock_vma_pages_range(stru
 		ret = 0;
 	}
 
-	lru_add_drain_all();	/* to update stats */
-
 	return ret;	/* count entire vma as locked_vm */
 }
 
Index: mmotm-2.6.27+/mm/swap.c
===================================================================
--- mmotm-2.6.27+.orig/mm/swap.c
+++ mmotm-2.6.27+/mm/swap.c
@@ -200,10 +200,19 @@ void __lru_cache_add(struct page *page, 
 {
 	struct pagevec *pvec = &get_cpu_var(lru_add_pvecs)[lru];
 
-	page_cache_get(page);
-	if (!pagevec_add(pvec, page))
-		____pagevec_lru_add(pvec, lru);
-	put_cpu_var(lru_add_pvecs);
+	if (likely(lru != LRU_UNEVICTABLE)) {
+		page_cache_get(page);
+		if (!pagevec_add(pvec, page))
+			____pagevec_lru_add(pvec, lru);
+		put_cpu_var(lru_add_pvecs);
+	} else {
+		/*
+		 * A page put into Unevictable List has no chance to come back
+  		 * to other LRU.(it can be unlocked while in pagevec.)
+  		 * We do add_to_lru in synchrous way.
+  		 */
+		add_page_to_unevictable_list(page);
+	}
 }
 
 /**

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/