On Thu, 16 Jun 2011, Andrea Arcangeli wrote: > 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.
Yes, of course you're right - thanks. Don't let me hold this up any longer (though the comments in the file could be less misleading!). Your patch with my Ack and Cc stable again at the bottom. Hugh > > 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]> Acked-by: Hugh Dickins <[email protected]> Cc: [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
