On Mon, Jan 13, 2020 at 05:30:04PM -0800, Darrick J. Wong wrote:

On Mon, Jan 13, 2020 at 04:46:10PM -0800, Ira Weiny wrote:

quoted

On Mon, Jan 13, 2020 at 02:22:12PM -0800, Darrick J. Wong wrote:

quoted

On Fri, Jan 10, 2020 at 11:29:39AM -0800, ira.weiny@intel.com wrote:

quoted

From: Ira Weiny <ira.weiny@intel.com>

[snip]

quoted

quoted

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index bc3654fe3b5d..1ab0906c6c7f 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c

@@ -1200,6 +1200,14 @@ xfs_ioctl_setattr_dax_invalidate(
 		goto out_unlock;
 	}
 
+	/*
+	 * If there is a mapping in place we must remain in our current mode.
+	 */
+	if (atomic64_read(&inode->i_mapped)) {

Urk, should we really be messing around with the address space
internals?

I contemplated a function call instead of checking i_mapped directly?  Is that
what you mean?

Yeah.  Abstracting the details just enough that filesystems don't have
to know that i_mapped is atomic64 etc.

Done.

quoted

quoted

quoted

+		error = -EBUSY;
+		goto out_unlock;
+	}
+
 	error = filemap_write_and_wait(inode->i_mapping);
 	if (error)
 		goto out_unlock;

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 631f11d6246e..6e7dc626b657 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -740,6 +740,7 @@ struct inode {
 #endif
 
 	void			*i_private; /* fs or device private pointer */
+	atomic64_t               i_mapped;

I would have expected to find this in struct address_space since the
mapping count is a function of the address space, right?

I suppose but the only external call (above) would be passing an inode.  So to
me it seemed better here.

But the number of memory mappings reflects the state of the address
space, not the inode.  Or maybe put another way, if I were an mm
developer I would not expect to look in struct inode for mm state.

This is a good point...

static inline bool inode_has_mappings(struct inode *inode)
{
	return atomic64_read(&inode->i_mapping->mapcount) > 0;
}

OTOH if there exist other mm developers who /do/ find that storing the
mmap count in struct inode is more logical, please let me know. :)

...  My thinking was that the number of mappings does not matters to the mm
system...  However, I'm starting to think you are correct...  ;-)

I've made a note of it and we will see what others think.

Ira

--D

quoted

Ira

quoted

--D