On Wed, 28 Jun 2006 13:38:51 +0400
Evgeniy Dushistov <dushistov@xxxxxxx> wrote:
> +struct page *ufs_get_locked_page(struct address_space *mapping,
> + pgoff_t index)
> +{
> + struct page *page;
> +
> +try_again:
> + page = find_lock_page(mapping, index);
> + if (!page) {
> + page = read_cache_page(mapping, index,
> + (filler_t*)mapping->a_ops->readpage,
> + NULL);
> + if (IS_ERR(page)) {
> + printk(KERN_ERR "ufs_change_blocknr: "
> + "read_cache_page error: ino %lu, index: %lu\n",
> + mapping->host->i_ino, index);
> + goto out;
> + }
> +
> + lock_page(page);
> +
> + if (!PageUptodate(page) || PageError(page)) {
> + unlock_page(page);
> + page_cache_release(page);
> +
> + printk(KERN_ERR "ufs_change_blocknr: "
> + "can not read page: ino %lu, index: %lu\n",
> + mapping->host->i_ino, index);
> +
> + page = ERR_PTR(-EIO);
> + goto out;
> + }
> + }
> +
> + if (unlikely(!page->mapping || !page_has_buffers(page))) {
> + unlock_page(page);
> + page_cache_release(page);
> + goto try_again;/*we really need these buffers*/
> + }
> +out:
> + return page;
> +}
I think there's a (preexisting) problem here. When one thread is executing
ufs_get_locked_page() while a second thread is running truncate().
If truncate got to the page first, truncate_complete_page() will mark the
page !uptodate and will later unlock it. Now this function gets the page
lock and emits a printk (bad) and assumes -EIO (worse).
That scenario might not be possible because of i_mutex coverage, dunno.
But if it _is_ possible, it can be simply fixed by doing
lock_page(page);
+ if (page->mapping == NULL) {
+ /* truncate() got there first */
+ page_cache_release(page);
+ goto try_again;
+ }
That's if it is appropriate to re-instantiate the page at a place which is
now outside i_size...
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
