Re: [mmtom] please drop memcg-handle-swap-cache set (memcg handle swap cache rework).

[Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>]

> > > @@ -398,12 +395,27 @@ static void migrate_page_copy(struct pag
> > >   	}
> > >  
> > >  #ifdef CONFIG_SWAP
> > > -	ClearPageSwapCache(page);
> > > +	if (PageSwapCache(page)) {
> > > +		/*
> > > +		 * SwapCache is removed implicitly. To uncharge SwapCache,
> > > +		 * SwapCache flag should be cleared.
> > > +		 */
> > > +		ClearPageSwapCache(page);
> > > +		mem_cgroup_uncharge_page(page);
> > > +	}
> > >  #endif
> > >  	ClearPageActive(page);
> > >  	ClearPagePrivate(page);
> > >  	set_page_private(page, 0);
> > > -	page->mapping = NULL;
> > > +
> > > +	if (!PageAnon(page)) {
> > > +		/*
> > > +		 * This page was removed from radix-tree implicitly.
> > > +		 */
> > > +		page->mapping = NULL;
> > > +		mem_cgroup_uncharge_cache_page(page);
> > > +	} else
> > > +		page->mapping = NULL;
> > >  
> > 
> > page->mapping will be cleared anyway, so I prefer:
> > 
> > ===
> > 	page->mapping = NULL;
> > 
> > 	if (!PageAnon(page))
> > 		/*
> > 		 * This page was removed from radix-tree implicitly.
> > 		 */
> > 		mem_cgroup_uncharge_cache_page(page);
> > ===
> > 
> Ah.., PageAnon(page) check (page->mapping & 0x1) so, we cant do this.
> 
Ooops! Sorry for saying stupid thing.

> Hmm, like this ?
> ==
> anon = PageAnon(page);
> page->mapping = NULL;
> if (anon)
> 	mem_cgroup_....
> ==
> 
Looks good.
But I don't have any objection to your original code.
I just thought 2 lines of "page->mapping = NULL" was verbose.


Thanks,
Daisuke Nishimura.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>