diff options
author | Hugh Dickins <hughd@google.com> | 2011-03-22 19:33:06 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2011-03-22 20:44:04 -0400 |
commit | 9d8aa4ea855e0d64bb6926acb5618e6d1e2ed344 (patch) | |
tree | a21357f21084169e0a322f8fa8ce8abe343d4f06 | |
parent | c033a93c0d961fc7ec5b0872649143e061d97dd4 (diff) |
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 <hughd@google.com>
Cc: Nick Piggin <npiggin@kernel.dk>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Cc: Salman Qazi <sqazi@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | mm/filemap.c | 18 |
1 files changed, 16 insertions, 2 deletions
diff --git a/mm/filemap.c b/mm/filemap.c index cb476e70cf19..a29318147365 100644 --- a/mm/filemap.c +++ b/mm/filemap.c | |||
@@ -863,9 +863,13 @@ repeat: | |||
863 | page = radix_tree_deref_slot((void **)pages[i]); | 863 | page = radix_tree_deref_slot((void **)pages[i]); |
864 | if (unlikely(!page)) | 864 | if (unlikely(!page)) |
865 | continue; | 865 | 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 | */ | ||
866 | if (radix_tree_deref_retry(page)) { | 871 | if (radix_tree_deref_retry(page)) { |
867 | if (ret) | 872 | WARN_ON(start | i); |
868 | start = pages[ret-1]->index; | ||
869 | goto restart; | 873 | goto restart; |
870 | } | 874 | } |
871 | 875 | ||
@@ -915,6 +919,11 @@ repeat: | |||
915 | page = radix_tree_deref_slot((void **)pages[i]); | 919 | page = radix_tree_deref_slot((void **)pages[i]); |
916 | if (unlikely(!page)) | 920 | if (unlikely(!page)) |
917 | continue; | 921 | 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 | */ | ||
918 | if (radix_tree_deref_retry(page)) | 927 | if (radix_tree_deref_retry(page)) |
919 | goto restart; | 928 | goto restart; |
920 | 929 | ||
@@ -975,6 +984,11 @@ repeat: | |||
975 | page = radix_tree_deref_slot((void **)pages[i]); | 984 | page = radix_tree_deref_slot((void **)pages[i]); |
976 | if (unlikely(!page)) | 985 | if (unlikely(!page)) |
977 | continue; | 986 | 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 | */ | ||
978 | if (radix_tree_deref_retry(page)) | 992 | if (radix_tree_deref_retry(page)) |
979 | goto restart; | 993 | goto restart; |
980 | 994 | ||