Re: 2.4.0-test10-pre3:Oops in mm/filemap.c:filemap_write_pa

From: Linus Torvalds (torvalds@transmeta.com)
Date: Mon Oct 23 2000 - 16:24:04 EST


On Mon, 23 Oct 2000, Alexander Viro wrote:
>
> That's fine, but I'm afraid that we'll need a bit more than that. A couple of
> obvious ones:
> * filemap_nopage() needs the second check for ->i_size. Upon exit.

Nope, that just makes the race window smaller. We should check for i_size
after we've gotten the page table lock and just before actually entering
the page into the page tables. Otherwise we'll still race on SMP (a _very_
hard window to get into, admittedly).

> Moreover, what is (area->vm_mm == current->mm) doing in the existing check?

It's for ptrace. You can do ugly things with ptrace that aren't possible
for the process itself.

> * truncate() should zero the page out if it doesn't remove it from
> cache.

So how about this truncate_complete_page() implementation:

        /*
         * Try to get rid of a page.. Clear it if it fails
         * for some reason. The page must be locked upon calling
         * this function.
         *
         * We remove the page from the page cache _after_ we have
         * destroyed all buffer-cache references to it. Otherwise some
         * other process might think this inode page is not in the
         * page cache and creates a buffer-cache alias to it causing
         * all sorts of fun problems ...
         */
        static inline void truncate_complete_page(struct page *page)
        {
                /* Try to get rid of buffers */
                if (page->buffers)
                        block_flushpage(page, 0);
        
                spin_lock(&pagecache_lock);
                spin_lock(&pagemap_lru_lock);
        
                if (page_count(page) != 1) {
                        memclear_highpage_flush(page, 0, PAGE_CACHE_SIZE);
                } else {
                        ClearPageDirty(page);
                        __lru_cache_del(page);
                        __remove_inode_page(page);
                        page_cache_release(page);
                }
                spin_unlock(&pagemap_lru_lock);
                spin_unlock(&pagecache_lock);
        }

we should probably special-case the "block_flushpage()" failed case, but
the above should do reasonable things with it (because page_count() will
be > 1 due to buffers).

The above is obviously completely and utterly untested. Petr? Willing to
give this a go?

                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/



This archive was generated by hypermail 2b29 : Mon Oct 23 2000 - 21:00:21 EST