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

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

[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().

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;

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

[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().

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;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

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

[William Lee Irwin III]

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

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

[William Lee Irwin III]

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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>