David Chinner wrote:
On Mon, Mar 19, 2007 at 05:37:03PM +1100, Nick Piggin wrote:
David Chinner wrote:
+block_page_mkwrite(struct vm_area_struct *vma, struct page *page,
+ get_block_t get_block)
+{
+ struct inode *inode = vma->vm_file->f_path.dentry->d_inode;
+ unsigned long end;
+ loff_t size;
+ int ret = -EINVAL;
+
+ lock_page(page);
+ size = i_size_read(inode);
+ if ((page->mapping != inode->i_mapping) ||
+ ((page->index << PAGE_CACHE_SHIFT) > size)) {
+ /* page got truncated out from underneath us */
+ goto out_unlock;
+ }
I see your explanation above, but I still don't see why this can't
just follow the conventional if (!page->mapping) check for truncation.
If the test happens to be performed after truncate concurrently
decreases i_size, then the blocks are going to get truncated by the
truncate afterwards anyway.
We have to read the inode size in the normal case so that we know if
the page is at EOF and is a partial page so we don't allocate past EOF in
block_prepare_write(). Hence it seems like a no-brainer to me to check
and error out on a page that we *know* is beyond EOF.
I can drop the check if you see no value in it - I just don't
like the idea of ignoring obvious boundary condition violations...
I would prefer it dropped, to be honest. I can see how the check does
pick up that corner case, however truncate is difficult enough (at
least, it has been an endless source of problems) that we want to keep
everyone else simple and have all the non-trivial stuff in truncate.
--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
