On Tue, Oct 08, 2019 at 08:00:44AM -0700, Darrick J. Wong wrote:
> > unsigned long vaddr = vmf->address;
> > loff_t pos = (loff_t)vmf->pgoff << PAGE_SHIFT;
> > struct iomap iomap = { 0 };
>
> Does this definition ^^^^^ need to be converted too? You convert the
> one in iomap_apply()...
Doesn't strictly need to, but it sure would look nicer and fit the theme.
> /*
> * The @iomap and @srcmap parameters should be set to a hole
> * prior to calling ->iomap_begin.
> */
> #define IOMAP_EMPTY_RECORD { .type = IOMAP_HOLE }
>
> ...and later...
>
> struct iomap srcmap = IOMAP_EMPTY_RECORD;
>
> ..but meh, I'm not sure that adds much.
I don't really see the point.
> > unsigned flags = IOMAP_FAULT;
> > int error, major = 0;
> > bool write = vmf->flags & FAULT_FLAG_WRITE;
> > @@ -1292,7 +1293,7 @@ static vm_fault_t dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
> > * the file system block size to be equal the page size, which means
> > * that we never have to deal with more than a single extent here.
> > */
> > - error = ops->iomap_begin(inode, pos, PAGE_SIZE, flags, &iomap);
> > + error = ops->iomap_begin(inode, pos, PAGE_SIZE, flags, &iomap, &srcmap);
>
> ->iomap_begin callers are never supposed to touch srcmap, right?
> Maybe we ought to check that srcmap.io_type == HOLE, at least until
> someone fixes this code to dax-copy the data from srcmap to iomap?
What do you mean with touch? ->iomap_begin fills it out and then the
caller looks at it, at least for places that can deal with
read-modify-write operations (DAX currently can't).
