>>> On 21.07.11 at 15:41, David Howells <dhowe...@redhat.com> wrote:
> Jan Beulich <jbeul...@novell.com> wrote:
>
>> - ASSERTCMP(page_index, >=, next);
>> - next = page_index + 1;
>> + ASSERTCMP(page_index, >, next);
>> + next = page_index;
>
> The assertion should now trip every time. Consider what happens on an inode
> with page 0. Looking at the code:
>
> next = 0;
> do {
> if (!pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE))
> break;
>
> We start off looking for pages from 0 and up, which we then loop through:
>
> for (i = 0; i < pagevec_count(&pvec); i++) {
> struct page *page = pvec.pages[i];
> pgoff_t page_index = page->index;
>
> 'next' hasn't been modified to this point, so that it's still 0. The first
> page attached to the inode will have an index of 0.
>
> Thus the following assertion will fail:
>
> ASSERTCMP(page_index, >, next);
>
> You really do need the >= here.
Indeed, the assertion was imprecise before (and I assumed it to be
precise). I'll send a refreshed patch in a minute.
> Or, better still, I think, just get rid of the assertion - if
> pagevec_lookup()
> returns pages in an unordered list, I suspect we have problems elsewhere
> too.
I'd rather not - that would seem like an unrelated change.
Jan
>
> David
_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable
