On Tue, 1 Jun 2021, Matthew Wilcox wrote: > On Tue, Jun 01, 2021 at 09:55:56AM -0700, Hugh Dickins wrote: > > > > Well caught: you're absolutely right that there's a bug there. > > But isn't cond_resched() just papering over the real bug, and > > what it should do is a "page = compound_head(page);" before the > > get_page_unless_zero()? How does that work out in your testing? > > You do realise you're strengthening my case for folios by suggesting > that, don't you? ;-) Hah! Well, I do realize that I'm offering you a marketing opportunity. And you won't believe how many patches I dread to post for fear of that ;-) But I'm not so sure that it strengthens your case: apparently folios had not detected this? Or do you have a hoard of folio-detected fixes waiting for the day, and a folio-kit for each of the stable releases? > > I was going to suggest that it won't make any difference because the > page reference count is frozen, but the freezing happens after the call > to unmap_page(), so it may make a difference. I think that's a good point: I may have just jumped on the missing compound_head(), without thinking it through as far as you have. I'm having trouble remembering the dynamics now; but I think there are cond_resched()s in the unmap_page() part, so the splitter may get preempted even on a non-preempt kernel; whereas the frozen part is all done expeditiously, with interrupts disabled. Greg discovered the same issue recently, but we all got sidetracked, and I don't know where his investigation ended up. He was in favour of cond_resched(), I was in favour of compound_head(); and I think I was coming to the conclusion that if cond_resched() is needed, it should not be there in __migration_entry_wait(), but somewhere up in mm/gup.c, so that other faults that retry, expecting to reschedule on return to userspace, do not get trapped in kernelspace this way. Waiting on migration entries from THP splitting is an egregious example, but others may be suffering too. Hugh
