Re: [RFC PATCH v3 4/9] mm, fsdax: Refactor memory-failure handler for dax mapping

[Ruan Shiyang]

On 2020/12/17 上午5:26, Dave Chinner wrote:
> On Tue, Dec 15, 2020 at 08:14:09PM +0800, Shiyang Ruan wrote:
> > The current memory_failure_dev_pagemap() can only handle single-mapped
> > dax page for fsdax mode.  The dax page could be mapped by multiple files
> > and offsets if we let reflink feature & fsdax mode work together.  So,
> > we refactor current implementation to support handle memory failure on
> > each file and offset.
> >
> > Signed-off-by: Shiyang Ruan <ruansy.fnst@xxxxxxxxxxxxxx>
> > ---
> .....
> >   static const char *action_name[] = {
> > @@ -1147,6 +1148,60 @@ static int try_to_split_thp_page(struct page *page, const char *msg)
> >   	return 0;
> >   }
> >   
> > +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;
> > +	dax_entry_t cookie;
> > +
> > +	/*
> > +	 * Prevent the inode from being freed while we are interrogating
> > +	 * the address_space, typically this would be handled by
> > +	 * lock_page(), but dax pages do not use the page lock. This
> > +	 * also prevents changes to the mapping of this pfn until
> > +	 * poison signaling is complete.
> > +	 */
> > +	cookie = dax_lock(mapping, index, &pfn);
> > +	if (!cookie)
> > +		goto unlock;
>
> Why do we need to prevent the inode from going away here? This
> function gets called by XFS after doing an xfs_iget() call to grab
> the inode that owns the block. Hence the the inode (and the mapping)
> are guaranteed to be referenced and can't go away. Hence for the
> filesystem based callers, this whole "dax_lock()" thing can go away >
> So, AFAICT, the dax_lock() stuff is only necessary when the
> filesystem can't be used to resolve the owner of physical page that
> went bad....

Yes, you are right.  I made a mistake in the calling sequence here. 
Thanks for pointing out.


--
Thanks,
Ruan Shiyang.

> Cheers,
>
> Dave.