Skip to contentSkip to conversation

block_truncate_page fix

PATCH In review Andrew Morton 1 participant 1 message May 26, 2002 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 2/18 block_truncate_page fix

Fix bug in block_truncate_page().

When buffers are attached to an uptodate page, they are marked as being uptodate. To preserve buffer/page state coherency. Dirtiness is handled in the same way.

But block_truncate_page() assumes that a buffer which is unmapped and uptodate is over a hole. That's not the case, and the net effect is that block_truncate_page() is failing to zero the block outside the truncation point.

This only happens if the page has a disk mapping but has no attached buffers on entry to block_truncate_page(). That's never the case in current kernels, so the problem does not exhibit (it _does_ exhibit with direct-to-BIO bypass-the-buffers I/O).

There are actually three possible states of buffer mappedness:

- Buffer has a disk mapping (buffer_mapped(bh) == true)

- buffer is over a hole (buffer_mapped(bh) == false)

- don't know. Need to run get_block() (buffer_mapped(bh) == false)

This ambiguity could be resolved by added another buffer state bit (BH_mapping_state_known?) but given that we already elide the get_block calls for the common case (buffer outside i_size) it is unlikely that the complexity is worthwhile.

=====================================

fs/buffer.c renamed from fs/buffer.c~block_truncate_page +3 -4
@@ -2079,11 +2079,10 @@ int block_truncate_page(struct address_s
 
 	err = 0;
 	if (!buffer_mapped(bh)) {
-		/* Hole? Nothing to do */
-		if (buffer_uptodate(bh))
+		err = get_block(inode, iblock, bh, 0);
+		if (err)
 			goto unlock;
-		get_block(inode, iblock, bh, 0);
-		/* Still unmapped? Nothing to do */
+		/* unmapped? It's a hole - nothing to do */
 		if (!buffer_mapped(bh))
 			goto unlock;
 	}
fs/ext3/inode.c renamed from fs/ext3/inode.c~block_truncate_page +1 -4
@@ -1408,11 +1408,8 @@ static int ext3_block_truncate_page(hand
 
 	err = 0;
 	if (!buffer_mapped(bh)) {
-		/* Hole? Nothing to do */
-		if (buffer_uptodate(bh))
-			goto unlock;
 		ext3_get_block(inode, iblock, bh, 0);
-		/* Still unmapped? Nothing to do */
+		/* unmapped? It's a hole - nothing to do */
 		if (!buffer_mapped(bh))
 			goto unlock;
 	}

-