Re: writepage fs corruption fixes

[Andrew Morton]

Andrea Arcangeli <andrea@xxxxxxx> wrote:
>
> +page_is_mapped:
>  +
>  +	end_index = i_size >> PAGE_CACHE_SHIFT;
>   	if (page->index >= end_index) {
>  -		unsigned offset = i_size_read(inode) & (PAGE_CACHE_SIZE - 1);
>  +		unsigned offset = i_size & (PAGE_CACHE_SIZE - 1);
>   		char *kaddr;
>   
>   		if (page->index > end_index || !offset)
>  @@ -503,8 +506,6 @@ mpage_writepage(struct bio *bio, struct 
>   		kunmap_atomic(kaddr, KM_USER0);
>   	}
>   
>  -page_is_mapped:

What's the thinking behind moving the page_is_mapped label here?

We've established that we have found `first_unmapped' number of uptodate
and dirty buffers at the "front" of the page, and we're about to stick
(first_unmapped<<blkbits) bytes of this page into the BIO for writeout. 
Hence everything which will go into the BIO is known to be uptodate and
dirty.  So I'm wondering why this change was made.


The change is correct, though.  It prevents us from writing non-zero data
between i_size and the end of the final bh to the file. 
block_write_full_page() does it too:

	/*
	 * The page straddles i_size.  It must be zeroed out on each and every
	 * writepage invokation because it may be mmapped.  "A file is mapped
	 * in multiples of the page size.  For a file that is not a multiple of
	 * the  page size, the remaining memory is zeroed when mapped, and
	 * writes to that region are not written out to the file."
	 */

(Note that this is a "best effort" thing - userspace could still write
non-zero data into the mmapped page outside i_size even while I/O is in
flight.  Can't do much about that).

But was this the reason for you making this change?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/