* find_get_swapcache_page() question
@ 2001-08-28 0:27 Marcelo Tosatti
2001-08-28 11:27 ` Hugh Dickins
0 siblings, 1 reply; 4+ messages in thread
From: Marcelo Tosatti @ 2001-08-28 0:27 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Hugh Dickins, lkml
Ingo,
Looking at find_get_swapcache_page(), I can't see _how_ we can find a
page on the swapper pagecache table that is not a swapcache page.
How that can happen ?
spin_lock(&pagecache_lock);
page = __find_page_nolock(mapping, offset, *hash);
if (page) {
spin_lock(&pagemap_lru_lock);
if (PageSwapCache(page))
page_cache_get(page);
else
page = NULL; <-----
spin_unlock(&pagemap_lru_lock);
}
spin_unlock(&pagecache_lock);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: find_get_swapcache_page() question
2001-08-28 0:27 find_get_swapcache_page() question Marcelo Tosatti
@ 2001-08-28 11:27 ` Hugh Dickins
2001-08-28 15:45 ` Rik van Riel
0 siblings, 1 reply; 4+ messages in thread
From: Hugh Dickins @ 2001-08-28 11:27 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Ingo Molnar, lkml
On Mon, 27 Aug 2001, Marcelo Tosatti wrote:
>
> Looking at find_get_swapcache_page(), I can't see _how_ we can find a
> page on the swapper pagecache table that is not a swapcache page.
I had a look at this a couple of weeks back. If I remember rightly,
the page found by __find_page_nolock() cannot be other than a swapcache
page, and will always have PageSwapCache bit set... until pagecache_lock
is dropped on return to its only caller lookup_swap_cache(). In which the
if (!PageSwapCache(found))
BUG();
if (found->mapping != &swapper_space)
BUG();
are not safe, since there may a concurrent remove_from_swap_cache(),
either from try_to_unuse() or from Rik's new vm_swap_full() deletion.
Those tests would be safe if the page were locked, but it's not.
I say find_get_swapcache_page() serves no purpose, should be deleted,
and find_get_page() used instead. That was one of various things in
the swapoff patch I posted to linux-mm on 16 Aug, which I need to
finish off, cut into pieces and submit to Linus.
Hugh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: find_get_swapcache_page() question
2001-08-28 11:27 ` Hugh Dickins
@ 2001-08-28 15:45 ` Rik van Riel
2001-08-28 16:27 ` Hugh Dickins
0 siblings, 1 reply; 4+ messages in thread
From: Rik van Riel @ 2001-08-28 15:45 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Marcelo Tosatti, Ingo Molnar, lkml
On Tue, 28 Aug 2001, Hugh Dickins wrote:
> if (!PageSwapCache(found))
> BUG();
> if (found->mapping != &swapper_space)
> BUG();
> are not safe, since there may a concurrent remove_from_swap_cache(),
> either from try_to_unuse() or from Rik's new vm_swap_full() deletion.
> Those tests would be safe if the page were locked, but it's not.
vm_swap_full() is called with the page locked,
remove_from_swap_cache() also seems to want the
page locked...
Rik
--
IA64: a worthy successor to i860.
http://www.surriel.com/ http://distro.conectiva.com/
Send all your spam to aardvark@nl.linux.org (spam digging piggy)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: find_get_swapcache_page() question
2001-08-28 15:45 ` Rik van Riel
@ 2001-08-28 16:27 ` Hugh Dickins
0 siblings, 0 replies; 4+ messages in thread
From: Hugh Dickins @ 2001-08-28 16:27 UTC (permalink / raw)
To: Rik van Riel; +Cc: Marcelo Tosatti, Ingo Molnar, lkml
On Tue, 28 Aug 2001, Rik van Riel wrote:
> On Tue, 28 Aug 2001, Hugh Dickins wrote:
>
> > if (!PageSwapCache(found))
> > BUG();
> > if (found->mapping != &swapper_space)
> > BUG();
> > are not safe, since there may a concurrent remove_from_swap_cache(),
> > either from try_to_unuse() or from Rik's new vm_swap_full() deletion.
> > Those tests would be safe if the page were locked, but it's not.
>
> vm_swap_full() is called with the page locked,
> remove_from_swap_cache() also seems to want the
> page locked...
Certainly (I seem to recall composing a comment to that effect
recently :-)! Sorry for being unclear, I meant that those tests
in lookup_swap_cache() are made without the page being locked
by lookup_swap_cache() or its caller (and I'm not proposing
that the page should be locked, but the unsafe tests removed).
Hugh
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2001-08-28 16:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-08-28 0:27 find_get_swapcache_page() question Marcelo Tosatti
2001-08-28 11:27 ` Hugh Dickins
2001-08-28 15:45 ` Rik van Riel
2001-08-28 16:27 ` Hugh Dickins
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).