lkml.org 
[lkml]   [2011]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRE: [PATCH 1/14] mm: invalidate_mapping_pages flush cleancache
On Tue, 31 May 2011, Chris Mason wrote:
> Excerpts from Hugh Dickins's message of 2011-05-31 13:05:27 -0400:
> > On Tue, 31 May 2011, Dan Magenheimer wrote:
> > > >
> > > > truncate_inode_pages_range() and invalidate_inode_pages2_range()
> > > > call cleancache_flush_inode(mapping) before and after: shouldn't
> > > > invalidate_mapping_pages() be doing the same?
> > >
> > > I don't claim to be an expert on VFS, and so I have cc'ed
> > > Chris Mason who originally placed the cleancache hooks
> > > in VFS, but I think this patch is unnecessary. Instead
> > > of flushing ALL of the cleancache pages belonging to
> > > the inode with cleancache_flush_inode, the existing code
> > > eventually calls __delete_from_page_cache on EACH page
> > > that is being invalidated.
> >
> > On each one that's in pagecache (and satisfies the other "can we
> > do it easily?" conditions peculiar to invalidate_mapping_pages()).
> > But there may be other slots in the range that don't reach
> > __delete_from_page_cache() e.g. because not currently in pagecache,
> > but whose cleancache ought to be flushed. I think that's what a
> > caller of invalidate_mapping_pages(), e.g. drop caches, expects.
>
> We call invalidate_mapping_pages from prune_icache, so if we drop the
> cleancache there we lose the cache entries any time the inode is dropped
> from ram.

I hadn't noticed that use of invalidate_mapping_pages(). Right,
I can understand that you wouldn't want to drop the cleancache there.

I was more conscious of the dispose_list() at the end of prune_icache(),
which would call truncate_inode_pages(), which would flush cleancache.

Ah, but inode only gets on the freeable list if can_unuse(inode),
and one of the reasons to retain the inode is if nrpages is non-0.

All rather odd, and what it adds up to, I think, is that if that
invalidate_mapping_pages() succeeds in removing all the pages from
the page cache (but leaving some in cleancache), then the inode may
advance to truncate_inode_pages(), and meet cleancache_flush_inode()
there. (It happens to be called before the mapping->nrpages test.)

All rather odd, both the pruning decisions and the cleancache decisions.

>
> Is there a specific case you're thinking of where we want to drop the
> cleancache but don't have the pages?

The case I was thinking of, where I'd met invalidate_mapping_pages()
before, is fs/drop_caches.c ... which is about, er, dropping caches.

(But in general, why would you care to keep the cleancache when you
do have the pages? I thought its use was for the pages we don't have.)

>
> O_DIRECT perhaps?

Hadn't given it a thought. But now you mention it, yes,
that one looks like a worry too.

Hugh


\
 
 \ /
  Last update: 2011-06-01 00:03    [W:0.098 / U:0.576 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site