[linux-mm-cc] Re: CCache problem
Nitin Gupta
nitingupta.mail at gmail.com
Wed Jun 28 05:31:39 EDT 2006
Rik van Riel wrote:
> On Tue, 27 Jun 2006, Nitin Gupta wrote:
>> Problem 2: These allocation can sleep (esp. under mem pressure).
>> If find_get_page() cannot sleep then how can we do decompression?
>
> After you allocate a fresh page, re-take the lock.
>
> It is possible somebody else also allocated a page simultaneously,
> and hooked that page into the the page cache before you. In that
> case, free your page and use the other guy's page :)
>
First, I forgot to unlock chunk_head :)
WLQ();
*slot = new_page;
page_cache_get(new_page);
+ unlock_chunk_head(ch);
/* free chunk_head if only we are the user */
if (put_page_testzero(ch)) kfree(ch);
WUQ();
Now, I have doubts in what you suggested:
If 100 people come at find_get_page() (and other lookup functions), only one of
them will get lock on chunk_head and get to point where we alloc_page and
kmalloc(64kb). Rest of 99 cannot go beyond wait_on_chunk_head() until that one
guy frees lock on chunk_head. So, there's no one else who can alloc the page
simultaneously.
So, the problem still remains: The allocation done by this one guy can sleep
while find_get_page() cannot.
If allocation sleeps it will cause another problem: chunk_head cannot be
unlocked till we decompress the page and then hook it into page cache. And we
have taken spinlock (from prob 1) to wait for decompression to finish.
>> Do you think there are problems in above code?
>
> You do not check *slot after you re-take the lock.
>
> Remember that you dropped the lock, so anything could have
> happened. Better start from the beginning, maybe using a
> goto?
>
> The function do_generic_mapping_read() in mm/filemap.c
> is a nice example of all the things that could go wrong,
> and how easy it is to deal with those by simply starting
> your lookup from the beginning again :)
>
...
RUQ();
wait_on_chunk_head(ch);
RLQ();
Now, I think, only foll. cases can occur:
1. Radix node corresponding to slot has disappeared. In this case
find_get_page() should return NULL. Doing *slot in this case is an error.
2. Page has been freed/truncated. In this case *slot will be NULL and this is
what we return.
3. *slot != NULL. Here, just return this *slot (as in case 2).
Currently, I'm not making sure if radix node for slot itself has disappeared or
not. This, I think, can be done simply by:
...
RUQ();
wait_on_chunk_head(ch);
RLQ();
/* slot now has decompressed page
* return slot (it may be NULL) */
- page = *slot;
+ page = radix_tree_lookup(&mapping->page_tree, offset);
if (page) page_cache_get(page);
/* free chunk_head if only we are the user */
if (put_page_testzero(ch)) kfree(ch);
RUQ();
return page;
...
Using goto to beginning (just after very first RLQ) should also do.
-- Nitin
More information about the linux-mm-cc
mailing list