Re: [patch 03/11] mm: shmem: do not try to uncharge known swapcachepages

[Hugh Dickins]

On Tue, 10 Jul 2012, Michal Hocko wrote:
> On Mon 09-07-12 13:37:39, Hugh Dickins wrote:
> > On Mon, 9 Jul 2012, Michal Hocko wrote:
> > > 
> > > Maybe I am missing something but who does the uncharge from:
> > > shmem_unuse
> > >   mem_cgroup_cache_charge
> > >   shmem_unuse_inode
> > >     shmem_add_to_page_cache
> > 
> > There isn't any special uncharge for shmem_unuse(): once the swapcache
> > page is matched up with its memcg, it will get uncharged by one of the
> > usual routes to swapcache_free() when the page is freed: maybe in the
> > call from __remove_mapping(), maybe when free_page_and_swap_cache()
> > ends up calling it.
> > 
> > Perhaps you're worrying about error (or unfound) paths in shmem_unuse()?
> 
> Yes that was exactly my concern.
> 
> > By the time we make the charge, we know for sure that it's a shmem page,
> > and make the charge appropriately; in racy cases it might get uncharged
> > again in the delete_from_swap_cache().  Can the unfound case occur these
> > days?  
> 
> I cannot find a change that would prevent from that.

Yes.

> 
> > I'd have to think more deeply to answer that, but the charge will
> > not go missing.

Yes, the unfound case certainly can still occur these days.  It's very
similar to the race with truncation/eviction which shmem_unuse_inode()
already allows for (-ENOENT from shmem_add_to_page_cache()).  In that
"error" case, the swap entry got removed after we found it in the
file's radix tree, before we get to replace it there.  Whereas in the
"unfound" case, the swap entry got removed from the file's radix tree
before we even found it there, so we haven't a clue which file it ever
belonged to.

But it doesn't matter.  We have charged the memcg (the original memcg if
memsw is enabled, or swapoff's own if memsw is disabled), and the charge
is redundant now that the page has been truncated; but it's a common
occurrence with swapcache (most common while PageWriteback or PageLocked)
that the swap and charge cannot be released immediately, and it sorts
itself out under pressure once the page reaches the bottom of the
inactive anon and __remove_mapping()'s swapcache_free().

The worst of it is misleading stats meanwhile; but SwapCache has
always been tiresome that way (duplicated in memory and on swap).

The crucial change with regard to unfound entries was back in 2.6.33,
when we added SWAP_MAP_SHMEM: prior to that, we didn't know in advance
if the swap belonged to shmem or to task, and had to be more careful
about when we charge.

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