diff options
| author | Hugh Dickins <hughd@google.com> | 2011-03-22 16:33:06 -0700 |
|---|---|---|
| committer | Linus Torvalds <torvalds@linux-foundation.org> | 2011-03-22 17:44:04 -0700 |
| commit | 9d8aa4ea855e0d64bb6926acb5618e6d1e2ed344 (patch) | |
| tree | a21357f21084169e0a322f8fa8ce8abe343d4f06 /mm/filemap.c | |
| parent | c033a93c0d961fc7ec5b0872649143e061d97dd4 (diff) | |
| download | linux-9d8aa4ea855e0d64bb6926acb5618e6d1e2ed344.tar.gz linux-9d8aa4ea855e0d64bb6926acb5618e6d1e2ed344.tar.bz2 linux-9d8aa4ea855e0d64bb6926acb5618e6d1e2ed344.zip | |
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>
Diffstat (limited to 'mm/filemap.c')
| -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: page = radix_tree_deref_slot((void **)pages[i]); if (unlikely(!page)) continue; + + /* + * This can only trigger when the entry at index 0 moves out + * of or back to the root: none yet gotten, safe to restart. + */ if (radix_tree_deref_retry(page)) { - if (ret) - start = pages[ret-1]->index; + WARN_ON(start | i); goto restart; } @@ -915,6 +919,11 @@ repeat: page = radix_tree_deref_slot((void **)pages[i]); if (unlikely(!page)) continue; + + /* + * This can only trigger when the entry at index 0 moves out + * of or back to the root: none yet gotten, safe to restart. + */ if (radix_tree_deref_retry(page)) goto restart; @@ -975,6 +984,11 @@ repeat: page = radix_tree_deref_slot((void **)pages[i]); if (unlikely(!page)) continue; + + /* + * This can only trigger when the entry at index 0 moves out + * of or back to the root: none yet gotten, safe to restart. + */ if (radix_tree_deref_retry(page)) goto restart; |