Re: [PATCH V7 05/19] dax: Add dax_operations for use by fs-dax on fsdev dax

[John Groves <John@groves.net>]

On 26/02/14 10:10AM, Ira Weiny wrote:
> John Groves wrote:
> > From: John Groves <John@Groves.net>
> > 
> > fsdev: Add dax_operations for use by famfs
> > 
> > - These methods are based on pmem_dax_ops from drivers/nvdimm/pmem.c
> > - fsdev_dax_direct_access() returns the hpa, pfn and kva. The kva was
> >   newly stored as dev_dax->virt_addr by dev_dax_probe().
> > - The hpa/pfn are used for mmap (dax_iomap_fault()), and the kva is used
> >   for read/write (dax_iomap_rw())
> 
> I thought this driver did not support mmap?

If a daxdev /dev/dax0.0 is in 'famfs' mode (bound to drivers/dax/fsdev.c),
and you open it and try to mmap - you can't - that's true.

This stuff is necessary to support mmap/read/write on famfs files.

> 
> > - fsdev_dax_recovery_write() and dev_dax_zero_page_range() have not been
> >   tested yet. I'm looking for suggestions as to how to test those.
> > - dax-private.h: add dev_dax->cached_size, which fsdev needs to
> >   remember. The dev_dax size cannot change while a driver is bound
> >   (dev_dax_resize returns -EBUSY if dev->driver is set). Caching the size
> >   at probe time allows fsdev's direct_access path can use it without
> >   acquiring dax_dev_rwsem (which isn't exported anyway).
> > 
> 
> None of the above explains exactly why this code is needed.  Rather it
> just explains what it does.
> 
> I'm not 100% clear on why this is needed in the driver and why this is not
> a layering violation which is going to bite us later?

I'll update the description to make it clear.

But basically: this is the stuff that xfs uses in /dev/pmem when it's in
fs-dax mode, to to resolve read/write to a memcpy variant, and to handle
faults via dax_iomap_fault() (which lets famfs resolve (file, offset) to
(daxdev, offset), and then dax finishes the job by resolving to PFN (or HPA -
whatever).

So for famfs to support file read/write/mmap on a devdax backing device,
this is the necessary glue.

Next patch version (v8) will make this more clear.

Thanks Ira!
John

[snip]