On Mon, Apr 30, 2018 at 04:07:30PM +0200, Laurent Dufour wrote: > On 23/04/2018 08:31, Minchan Kim wrote: > > On Tue, Apr 17, 2018 at 04:33:12PM +0200, Laurent Dufour wrote: > >> pte_unmap_same() is making the assumption that the page table are still > >> around because the mmap_sem is held. > >> This is no more the case when running a speculative page fault and > >> additional check must be made to ensure that the final page table are still > >> there. > >> > >> This is now done by calling pte_spinlock() to check for the VMA's > >> consistency while locking for the page tables. > >> > >> This is requiring passing a vm_fault structure to pte_unmap_same() which is > >> containing all the needed parameters. > >> > >> As pte_spinlock() may fail in the case of a speculative page fault, if the > >> VMA has been touched in our back, pte_unmap_same() should now return 3 > >> cases : > >> 1. pte are the same (0) > >> 2. pte are different (VM_FAULT_PTNOTSAME) > >> 3. a VMA's changes has been detected (VM_FAULT_RETRY) > >> > >> The case 2 is handled by the introduction of a new VM_FAULT flag named > >> VM_FAULT_PTNOTSAME which is then trapped in cow_user_page(). > > > > I don't see such logic in this patch. > > Maybe you introduces it later? If so, please comment on it. > > Or just return 0 in case of 2 without introducing VM_FAULT_PTNOTSAME. > > Late in the series, pte_spinlock() will check for the VMA's changes and may > return 1. This will be then required to handle the 3 cases presented above. > > I can move this handling later in the series, but I wondering if this will make > it more easier to read. Just nit: During reviewing this patch, I was just curious you introduced new thing here but I couldn't find any site where use it. It makes review hard. :( That's why I said to you that please commet on it if you will use new thing late in this series. If you think as-is is better for review, it would be better to mention it explicitly. Thanks.
