On Tue, Apr 07, 2015 at 04:17:00PM +0300, Kirill A. Shutemov wrote:
> On Tue, Apr 07, 2015 at 03:57:32PM +0300, Boaz Harrosh wrote:
> > +/*
> > + * Handle write page faults for VM_MIXEDMAP or VM_PFNMAP for a VM_SHARED
> > + * mapping
> > + */
> > +static int wp_pfn_shared(struct mm_struct *mm,
> > +                   struct vm_area_struct *vma, unsigned long address,
> > +                   pte_t *page_table, spinlock_t *ptl, pte_t orig_pte,
> > +                   pmd_t *pmd)
> > +{
> > +   if (vma->vm_ops && vma->vm_ops->pfn_mkwrite) {
> > +           struct vm_fault vmf = {
> > +                   .page = NULL,
> > +                   .pgoff = linear_page_index(vma, address),
> > +                   .virtual_address = (void __user *)(address & PAGE_MASK),
> > +                   .flags = FAULT_FLAG_WRITE | FAULT_FLAG_MKWRITE,
> > +           };
> > +           int ret;
> > +
> > +           pte_unmap_unlock(page_table, ptl);
> > +           ret = vma->vm_ops->pfn_mkwrite(vma, &vmf);
> > +           if (ret & VM_FAULT_ERROR)
> > +                   return ret;
> > +           page_table = pte_offset_map_lock(mm, pmd, address, &ptl);
> > +           /* Did pfn_mkwrite already fixed up the pte */

Oh. I guess you've missunderstood why we need pte_same() check below.
It's not about ->pfn_mkwrite() changing the pte (generatlly, it should
not). It's requited to address race with parallel page fault to the pte.

> > +           if (!pte_same(*page_table, orig_pte)) {
> > +                   pte_unmap_unlock(page_table, ptl);
> > +                   return ret;
> 
> This should be "return 0;", shouldn't it?
> 
> VM_FAULT_NOPAGE would imply you've installed new pte, but you did not.
-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to