Re: Filling holes in ext2

[Andrew Morton]

Adrian Cox wrote:
> 
> Andrew Morton wrote:
> 
> > generic_file_write() will mark the page not up-to-date in this case.
> > I wonder what's actually going on?  Perhaps the fact that we've
> > instantiated a block in ext2 outside i_size?
> 
> The problem is that the on-disk metadata now says that that disk block
> is part of the file. So as the page is not up-to-date, the next read
> operation will go to the disk and fetch that block of garbage into the
> page cache.
> 

Ah.  Now I'm with you.  Yes, we need a better cleanup facility
to handle this.

We can sort-of fudge it with commit_write():

--- linux-2.4.8-ac9/mm/filemap.c	Wed Aug 22 10:57:47 2001
+++ ac/mm/filemap.c	Thu Aug 23 12:33:50 2001
@@ -2674,8 +2674,9 @@ generic_file_write(struct file *file,con
 		status = __copy_from_user(kaddr+offset, buf, bytes);
 		flush_dcache_page(page);
 		if (status) {
-			if (mapping->a_ops->abort_write)
-				mapping->a_ops->abort_write(file, page);
+			/* Zero the disk blocks so we don't expose stale data mid-file */
+			memset(kaddr + offset, 0, bytes);
+			mapping->a_ops->commit_write(file, page, offset, offset+bytes);
 			goto fail_write;
 		}
 		status = mapping->a_ops->commit_write(file, page, offset, offset+bytes);
@@ -2720,7 +2721,6 @@ out:
 fail_write:
 	status = -EFAULT;
 	ClearPageUptodate(page);
-	kunmap(page);
 	goto unlock;
 sync_failure:
 	UnlockPage(page);

Which is OK for mid-file blocks, but will cause i_size to be extended
at eof, which probably isn't too bad.  Needs more thought.

-