Re: [BUG] infinite loop in find_get_pages()

[Hugh Dickins]

On Thu, 15 Sep 2011, Shaohua Li wrote:
> On Thu, 2011-09-15 at 06:37 +0800, Linus Torvalds wrote:
> > On Wed, Sep 14, 2011 at 2:53 PM, Hugh Dickins <hughd@xxxxxxxxxx> wrote:
> > >
> > > Thanks, Eric, though it may not be worth spending your time on it.
> > > It occurred to me over lunch that it may take painfully longer than
> > > expected to invalidate_mapping_pages() on a single-swapped-out-page
> > > 1TB sparse tmpfs file - all those "start += 1" restarts until it
> > > reaches the end.
> > 
> > So can we have a stop-gap patch to just fixes it for now? I assume
> > that would be Shaohua's patch with the "nr_found > nr_skip" change?
> > 
> > Can you guys send whatever patch is appropriate for now with a nice
> > changelog and the appropriate sign-offs, please? So that we can at
> > least close the issue...
> here is my patch if you want to close the issue at hand.

Right, it closes one of the hangs, but not whatever the 3.0 hang is,
and not the unlikely SHM_UNLOCK issue I factored in.  I cannot consider
those issues closed, but I am happy to be let off the hook of providing
another fix tomorrow - thanks!

> 
> Subject: mm: account skipped entries to avoid looping in find_get_pages
> 
> The found entries by find_get_pages() could be all swap entries. In
> this case we skip the entries, but make sure the skipped entries are
> accounted, so we don't keep looping.
> Using nr_found > nr_skip to simplify code as suggested by Eric.
> 
> Reported-and-tested-by: Eric Dumazet <eric.dumazet@xxxxxxxxx>
> Signed-off-by: Shaohua Li <shaohua.li@xxxxxxxxx>

Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>

> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 645a080..7771871 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -827,13 +827,14 @@ unsigned find_get_pages(struct address_space *mapping, pgoff_t start,
>  {
>  	unsigned int i;
>  	unsigned int ret;
> -	unsigned int nr_found;
> +	unsigned int nr_found, nr_skip;
>  
>  	rcu_read_lock();
>  restart:
>  	nr_found = radix_tree_gang_lookup_slot(&mapping->page_tree,
>  				(void ***)pages, NULL, start, nr_pages);
>  	ret = 0;
> +	nr_skip = 0;
>  	for (i = 0; i < nr_found; i++) {
>  		struct page *page;
>  repeat:
> @@ -856,6 +857,7 @@ repeat:
>  			 * here as an exceptional entry: so skip over it -
>  			 * we only reach this from invalidate_mapping_pages().
>  			 */
> +			nr_skip++;
>  			continue;
>  		}
>  
> @@ -876,7 +878,7 @@ repeat:
>  	 * If all entries were removed before we could secure them,
>  	 * try again, because callers stop trying once 0 is returned.
>  	 */
> -	if (unlikely(!ret && nr_found))
> +	if (unlikely(!ret && nr_found > nr_skip))
>  		goto restart;
>  	rcu_read_unlock();
>  	return ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/