Skip to contentSkip to conversation

Fix for latent bug in vmtruncate()

RFC PATCH In review Paul E. McKenney 2 participants 2 messages May 13, 2003

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 Fix for latent bug in vmtruncate()

The vmtruncate() function shifts down by PAGE_CACHE_SHIFT, then calls vmtruncate_list(), which deals in terms of PAGE_SHIFT instead. Currently, no harm done, since PAGE_CACHE_SHIFT and PAGE_SHIFT are identical. Some day they might not be, hence this patch.

I also took the liberty of modifying a hand-coded "if" that seems to optimize for files that are not mapped to instead use unlikely().

Thoughts?

Thanx, Paul

mm/memory.c +3 -8
diff -urN -X dontdiff linux-2.5.69/mm/memory.c linux-2.5.69.vmtruncate/mm/memory.c
@@ -1108,17 +1108,12 @@
 	if (inode->i_size < offset)
 		goto do_expand;
 	inode->i_size = offset;
+	pgoff = (offset + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	down(&mapping->i_shared_sem);
-	if (list_empty(&mapping->i_mmap) && list_empty(&mapping->i_mmap_shared))
-		goto out_unlock;
-
-	pgoff = (offset + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
-	if (!list_empty(&mapping->i_mmap))
+	if (unlikely(!list_empty(&mapping->i_mmap)))
 		vmtruncate_list(&mapping->i_mmap, pgoff);
-	if (!list_empty(&mapping->i_mmap_shared))
+	if (unlikely(!list_empty(&mapping->i_mmap_shared)))
 		vmtruncate_list(&mapping->i_mmap_shared, pgoff);
-
-out_unlock:
 	up(&mapping->i_shared_sem);
 	truncate_inode_pages(mapping, offset);
 	goto out_truncate;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw
Quoted 8 lines · Paul E. McKenney

The vmtruncate() function shifts down by PAGE_CACHE_SHIFT, then calls vmtruncate_list(), which deals in terms of PAGE_SHIFT instead. Currently, no harm done, since PAGE_CACHE_SHIFT and PAGE_SHIFT are identical. Some day they might not be, hence this patch.
I also took the liberty of modifying a hand-coded "if" that seems to optimize for files that are not mapped to instead use unlikely().

pgoff describes a file offset in the same units used to map files with (the size of an area covered by a PTE), which is PAGE_SIZE (in mainline; elsewhere it's called MMUPAGE_SIZE and I had to fix this already for my tree). When they differ this would lose the offset into the PAGE_CACHE_SIZE-sized file page; hence, well-spotted.

-- wli