On Wed, 22 Feb 2012, Konstantin Khlebnikov wrote:
> Hugh Dickins wrote:
> > On Wed, 22 Feb 2012, Konstantin Khlebnikov wrote:
> > > Hugh Dickins wrote:
> > > >
> > > > I'll have to come back to think about your locking later too;
> > > > or maybe that's exactly where I need to look, when investigating
> > > > the mm_inline.h:41 BUG.
> > >
> > > pages_count[] updates looks correct.
> > > This really may be bug in locking, and this VM_BUG_ON catch it before
> > > list-debug.
> >
> > I've still not got into looking at it yet.
> >
> > You're right to mention DEBUG_LIST: I have that on some of the machines,
> > and I would expect that to be the first to catch a mislocking issue.
> >
> > In the past my problems with that BUG (well, the spur to introduce it)
> > came from hugepages.
>
> My patchset hasn't your mem_cgroup_reset_uncharged_to_root protection,
> or something to replace it. So, there exist race between cgroup remove and
> isolated uncharged page put-back, but it shouldn't corrupt lru lists.
> There something different.
Yes, I'm not into removing cgroups yet.
I've got it: your "can differ only on lumpy reclaim" belief, first
commented in 17/22 but then assumed in 20/22, is wrong: those swapin
readahead pages, for example, may shift from root_mem_cgroup to another
mem_cgroup while the page is isolated by shrink_active or shrink_inactive.
Patch below against the top of my version of your tree: probably won't
quite apply to yours, since we used different bases here; but easy
enough to correct yours from it.
Bisection was misleading: it appeared to be much easier to reproduce
with 22/22 taken off, and led to 16/22, but that's because that one
introduced a similar bug, which actually got fixed in 22/22:
relock_page_lruvec() and relock_page_lruvec_irq() in 16/22 onwards
are wrong, in each case the if block needs an
} else
lruvec = page_lruvec(page);
You'll want to fix that in 16/22, but here's the patch for the end state:
Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
but forget that, just quietly fold the fixes into yours!
---
mm/vmscan.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
--- 3033K2.orig/mm/vmscan.c 2012-02-21 00:02:13.000000000 -0800
+++ 3033K2/mm/vmscan.c 2012-02-21 21:23:25.768381375 -0800
@@ -1342,7 +1342,6 @@ static int too_many_isolated(struct zone
*/
static noinline_for_stack struct lruvec *
putback_inactive_pages(struct lruvec *lruvec,
- struct scan_control *sc,
struct list_head *page_list)
{
struct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat;
@@ -1364,11 +1363,8 @@ putback_inactive_pages(struct lruvec *lr
continue;
}
- /* can differ only on lumpy reclaim */
- if (sc->order) {
- lruvec = __relock_page_lruvec(lruvec, page);
- reclaim_stat = &lruvec->reclaim_stat;
- }
+ lruvec = __relock_page_lruvec(lruvec, page);
+ reclaim_stat = &lruvec->reclaim_stat;
SetPageLRU(page);
lru = page_lru(page);
@@ -1566,7 +1562,7 @@ shrink_inactive_list(unsigned long nr_to
__count_vm_events(KSWAPD_STEAL, nr_reclaimed);
__count_zone_vm_events(PGSTEAL, zone, nr_reclaimed);
- lruvec = putback_inactive_pages(lruvec, sc, &page_list);
+ lruvec = putback_inactive_pages(lruvec, &page_list);
__mod_zone_page_state(zone, NR_ISOLATED_ANON, -nr_anon);
__mod_zone_page_state(zone, NR_ISOLATED_FILE, -nr_file);
@@ -1631,7 +1627,6 @@ shrink_inactive_list(unsigned long nr_to
static struct lruvec *
move_active_pages_to_lru(struct lruvec *lruvec,
- struct scan_control *sc,
struct list_head *list,
struct list_head *pages_to_free,
enum lru_list lru)
@@ -1643,10 +1638,7 @@ move_active_pages_to_lru(struct lruvec *
int numpages;
page = lru_to_page(list);
-
- /* can differ only on lumpy reclaim */
- if (sc->order)
- lruvec = __relock_page_lruvec(lruvec, page);
+ lruvec = __relock_page_lruvec(lruvec, page);
VM_BUG_ON(PageLRU(page));
SetPageLRU(page);
@@ -1770,9 +1762,9 @@ static void shrink_active_list(unsigned
*/
reclaim_stat->recent_rotated[file] += nr_rotated;
- lruvec = move_active_pages_to_lru(lruvec, sc, &l_active, &l_hold,
+ lruvec = move_active_pages_to_lru(lruvec, &l_active, &l_hold,
LRU_ACTIVE + file * LRU_FILE);
- lruvec = move_active_pages_to_lru(lruvec, sc, &l_inactive, &l_hold,
+ lruvec = move_active_pages_to_lru(lruvec, &l_inactive, &l_hold,
LRU_BASE + file * LRU_FILE);
__mod_zone_page_state(zone, NR_ISOLATED_ANON + file, -nr_taken);
unlock_lruvec_irq(lruvec);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>