On Wed, Jan 15, 2020 at 12:34:55PM +0100, Jan Kara wrote:

On Tue 14-01-20 09:53:54, Ira Weiny wrote:

quoted

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

quoted

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...

quoted

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.

Well, more importantly mapping != inode. There can be multiple inodes
pointing to the same mapping (struct address_space) as is the case for
example for block devices. So this counter definitely belongs into struct
address_space.

Ah Yes, great point.  Done.

Ira