Skip to content

Commit 9d8aa4e

Browse files
Hugh Dickinstorvalds
authored andcommitted
mm: remove worrying dead code from find_get_pages()
The radix_tree_deref_retry() case in find_get_pages() has a strange little excrescence, not seen in the other gang lookups: it looks like the start of an abandoned attempt to guarantee forward progress in a case that cannot arise. ret should always be 0 here: if it isn't, then going back to restart will leak references to pages already gotten. There used to be a comment saying nr_found is necessarily 1 here: that's not quite true, but the radix_tree_deref_retry() case is peculiar to the entry at index 0, when we race with it being moved out of the radix_tree root or back. Remove the worrisome two lines, add a brief comment here and in find_get_pages_contig() and find_get_pages_tag(), and a WARN_ON in find_get_pages() should it ever be seen elsewhere than at 0. Signed-off-by: Hugh Dickins <[email protected]> Cc: Nick Piggin <[email protected]> Acked-by: Peter Zijlstra <[email protected]> Cc: Wu Fengguang <[email protected]> Cc: Salman Qazi <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent c033a93 commit 9d8aa4e

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

mm/filemap.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,9 +863,13 @@ unsigned find_get_pages(struct address_space *mapping, pgoff_t start,
863863
page = radix_tree_deref_slot((void **)pages[i]);
864864
if (unlikely(!page))
865865
continue;
866+
867+
/*
868+
* This can only trigger when the entry at index 0 moves out
869+
* of or back to the root: none yet gotten, safe to restart.
870+
*/
866871
if (radix_tree_deref_retry(page)) {
867-
if (ret)
868-
start = pages[ret-1]->index;
872+
WARN_ON(start | i);
869873
goto restart;
870874
}
871875

@@ -915,6 +919,11 @@ unsigned find_get_pages_contig(struct address_space *mapping, pgoff_t index,
915919
page = radix_tree_deref_slot((void **)pages[i]);
916920
if (unlikely(!page))
917921
continue;
922+
923+
/*
924+
* This can only trigger when the entry at index 0 moves out
925+
* of or back to the root: none yet gotten, safe to restart.
926+
*/
918927
if (radix_tree_deref_retry(page))
919928
goto restart;
920929

@@ -975,6 +984,11 @@ unsigned find_get_pages_tag(struct address_space *mapping, pgoff_t *index,
975984
page = radix_tree_deref_slot((void **)pages[i]);
976985
if (unlikely(!page))
977986
continue;
987+
988+
/*
989+
* This can only trigger when the entry at index 0 moves out
990+
* of or back to the root: none yet gotten, safe to restart.
991+
*/
978992
if (radix_tree_deref_retry(page))
979993
goto restart;
980994

0 commit comments

Comments
 (0)