> -----Original Message-----
> From: zhong jiang <zhongjiang-ali@xxxxxxxxxxxxxxxxx>
> 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);
>
