On Tue, Aug 01, 2006 at 12:39:58AM -0700, Andrew Morton wrote: > I'm not sure that the `goto repeat' is needed if truncate got there first, > really - if truncate took the page down then it's now outside i_size and > shouldn't be coming back. > > If the page _can_ come back then this code is all rather problematic. > Because this means that the page can come back (via an extending write()) > one nanosecond after ufs_get_locked_page() returns NULL. Won't the callers > of ufs_get_locked_page() get confused by that? ufs_get_locked_page is called twice in ufs code, one time in ufs_truncate path(we allocated last block), and another time when fragments are reallocated. In ideal world in the second case on allocation/free block layer we should not know that things like `truncate' exists, but now with such crutch like ufs_get_locked_page we can (or should?) skip truncated pages. Signed-off-by: Evgeniy Dushistov <dushistov@xxxxxxx> --- Index: linux-2.6.18-rc2-mm1/fs/ufs/balloc.c =================================================================== --- linux-2.6.18-rc2-mm1.orig/fs/ufs/balloc.c +++ linux-2.6.18-rc2-mm1/fs/ufs/balloc.c @@ -248,7 +248,7 @@ static void ufs_change_blocknr(struct in if (likely(cur_index != index)) { page = ufs_get_locked_page(mapping, index); - if (IS_ERR(page)) + if (!page || IS_ERR(page)) /* it was truncated or EIO */ continue; } else page = locked_page; Index: linux-2.6.18-rc2-mm1/fs/ufs/util.c =================================================================== --- linux-2.6.18-rc2-mm1.orig/fs/ufs/util.c +++ linux-2.6.18-rc2-mm1/fs/ufs/util.c @@ -251,7 +251,6 @@ struct page *ufs_get_locked_page(struct { struct page *page; -try_again: page = find_lock_page(mapping, index); if (!page) { page = read_cache_page(mapping, index, @@ -271,7 +270,8 @@ try_again: /* Truncate got there first */ unlock_page(page); page_cache_release(page); - goto try_again; + page = NULL; + goto out; } if (!PageUptodate(page) || PageError(page)) { -- /Evgeniy - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html