On Wed, Jun 15, 2011 at 04:50:20PM -0700, Hugh Dickins wrote: > On Wed, 15 Jun 2011, [email protected] wrote: > > From: Andrea Arcangeli <[email protected]> > > > > swapcache will reach the below code path in migrate_page_move_mapping, > > and swapcache is accounted as NR_FILE_PAGES but it's not accounted as > > NR_SHMEM. > > > > This can make the NR_SHMEM counter underflow. > > > > Signed-off-by: Andrea Arcangeli <[email protected]> > > Reviewed-by: Minchan Kim <[email protected]> > > Acked-by: Mel Gorman <[email protected]> > > Cc: KOSAKI Motohiro <[email protected]> > > Cc: Hugh Dickins <[email protected]> > > Cc: <[email protected]> > > Signed-off-by: Andrew Morton <[email protected]> > > --- > > > > mm/migrate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff -puN mm/migrate.c~mm-migratec-dont-account-swapcache-as-shmem > > mm/migrate.c > > --- a/mm/migrate.c~mm-migratec-dont-account-swapcache-as-shmem > > +++ a/mm/migrate.c > > @@ -288,7 +288,7 @@ static int migrate_page_move_mapping(str > > */ > > __dec_zone_page_state(page, NR_FILE_PAGES); > > __inc_zone_page_state(newpage, NR_FILE_PAGES); > > - if (PageSwapBacked(page)) { > > + if (mapping != &swapper_space && PageSwapBacked(page)) { > > __dec_zone_page_state(page, NR_SHMEM); > > __inc_zone_page_state(newpage, NR_SHMEM); > > } > > _ > > This one breaks the CONFIG_MIGRATION=y CONFIG_SWAP=n build, > because of the reference to swapper_space. > > Noticing that now, I was about to follow up with a patch changing test to > if (PageSwapBacked(page) && !PageSwapCache(page)) { > as Mel had the good instinct to suggest originally. > > But then I read the comment above it which says "Note that anonymous > pages are accounted for via NR_FILE_PAGES and NR_ANON_PAGES if they > are mapped to swap space". So shouldn't we actually be adjusting > NR_ANON_PAGES in that case??
That I already thought of, NR_ANON_PAGES is handled all by rmap, and when we are at that point of migration we know there's not a single pte mapped so it doesn't need refiling to the other zone because it's not accounted as anon. I agree about the build failure. It wasn't obvious to me that swapper_space wouldn't build with CONFIG_SWAP=n, sorry. > If I'm not too late, let's hold back this patch for the moment. Here a version using PageSwapCache to make CONFIG_SWAP=n happy ;) === Subject: migrate: don't account swapcache as shmem From: Andrea Arcangeli <[email protected]> swapcache will reach the below code path in migrate_page_move_mapping, and swapcache is accounted as NR_FILE_PAGES but it's not accounted as NR_SHMEM. Hugh pointed out we must use PageSwapCache instead of comparing mapping to &swapper_space, to avoid build failure with CONFIG_SWAP=n. Signed-off-by: Andrea Arcangeli <[email protected]> --- diff --git a/mm/migrate.c b/mm/migrate.c index e4a5c91..2597a27 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -288,7 +288,7 @@ static int migrate_page_move_mapping(struct address_space *mapping, */ __dec_zone_page_state(page, NR_FILE_PAGES); __inc_zone_page_state(newpage, NR_FILE_PAGES); - if (PageSwapBacked(page)) { + if (!PageSwapCache(page) && PageSwapBacked(page)) { __dec_zone_page_state(page, NR_SHMEM); __inc_zone_page_state(newpage, NR_SHMEM); } _______________________________________________ stable mailing list [email protected] http://linux.kernel.org/mailman/listinfo/stable
