在 2021/5/11 17:34, Oscar Salvador 写道:
On Tue, May 11, 2021 at 10:46:00AM +0200, Jan Kara wrote:
We definitely need to wait for writeback of these pages and the change you
suggest makes sense to me. I'm just not sure whether the only problem with
these "pages in the process of being munlocked()" cannot confuse the state
machinery in memory_failure() also in some other way. Also I'm not sure if
are really allowed to call wait_on_page_writeback() on just any page that
hits memory_failure() - there can be slab pages, anon pages, completely
unknown pages given out by page allocator to device drivers etc. That needs
someone more familiar with these MM details than me.
I am not really into mm/writeback stuff, but:
shake_page() a few lines before tries to identifiy the page, and
make those sitting in lruvec real PageLRU, and then we take page's lock.
I thought that such pages (pages on writeback) are stored in the file
LRU, and maybe the code was written with that in mind? And given that
we are under the PageLock, such state could not have changed.
Hi,
Crash of this bug show we can clear page LRU without lock_page by follow
stack. So this page_lock in memory_failure seems useless to prevent this
BUG.
do_mmap->mmap_region->do_munmap->munlock_vma_pages_range->__munlock_pagevec
static void __munlock_pagevec(struct pagevec *pvec, struct zone *zone)
{
...
for (i = 0; i < nr; i++) {
struct page *page = pvec->pages[i];
if (TestClearPageMlocked(page)) {
if (TestClearPageLRU(page)) { <=== clear LRU flag
...
}
...
}
...
}
...
}
But if such pages are allowed to not be in the LRU (maybe they are taken
off before initiating the writeback?), I guess the change is correct.
Checking wait_on_page_writeback(), it seems it first checks for
Writeback bit, and since that bit is not "shared" and only being set
in mm/writeback code, it should be fine to call that.
But alternatively, we could also modify the check and go with:
if (!PageTransTail(p) && !PageLRU(p) && !PageWriteBack(p))
goto identify_page_state;
I have no idea should we process this page with such state. But it seems
reasonable to add some comments to clarify our change.
Thanks,
Kun.
and stating why a page under writeback might not be in the LRU, as I
think the code assumes.
AFAUI, mm/writeback locks the page before setting the bit, and since we
hold the lock, we could not race here.
