On Tue, Dec 22, 2015 at 02:46:05PM -0800, Andrew Morton wrote:

On Fri, 18 Dec 2015 22:22:15 -0700 Ross Zwisler [off-list ref] wrote:

quoted

Add support for tracking dirty DAX entries in the struct address_space
radix tree.  This tree is already used for dirty page writeback, and it
already supports the use of exceptional (non struct page*) entries.

In order to properly track dirty DAX pages we will insert new exceptional
entries into the radix tree that represent dirty DAX PTE or PMD pages.
These exceptional entries will also contain the writeback addresses for the
PTE or PMD faults that we can use at fsync/msync time.

There are currently two types of exceptional entries (shmem and shadow)
that can be placed into the radix tree, and this adds a third.  We rely on
the fact that only one type of exceptional entry can be found in a given
radix tree based on its usage.  This happens for free with DAX vs shmem but
we explicitly prevent shadow entries from being added to radix trees for
DAX mappings.

The only shadow entries that would be generated for DAX radix trees would
be to track zero page mappings that were created for holes.  These pages
would receive minimal benefit from having shadow entries, and the choice
to have only one type of exceptional entry in a given radix tree makes the
logic simpler both in clear_exceptional_entry() and in the rest of DAX.


...

--- a/include/linux/dax.h
+++ b/include/linux/dax.h

@@ -36,4 +36,9 @@ static inline bool vma_is_dax(struct vm_area_struct *vma)
 {
 	return vma->vm_file && IS_DAX(vma->vm_file->f_mapping->host);
 }
+
+static inline bool dax_mapping(struct address_space *mapping)
+{
+	return mapping->host && IS_DAX(mapping->host);
+}

Can we make this evaluate to plain old "0" when CONFIG_FS_DAX=n?  That
way a bunch of code in callers will fall away as well.

If the compiler has any brains then a good way to do this would be to
make IS_DAX be "0" but one would need to check that the zeroness
properly propagated out of the inline.

Ah, it already works that way due to some magic with IS_DAX().  I believe we
already use the fact that blocks protected by IS_DAX() go away if
CONFIG_FS_DAX isn't set.

The trick is that S_DAX is defined to be 0 if CONFIG_FS_DAX isn't set.

I'm pretty sure this is working because of the code in
filemap_write_and_wait_range().  I added a block with the later "dax: add
support for fsync/msync" patch which looks like this:

@@ -482,6 +482,9 @@ int filemap_write_and_wait_range(struct address_space *mapping,
 {
        int err = 0;
 
+       if (dax_mapping(mapping) && mapping->nrdax)
+               dax_writeback_mapping_range(mapping, lstart, lend);
+

Without the dax_mapping() check there the behavior is the same, but we fail to
compile if CONFIG_FS_DAX isn't set because dax_writeback_mapping_range() isn't
defined.  (Guess how I found that out.  :)  )

quoted

 #endif

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3aa5142..b9ac534 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -433,6 +433,7 @@ struct address_space {
 	/* Protected by tree_lock together with the radix tree */
 	unsigned long		nrpages;	/* number of total pages */
 	unsigned long		nrshadows;	/* number of shadow entries */
+	unsigned long		nrdax;	        /* number of DAX entries */

hm, that's unfortunate - machines commonly carry tremendous numbers of
address_spaces in memory and adding pork to them is rather a big deal. 
We can't avoid this somehow?  Maybe share the space with nrshadows by
some means?  Find some other field which is unused for dax files?

Jan Kara noticed the same thing:

https://lists.01.org/pipermail/linux-nvdimm/2015-December/003626.html

It'll be fixed in the next spin of the patch set.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs