> -----Original Message-----
> From: zhong jiang <zhongjiang-...@linux.alibaba.com>
> Subject: Re: [PATCH v3 05/11] mm, fsdax: Refactor memory-failure handler for
> dax mapping
> 
> > +int mf_dax_mapping_kill_procs(struct address_space *mapping, pgoff_t
> > +index, int flags) {
> > +   const bool unmap_success = true;
> > +   unsigned long pfn, size = 0;
> > +   struct to_kill *tk;
> > +   LIST_HEAD(to_kill);
> > +   int rc = -EBUSY;
> > +   loff_t start;
> > +
> > +   /* load the pfn of the dax mapping file */
> > +   pfn = dax_load_pfn(mapping, index);
> > +   if (!pfn)
> > +           return rc;
> > +   /*
> > +    * Unlike System-RAM there is no possibility to swap in a
> > +    * different physical page at a given virtual address, so all
> > +    * userspace consumption of ZONE_DEVICE memory necessitates
> > +    * SIGBUS (i.e. MF_MUST_KILL)
> > +    */
> > +   flags |= MF_ACTION_REQUIRED | MF_MUST_KILL;
> 
> MF_ACTION_REQUIRED only kill the current execution context. A page can be
> shared when reflink file be mapped by different process. We can not kill all
> process shared the page.  Other process still can access the posioned page ?

AFAIK, the other processes will receive a SIGBUS when accessing this corrupted 
range.  But I didn't add a testcase for this condition.  I'll test it.  Thanks 
for pointing out.


--
Thanks,
Ruan Shiyang.

> 
> Thanks,
> zhong jiang
> 
> > +   collect_procs_file(pfn_to_page(pfn), mapping, index, &to_kill,
> > +                      flags & MF_ACTION_REQUIRED);
> > +
> > +   list_for_each_entry(tk, &to_kill, nd)
> > +           if (tk->size_shift)
> > +                   size = max(size, 1UL << tk->size_shift);
> > +   if (size) {
> > +           /*
> > +            * Unmap the largest mapping to avoid breaking up
> > +            * device-dax mappings which are constant size. The
> > +            * actual size of the mapping being torn down is
> > +            * communicated in siginfo, see kill_proc()
> > +            */
> > +           start = (index << PAGE_SHIFT) & ~(size - 1);
> > +           unmap_mapping_range(mapping, start, start + size, 0);
> > +   }
> > +
> > +   kill_procs(&to_kill, flags & MF_MUST_KILL, !unmap_success,
> > +              pfn, flags);
> > +   rc = 0;
> > +   return rc;
> > +}
> > +EXPORT_SYMBOL_GPL(mf_dax_mapping_kill_procs);
> > +
> >   static int memory_failure_hugetlb(unsigned long pfn, int flags)
> >   {
> >     struct page *p = pfn_to_page(pfn);
> > @@ -1297,7 +1346,7 @@ static int memory_failure_dev_pagemap(unsigned
> long pfn, int flags,
> >     const bool unmap_success = true;
> >     unsigned long size = 0;
> >     struct to_kill *tk;
> > -   LIST_HEAD(tokill);
> > +   LIST_HEAD(to_kill);
> >     int rc = -EBUSY;
> >     loff_t start;
> >     dax_entry_t cookie;
> > @@ -1345,9 +1394,10 @@ static int
> memory_failure_dev_pagemap(unsigned long pfn, int flags,
> >      * SIGBUS (i.e. MF_MUST_KILL)
> >      */
> >     flags |= MF_ACTION_REQUIRED | MF_MUST_KILL;
> > -   collect_procs(page, &tokill, flags & MF_ACTION_REQUIRED);
> > +   collect_procs_file(page, page->mapping, page->index, &to_kill,
> > +                      flags & MF_ACTION_REQUIRED);
> >
> > -   list_for_each_entry(tk, &tokill, nd)
> > +   list_for_each_entry(tk, &to_kill, nd)
> >             if (tk->size_shift)
> >                     size = max(size, 1UL << tk->size_shift);
> >     if (size) {
> > @@ -1360,7 +1410,7 @@ static int memory_failure_dev_pagemap(unsigned
> long pfn, int flags,
> >             start = (page->index << PAGE_SHIFT) & ~(size - 1);
> >             unmap_mapping_range(page->mapping, start, start + size, 0);
> >     }
> > -   kill_procs(&tokill, flags & MF_MUST_KILL, !unmap_success, pfn, flags);
> > +   kill_procs(&to_kill, flags & MF_MUST_KILL, !unmap_success, pfn,
> > +flags);
> >     rc = 0;
> >   unlock:
> >     dax_unlock_page(page, cookie);
> 

Reply via email to