From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH]: ufs: truncate should allocate block for last byte
Date: Wed, 28 Jun 2006 04:50:29 -0700
Message-ID: <20060628045029.bc10d333.akpm@osdl.org>
References: <20060628093851.GA1719@rain.homenetwork>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from smtp.osdl.org ([65.172.181.4]:35786 "EHLO smtp.osdl.org")
	by vger.kernel.org with ESMTP id S932784AbWF1Lul (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Wed, 28 Jun 2006 07:50:41 -0400
To: Evgeniy Dushistov <dushistov@mail.ru>
In-Reply-To: <20060628093851.GA1719@rain.homenetwork>
Sender: linux-fsdevel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org

On Wed, 28 Jun 2006 13:38:51 +0400
Evgeniy Dushistov <dushistov@mail.ru> 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...