Messages in this thread | | | Date | Wed, 14 Sep 2011 14:53:52 -0700 (PDT) | From | Hugh Dickins <> | Subject | Re: [BUG] infinite loop in find_get_pages() |
| |
On Wed, 14 Sep 2011, Eric Dumazet wrote: > Le mercredi 14 septembre 2011 à 13:38 -0700, Hugh Dickins a écrit : > > > > I'd like to think about that a little more before finalizing the > > patch below - does it work, and does it look acceptable so far? > > Of course, the mods to truncate.c and vmscan.c are not essential > > parts of this fix, just things to tidy up while on the subject. > > Right now I must attend to some other stuff, will return tomorrow. > > Hello Hugh > > I am going to test this ASAP,
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.
I might decide to leave invalidate_mapping_pages() giving up early (unsatisfying, but no worse than before), and convert scan_mapping_ unevictable_pages() (which is used on nothing but shmem) to pass index vector to radix_tree_gang_whatever().
Dunno, I'll think about it more later.
> but have one question below : > > > /* > > * 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)) { > > + cond_resched(); > > + start += nr_found; > > Isnt it possible to go out of initial window ? > start could be greater than 'end' ? > > invalidate_mapping_pages() > > does some capping (end - index)
Good question, but even before the change (or any of my changes here) it's perfectly possible to go out of the initial window - the radix_tree gang interfaces allow you to specify the maximum you want back (i.e. size of buffer), but they do not actually allow you to specify end of range.
There's a few places where we trim the maximum to match our end of range, but that's just a slight optimization in the face of an arguably incomplete interface. But the radix_tree is not too inefficient this way, because of how empty nodes get removed immediately - there's a limit to the number of nodes it will have to look through before it fills the buffer.
> > > > pagevec_init(&pvec, 0); > > while (index <= end && pagevec_lookup(&pvec, mapping, index, > > min(end - index, (pgoff_t)PAGEVEC_SIZE - 1) + 1)) {
It does cap by "end - index", but it has already checked "index <= end", and it is only this minor optimization, nothing essential.
Hugh | |