2003-05-13 21:46:29

by Paul E. McKenney

Subject: [RFC][PATCH] 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

diff -urN -X dontdiff linux-2.5.69/mm/memory.c linux-2.5.69.vmtruncate/mm/memory.c
--- linux-2.5.69/mm/memory.c Sun May 4 16:53:14 2003
+++ linux-2.5.69.vmtruncate/mm/memory.c Fri May 9 17:29:02 2003
@@ -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;

2003-05-13 21:53:49

by William Lee Irwin III

Subject: Re: [RFC][PATCH] Fix for latent bug in vmtruncate()

On Tue, May 13, 2003 at 01:58:07PM -0700, Paul E. McKenney wrote:
> 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