On Fri, Apr 17, 2020 at 05:23:31PM +0200, Michal Hocko wrote: > On Fri 17-04-20 22:26:18, chenqiwu wrote: > > On Fri, Apr 17, 2020 at 01:43:12PM +0200, Michal Hocko wrote: > > > On Fri 17-04-20 18:31:54, qiwuchen55@xxxxxxxxx wrote: > > > > From: chenqiwu <chenqiwu@xxxxxxxxxx> > > > > > > > > If we get a special mapping page like device mapping page or zero page > > > > when copy_one_pte, it's necessary add the page refcount count. > > > > > > From the changelog it is not clear what is the actual problem and how > > > the patch address it. Please be more verbose. > > > > > I don't find any actual problem, but I think there should be addressed > > to update the page refcount for special mappings include devmap and zero > > page instead of doing nothing else, since we copy the pte from one task > > to the other. > > As Matthew pointed out, zero pages are special. Just check how > vm_normal_page returns NULL (the same is the case for pte_devmap). This > means, among other things that zap_pte_range which is called during > munmap will only clear the pte but it doesn't operate on those pages so > there is no put_page for your get_page here. > > I do realize that this might be a subtle details that might be > confusing. On the other hand trying to formulate the specific problem > and add an explanation of the fix in the changelog could have revealed > this. It is really trivial to generate mappings backed by zero pages and > if the reference count was not handled properly then it would blow up > pretty quickly. > I agree. But I can't see where to put the normal page refcount back in zap_pte_range(), is there an imbalnace between copy_one_pte() and zap_pte_range()? BTW, I think the else if condition can be removed since we don't need to operate devmap pages.
