We have a number of places where we look up a page in the page cache,
lock it, then have some kind of assertion that we got back the page we
asked for, eg filemap_fault():
page = find_get_page(mapping, offset);
...
if (!lock_page_maybe_drop_mmap(vmf, page, &fpin))
goto out_retry;
...
VM_BUG_ON_PAGE(page_to_pgoff(page) != offset, page);
but today I noticed this in shmem_undo_range():
pvec.nr = find_get_entries(mapping, index,
min(end - index, (pgoff_t)PAGEVEC_SIZE),
pvec.pages, indices);
...
VM_BUG_ON_PAGE(page_to_pgoff(page) != index, page);
...
if (!trylock_page(page))
continue;
So is page->index stable if we have a refcount on the page, or is a lock
on the page required? A refcount on the page prevents it from being
split or freed. And there's plenty of comments along the lines of:
mm/filemap.c: /* Leave page->index set: truncation lookup relies on it */
which indicates that once a page is removed from the page cache, its
index remains reliable (until it's freed).
It might be nice to remove all these assertions from the callers and
bury them down in find_get_(entry,page,entries,...), but we can't do
that if we need the lock to check the index. If we don't need the lock,
then it should be safe to check as soon as we've checked that
page == xas_reload().
