[lkml]   [2002]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] pagecache lock ordering
On Wed, 9 Jan 2002, Andrew Morton wrote:
> Hugh Dickins wrote:
> >
> > There's two places, do_buffer_fdatasync
> generic_buffer_fdatasync() and hence do_buffer_fdatasync()
> are completely unused. It may be simpler to just trash
> them.

Oh, nice observation. writeout_one_page could be trashed at
the same time (but not waitfor_one_page, still in use elsewhere).

But what's the chance that an out-of-tree filesystem might
be using generic_buffer_fdatasync, which is still prototyped
and exported? Remove from 2.5 but leave in 2.4? I'd like to
see them go completely - revenge for being misled by them -
but Marcelo may decide otherwise.

> > and __find_lock_page_helper,
> Yeah. The code can't deadlock because:
> page_cache_get();
===> spin_unlock(&pagecache_lock);
===> lock_page();
> spin_lock(&pagecache_lock);
> page_cache_release();
> we implicitly *know* that page_cache_release won't try
> to acquire pagemap_lru_lock, because the page is in the
> pagecache and has count=2 or more. Which is a bit, umm,
> subtle.

I'm not entirely convinced (you'll agree that the argument
depends on rather more than you've stated there), but today
I cannot support the counter-example I thought I had (a page
already "on its way out": but we're more careful to hold both
pagecache_lock and page lock when checking page count before
removing from inode cache than I remembered).

(Hmm, but if it's all thoroughly protected, why do we even bother
to recheck mapping and index same there? Perhaps the shmem case.)

I was also overlooking that the lru_cache_del call was added into
page_cache_release to cope with the _anonymous_ pages Linus put
on LRU. Cached pages were and are deleted from LRU before getting
there, and __find_lock_page_helper applies to cached not anon pages.

So I agree, both parts of my patch are unnecessary:
harmless, but no justification for applying it.

> I get the feeling that a lot of this would be cleaned up
> if presence on an LRU contributed to page->count. It
> seems strange, kludgy and probably racy that this is not
> the case.

That makes a lot of sense: but I feel much safer in agreeing
with you than in making the corresponding changes!

Many thanks for looking it over,

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-03-22 13:15    [W:0.077 / U:4.664 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site