Re: [PATCH v3 01/11] pagemap: Introduce ->memory_failure()

[ruansy.fnst@xxxxxxxxxxx]

> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/include/linux/memremap.h b/include/linux/memremap.h
> > > > index 79c49e7f5c30..0bcf2b1e20bd 100644
> > > > --- a/include/linux/memremap.h
> > > > +++ b/include/linux/memremap.h
> > > > @@ -87,6 +87,14 @@ struct dev_pagemap_ops {
> > > >          * the page back to a CPU accessible page.
> > > >          */
> > > >         vm_fault_t (*migrate_to_ram)(struct vm_fault *vmf);
> > > > +
> > > > +       /*
> > > > +        * Handle the memory failure happens on one page.  Notify the processes
> > > > +        * who are using this page, and try to recover the data on this page
> > > > +        * if necessary.
> > > > +        */
> > > > +       int (*memory_failure)(struct dev_pagemap *pgmap, unsigned long pfn,
> > > > +                             int flags);
> > > >  };
> > >
> > > After the conversation with Dave I don't see the point of this. If
> > > there is a memory_failure() on a page, why not just call
> > > memory_failure()? That already knows how to find the inode and the
> > > filesystem can be notified from there.
> >
> > We want memory_failure() supports reflinked files.  In this case, we are not
> > able to track multiple files from a page(this broken page) because
> > page->mapping,page->index can only track one file.  Thus, I introduce this
> > ->memory_failure() implemented in pmem driver, to call ->corrupted_range()
> > upper level to upper level, and finally find out files who are
> > using(mmapping) this page.
> >
> 
> I know the motivation, but this implementation seems backwards. It's
> already the case that memory_failure() looks up the address_space
> associated with a mapping. From there I would expect a new 'struct
> address_space_operations' op to let the fs handle the case when there
> are multiple address_spaces associated with a given file.
> 

Let me think about it.  In this way, we
    1. associate file mapping with dax page in dax page fault;
    2. iterate files reflinked to notify `kill processes signal` by the
          new address_space_operation;
    3. re-associate to another reflinked file mapping when unmmaping
        (rmap qeury in filesystem to get the another file).

It did not handle those dax pages are not in use, because their ->mapping are
not associated to any file.  I didn't think it through until reading your
conversation.  Here is my understanding: this case should be handled by
badblock mechanism in pmem driver.  This badblock mechanism will call
->corrupted_range() to tell filesystem to repaire the data if possible.

So, we split it into two parts.  And dax device and block device won't be mixed
up again.   Is my understanding right?

But the solution above is to solve the hwpoison on one or couple pages, which
happens rarely(I think).  Do the 'pmem remove' operation cause hwpoison too?
Call memory_failure() so many times?  I havn't understood this yet.


--
Thanks,
Ruan Shiyang.