Messages in this thread |  | | Date | Sun, 29 Oct 2000 11:12:15 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: page->mapping == 0 |
| |
On Sun, 29 Oct 2000, Alexander Viro wrote: > > One possible way is to access page->mapping _only_ under the page lock > and in cases when we call ->i_mapping->a_ops->foo check the ->mapping > before the method call.
I'm leaning towards this for a 2.4.x solution.
As far as I can tell, page->mapping is _already_ only accessed and modified with the page lock held. It's just that we don't test it for NULL, in case an earlier lock holder decided to clear it.
(No, I didn't look through all the users, but at least conceptually it _should_ be true that we only look at "mapping" with the lock held: it's mainly used for pagein, and pageout, buth with the lock held for other reasons already. Certainly all the places where we have had bug-reports have been of this type).
Making it policy that we have to re-test page->mapping after aquireing the page lock might be the simplest fix for 2.4.x. It still means that we might end up allowing people to have a "bad" page in the VM space due to the "test->insert" race condition, but it woul dmake that event pretty much a harmless bug (and thus move it to the "beauty wart - to be fixed later" category).
And the places where we get the page lock and use page->mapping are not that many, I think.
(And notice how we actually _have_ this approach already in do_buffer_fdatasync(), for similar reasons - we use the "re-test the page->buffers" thing there. Of course, there we do it because the clearing of page->buffers is easier to see, and can happen as a result of memory pressure, and not just truncate()).
So, for example, in __find_lock_page() we should re-test the mapping after we aquired the page lock. Which is fairly easy, just add something like
/* Race: did the mapping go away before we got the page lock? */ if (page && page->mapping != mapping) { page_cache_release(page); goto repeat; }
to the end of __find_lock_page(). Add similar logic to do_generic_file_read(), filemap_nopage() and filemap_sync_pte() and read_cache_page(), and you're pretty much done.
Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |