[PATCH 0/2] Anonymous Inode Allocations

[PATCH 0/2] Anonymous Inode Allocations

[David Herrmann]

Hi

This implements anon_inodes_new() to create anonymous inodes. Patch #1
describes the changes to anon_inodes.c and why DRM could make great use of this.
Patch #2 converts DRM core to use anon_inodes_new() instead of delayed
dev_mapping initialization (but kept simple, TTM can be converted later).

The idea is to get an anonymous backing inode for DRM devices without waiting
for the first char-dev inode. We use it to unmap userspace mappings via
unmap_mapping_range() if we have to evict buffers if GTT runs short or if the FW
framebuffers are destroyed.
We need the inode only once the first user-space mapping was created, but the
delayed initialization causes an ugly code base and keeps inodes of char-devs
around just to preserve the address_space.

We actually only need the address_space, but mm core requires an corresponding
file* and FS core requires mapping->host to be set (although I am not sure
whether we can hit those paths with char-devs). So I went with the whole
anonymous inode approach.

If anyone has an idea how to use an embedded "struct address_space" inside of
"drm_device" and set "dev_mapping->host" to the shared anon_inode_inode, I'd be
happy to implement it. However, I didn't succeed and I am actually not sure that
separate "struct address_space" are actually supported. For instance, DRM core
uses code like:
  container_of(dev_mapping, struct inode, i_data)
So I didn't spent much time on that approach.

Cheers
David

David Herrmann (2):
  anon_inodes: allow external inode allocations
  DRM: use anon_inode instead of delayed inode init

 drivers/gpu/drm/drm_drv.c              |  1 -
 drivers/gpu/drm/drm_fops.c             | 24 +++--------------------
 drivers/gpu/drm/drm_stub.c             | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_gem.c        |  4 ++--
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c     |  7 ++++---
 drivers/gpu/drm/qxl/qxl_object.c       |  2 +-
 drivers/gpu/drm/qxl/qxl_ttm.c          |  2 +-
 drivers/gpu/drm/radeon/radeon_object.c |  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
 fs/anon_inodes.c                       | 36 +++++++++++++++++++++++++++-------
 include/drm/drmP.h                     |  2 +-
 include/linux/anon_inodes.h            |  1 +
 14 files changed, 57 insertions(+), 42 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] anon_inodes: allow external inode allocations

[David Herrmann]

DRM core shares a single address_space across all inodes that belong to
the same DRM device. This allows efficient unmapping of user-space
mappings during buffer destruction. However, there is no easy way to get a
shared address_space for DRM devices during initialization. Therefore, we
currently delay this step until the first ->open() and save the given
inode for future use.

This causes ugly delayed initialization throughout the DRM core. TTM
devices end up without a dev_mapping pointer and we have to carefully
respect any underlying filesystem implementation so we don't corrupt the
inode->i_mapping and inode->i_data fields.

We can avoid this if we were allowed to allocate an anonymous inode for
each DRM device. We only have to set file->f_mapping during ->open()
and no longer need to adjust inode mappings. As fs/anon_inodes.c already
provides a minimal internal FS mount, we extend it to also provide
anonymous inodes for use in drivers like DRM.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 fs/anon_inodes.c            | 36 +++++++++++++++++++++++++++++-------
 include/linux/anon_inodes.h |  1 +
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 47a65df..7d8a80a 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -25,6 +25,7 @@
 static struct vfsmount *anon_inode_mnt __read_mostly;
 static struct inode *anon_inode_inode;
 static const struct file_operations anon_inode_fops;
+static struct dentry *anon_inode_root;
 
 /*
  * anon_inodefs_dname() is called from d_path().
@@ -87,19 +88,18 @@ static struct inode *anon_inode_mkinode(struct super_block *s)
 static struct dentry *anon_inodefs_mount(struct file_system_type *fs_type,
 				int flags, const char *dev_name, void *data)
 {
-	struct dentry *root;
-	root = mount_pseudo(fs_type, "anon_inode:", NULL,
+	anon_inode_root = mount_pseudo(fs_type, "anon_inode:", NULL,
 			&anon_inodefs_dentry_operations, ANON_INODE_FS_MAGIC);
-	if (!IS_ERR(root)) {
-		struct super_block *s = root->d_sb;
+	if (!IS_ERR(anon_inode_root)) {
+		struct super_block *s = anon_inode_root->d_sb;
 		anon_inode_inode = anon_inode_mkinode(s);
 		if (IS_ERR(anon_inode_inode)) {
-			dput(root);
+			dput(anon_inode_root);
 			deactivate_locked_super(s);
-			root = ERR_CAST(anon_inode_inode);
+			anon_inode_root = ERR_CAST(anon_inode_inode);
 		}
 	}
-	return root;
+	return anon_inode_root;
 }
 
 static struct file_system_type anon_inode_fs_type = {
@@ -219,6 +219,28 @@ err_put_unused_fd:
 }
 EXPORT_SYMBOL_GPL(anon_inode_getfd);
 
+/**
+ * anon_inode_new - create private anonymous inode
+ *
+ * Creates a new inode on the anonymous inode FS for driver's use. The inode has
+ * it's own address_space compared to the shared anon_inode_inode. It can be
+ * used in situations where user-space mappings have to be shared across
+ * different files but no backing inode is available.
+ *
+ * Call iput(inode) to release the inode.
+ *
+ * RETURNS:
+ * New inode on success, error pointer on failure.
+ */
+struct inode *anon_inode_new(void)
+{
+	if (IS_ERR(anon_inode_root))
+		return ERR_CAST(anon_inode_root);
+
+	return anon_inode_mkinode(anon_inode_root->d_sb);
+}
+EXPORT_SYMBOL_GPL(anon_inode_new);
+
 static int __init anon_inode_init(void)
 {
 	int error;
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index 8013a45..ddbd67f 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -15,6 +15,7 @@ struct file *anon_inode_getfile(const char *name,
 				void *priv, int flags);
 int anon_inode_getfd(const char *name, const struct file_operations *fops,
 		     void *priv, int flags);
+struct inode *anon_inode_new(void);
 
 #endif /* _LINUX_ANON_INODES_H */
 
-- 
1.8.3.2

Re: [PATCH 1/2] anon_inodes: allow external inode allocations

[Daniel Vetter]

On Thu, Jul 11, 2013 at 01:45:29AM +0200, David Herrmann wrote:
> DRM core shares a single address_space across all inodes that belong to
> the same DRM device. This allows efficient unmapping of user-space
> mappings during buffer destruction. However, there is no easy way to get a
> shared address_space for DRM devices during initialization. Therefore, we
> currently delay this step until the first ->open() and save the given
> inode for future use.

Quick correction for the drm use-case: We don't need the address space at
buffer destruction, since each vma holds a reference to the backing gem
object. So buffer destruction can only happen once there's guaranteed to
be no mmapping around any more.

But we make extensive use of the address_space to shoot down ptes with
unmap_mapping_range before we evict a buffer from the mmio gart. In the
page fault handler we can then move the buffer object back to a place
userspace can get at it and set up new ptes.

> This causes ugly delayed initialization throughout the DRM core. TTM
> devices end up without a dev_mapping pointer and we have to carefully
> respect any underlying filesystem implementation so we don't corrupt the
> inode->i_mapping and inode->i_data fields.
> 
> We can avoid this if we were allowed to allocate an anonymous inode for
> each DRM device. We only have to set file->f_mapping during ->open()
> and no longer need to adjust inode mappings. As fs/anon_inodes.c already
> provides a minimal internal FS mount, we extend it to also provide
> anonymous inodes for use in drivers like DRM.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

I'm not an fs guy so no comment on the patch itself, but having an
inode/address space available at driver load time will allow us to get rid
of tons of ulgy

	if (dev_mapping)
		unmap_mapping_rang(dev_mapping, ...);

code sprinkled all over drm core and drivers. So

Wanted-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  fs/anon_inodes.c            | 36 +++++++++++++++++++++++++++++-------
>  include/linux/anon_inodes.h |  1 +
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 47a65df..7d8a80a 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -25,6 +25,7 @@
>  static struct vfsmount *anon_inode_mnt __read_mostly;
>  static struct inode *anon_inode_inode;
>  static const struct file_operations anon_inode_fops;
> +static struct dentry *anon_inode_root;
>  
>  /*
>   * anon_inodefs_dname() is called from d_path().
> @@ -87,19 +88,18 @@ static struct inode *anon_inode_mkinode(struct super_block *s)
>  static struct dentry *anon_inodefs_mount(struct file_system_type *fs_type,
>  				int flags, const char *dev_name, void *data)
>  {
> -	struct dentry *root;
> -	root = mount_pseudo(fs_type, "anon_inode:", NULL,
> +	anon_inode_root = mount_pseudo(fs_type, "anon_inode:", NULL,
>  			&anon_inodefs_dentry_operations, ANON_INODE_FS_MAGIC);
> -	if (!IS_ERR(root)) {
> -		struct super_block *s = root->d_sb;
> +	if (!IS_ERR(anon_inode_root)) {
> +		struct super_block *s = anon_inode_root->d_sb;
>  		anon_inode_inode = anon_inode_mkinode(s);
>  		if (IS_ERR(anon_inode_inode)) {
> -			dput(root);
> +			dput(anon_inode_root);
>  			deactivate_locked_super(s);
> -			root = ERR_CAST(anon_inode_inode);
> +			anon_inode_root = ERR_CAST(anon_inode_inode);
>  		}
>  	}
> -	return root;
> +	return anon_inode_root;
>  }
>  
>  static struct file_system_type anon_inode_fs_type = {
> @@ -219,6 +219,28 @@ err_put_unused_fd:
>  }
>  EXPORT_SYMBOL_GPL(anon_inode_getfd);
>  
> +/**
> + * anon_inode_new - create private anonymous inode
> + *
> + * Creates a new inode on the anonymous inode FS for driver's use. The inode has
> + * it's own address_space compared to the shared anon_inode_inode. It can be
> + * used in situations where user-space mappings have to be shared across
> + * different files but no backing inode is available.
> + *
> + * Call iput(inode) to release the inode.
> + *
> + * RETURNS:
> + * New inode on success, error pointer on failure.
> + */
> +struct inode *anon_inode_new(void)
> +{
> +	if (IS_ERR(anon_inode_root))
> +		return ERR_CAST(anon_inode_root);
> +
> +	return anon_inode_mkinode(anon_inode_root->d_sb);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_new);
> +
>  static int __init anon_inode_init(void)
>  {
>  	int error;
> diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
> index 8013a45..ddbd67f 100644
> --- a/include/linux/anon_inodes.h
> +++ b/include/linux/anon_inodes.h
> @@ -15,6 +15,7 @@ struct file *anon_inode_getfile(const char *name,
>  				void *priv, int flags);
>  int anon_inode_getfd(const char *name, const struct file_operations *fops,
>  		     void *priv, int flags);
> +struct inode *anon_inode_new(void);
>  
>  #endif /* _LINUX_ANON_INODES_H */
>  
> -- 
> 1.8.3.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/2] anon_inodes: allow external inode allocations

[David Herrmann]

Hi Al

Any comment on this?

Regards
David

On Thu, Jul 11, 2013 at 1:45 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> DRM core shares a single address_space across all inodes that belong to
> the same DRM device. This allows efficient unmapping of user-space
> mappings during buffer destruction. However, there is no easy way to get a
> shared address_space for DRM devices during initialization. Therefore, we
> currently delay this step until the first ->open() and save the given
> inode for future use.
>
> This causes ugly delayed initialization throughout the DRM core. TTM
> devices end up without a dev_mapping pointer and we have to carefully
> respect any underlying filesystem implementation so we don't corrupt the
> inode->i_mapping and inode->i_data fields.
>
> We can avoid this if we were allowed to allocate an anonymous inode for
> each DRM device. We only have to set file->f_mapping during ->open()
> and no longer need to adjust inode mappings. As fs/anon_inodes.c already
> provides a minimal internal FS mount, we extend it to also provide
> anonymous inodes for use in drivers like DRM.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  fs/anon_inodes.c            | 36 +++++++++++++++++++++++++++++-------
>  include/linux/anon_inodes.h |  1 +
>  2 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 47a65df..7d8a80a 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -25,6 +25,7 @@
>  static struct vfsmount *anon_inode_mnt __read_mostly;
>  static struct inode *anon_inode_inode;
>  static const struct file_operations anon_inode_fops;
> +static struct dentry *anon_inode_root;
>
>  /*
>   * anon_inodefs_dname() is called from d_path().
> @@ -87,19 +88,18 @@ static struct inode *anon_inode_mkinode(struct super_block *s)
>  static struct dentry *anon_inodefs_mount(struct file_system_type *fs_type,
>                                 int flags, const char *dev_name, void *data)
>  {
> -       struct dentry *root;
> -       root = mount_pseudo(fs_type, "anon_inode:", NULL,
> +       anon_inode_root = mount_pseudo(fs_type, "anon_inode:", NULL,
>                         &anon_inodefs_dentry_operations, ANON_INODE_FS_MAGIC);
> -       if (!IS_ERR(root)) {
> -               struct super_block *s = root->d_sb;
> +       if (!IS_ERR(anon_inode_root)) {
> +               struct super_block *s = anon_inode_root->d_sb;
>                 anon_inode_inode = anon_inode_mkinode(s);
>                 if (IS_ERR(anon_inode_inode)) {
> -                       dput(root);
> +                       dput(anon_inode_root);
>                         deactivate_locked_super(s);
> -                       root = ERR_CAST(anon_inode_inode);
> +                       anon_inode_root = ERR_CAST(anon_inode_inode);
>                 }
>         }
> -       return root;
> +       return anon_inode_root;
>  }
>
>  static struct file_system_type anon_inode_fs_type = {
> @@ -219,6 +219,28 @@ err_put_unused_fd:
>  }
>  EXPORT_SYMBOL_GPL(anon_inode_getfd);
>
> +/**
> + * anon_inode_new - create private anonymous inode
> + *
> + * Creates a new inode on the anonymous inode FS for driver's use. The inode has
> + * it's own address_space compared to the shared anon_inode_inode. It can be
> + * used in situations where user-space mappings have to be shared across
> + * different files but no backing inode is available.
> + *
> + * Call iput(inode) to release the inode.
> + *
> + * RETURNS:
> + * New inode on success, error pointer on failure.
> + */
> +struct inode *anon_inode_new(void)
> +{
> +       if (IS_ERR(anon_inode_root))
> +               return ERR_CAST(anon_inode_root);
> +
> +       return anon_inode_mkinode(anon_inode_root->d_sb);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_new);
> +
>  static int __init anon_inode_init(void)
>  {
>         int error;
> diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
> index 8013a45..ddbd67f 100644
> --- a/include/linux/anon_inodes.h
> +++ b/include/linux/anon_inodes.h
> @@ -15,6 +15,7 @@ struct file *anon_inode_getfile(const char *name,
>                                 void *priv, int flags);
>  int anon_inode_getfd(const char *name, const struct file_operations *fops,
>                      void *priv, int flags);
> +struct inode *anon_inode_new(void);
>
>  #endif /* _LINUX_ANON_INODES_H */
>
> --
> 1.8.3.2
>

[PATCH 2/2] DRM: use anon_inode instead of delayed inode init

[David Herrmann]

Instead of delaying inode initialization until first ->open(), we can use
an anonymous inode. This avoids modifying FS internal inode fields and
provides us a private address_space right during initialization.

Delayed TTM dev_mapping initialization is currently left untouched to keep
this simple. But we could now safely provide the address_space during
ttm_bo_device_init() instead of delaying until first buffer ->mmap().

Note that this also fixes several bugs:
 - We currently call iput(container_of(..dev_mapping..)) before
   drm_lastclose(), but we reset dev_mapping to zero at the end of
   drm_lastclose(). This fails if dev_mapping points to an address_space
   other than the current inode and the char-dev got already removed.
 - We also drop dev_mapping during any drm_lastclose() call. So if
   user-space still has VMAs to our buffers, we will be unable to unmap
   them if the next ->firstopen() is on another inode. dev_mapping will
   then point to a new address_space and we leak mappings that we no
   longer control.
 - We ignore inode->i_mapping completely. It is unlikely that a FS uses it
   to overwrite inode->i_data for char-devs, but it definitely doesn't
   look very nice to ignore it silently.

Regarding legacy drivers: We no longer reset the address_space during
drm_lastclose() to avoid re-allocating inodes all the time. However,
legacy UMS drivers are weird and it is not clear to me whether some of the
old drivers might depend on this (for what reason?), but I remember Daniel
told me that i810 might.

Tested with nouveau on x86_64.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_drv.c              |  1 -
 drivers/gpu/drm/drm_fops.c             | 24 +++---------------------
 drivers/gpu/drm/drm_stub.c             | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_gem.c        |  4 ++--
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c     |  7 ++++---
 drivers/gpu/drm/qxl/qxl_object.c       |  2 +-
 drivers/gpu/drm/qxl/qxl_ttm.c          |  2 +-
 drivers/gpu/drm/radeon/radeon_object.c |  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
 include/drm/drmP.h                     |  2 +-
 12 files changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 99fcd7c..9797613 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -232,7 +232,6 @@ int drm_lastclose(struct drm_device * dev)
 	    !drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_dma_takedown(dev);
 
-	dev->dev_mapping = NULL;
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG("lastclose completed\n");
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 3a24385..6752f59 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -122,8 +122,6 @@ int drm_open(struct inode *inode, struct file *filp)
 	struct drm_minor *minor;
 	int retcode = 0;
 	int need_setup = 0;
-	struct address_space *old_mapping;
-	struct address_space *old_imapping;
 
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (!minor)
@@ -137,16 +135,9 @@ int drm_open(struct inode *inode, struct file *filp)
 
 	if (!dev->open_count++)
 		need_setup = 1;
-	mutex_lock(&dev->struct_mutex);
-	old_imapping = inode->i_mapping;
-	old_mapping = dev->dev_mapping;
-	if (old_mapping == NULL)
-		dev->dev_mapping = &inode->i_data;
-	/* ihold ensures nobody can remove inode with our i_data */
-	ihold(container_of(dev->dev_mapping, struct inode, i_data));
-	inode->i_mapping = dev->dev_mapping;
-	filp->f_mapping = dev->dev_mapping;
-	mutex_unlock(&dev->struct_mutex);
+
+	/* set address_space for shared mappings */
+	filp->f_mapping = dev->anon_inode->i_mapping;
 
 	retcode = drm_open_helper(inode, filp, dev);
 	if (retcode)
@@ -160,12 +151,6 @@ int drm_open(struct inode *inode, struct file *filp)
 	return 0;
 
 err_undo:
-	mutex_lock(&dev->struct_mutex);
-	filp->f_mapping = old_imapping;
-	inode->i_mapping = old_imapping;
-	iput(container_of(dev->dev_mapping, struct inode, i_data));
-	dev->dev_mapping = old_mapping;
-	mutex_unlock(&dev->struct_mutex);
 	dev->open_count--;
 	return retcode;
 }
@@ -543,9 +528,6 @@ int drm_release(struct inode *inode, struct file *filp)
 		}
 	}
 
-	BUG_ON(dev->dev_mapping == NULL);
-	iput(container_of(dev->dev_mapping, struct inode, i_data));
-
 	/* drop the reference held my the file priv */
 	drm_master_put(&file_priv->master);
 	file_priv->is_master = 0;
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 327ca19..45804f1 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -31,6 +31,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/anon_inodes.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/slab.h>
@@ -267,8 +268,14 @@ int drm_fill_in_dev(struct drm_device *dev,
 	mutex_init(&dev->struct_mutex);
 	mutex_init(&dev->ctxlist_mutex);
 
+	/* create private address_space on anon inode */
+	dev->anon_inode = anon_inode_new();
+	if (IS_ERR(dev->anon_inode))
+		return PTR_ERR(dev->anon_inode);
+
 	if (drm_ht_create(&dev->map_hash, 12)) {
-		return -ENOMEM;
+		retcode = -ENOMEM;
+		goto err_inode;
 	}
 
 	/* the DRM has 6 basic counters */
@@ -309,6 +316,8 @@ int drm_fill_in_dev(struct drm_device *dev,
 
       error_out_unreg:
 	drm_lastclose(dev);
+err_inode:
+	iput(dev->anon_inode);
 	return retcode;
 }
 EXPORT_SYMBOL(drm_fill_in_dev);
@@ -478,6 +487,7 @@ void drm_put_dev(struct drm_device *dev)
 
 	drm_put_minor(&dev->primary);
 
+	iput(dev->anon_inode);
 	list_del(&dev->driver_item);
 	kfree(dev->devname);
 	kfree(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 769f752..37c73e3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1425,8 +1425,8 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
 	if (!obj->fault_mappable)
 		return;
 
-	if (obj->base.dev->dev_mapping)
-		unmap_mapping_range(obj->base.dev->dev_mapping,
+	if (obj->base.dev->anon_inode->i_mapping)
+		unmap_mapping_range(obj->base.dev->anon_inode->i_mapping,
 				    (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
 				    obj->base.size, 1);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index e72d09c..ceb4d34 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -243,7 +243,7 @@ nouveau_gem_ioctl_new(struct drm_device *dev, void *data,
 	struct nouveau_bo *nvbo = NULL;
 	int ret = 0;
 
-	drm->ttm.bdev.dev_mapping = drm->dev->dev_mapping;
+	drm->ttm.bdev.dev_mapping = drm->dev->anon_inode->i_mapping;
 
 	if (!pfb->memtype_valid(pfb, req->info.tile_flags)) {
 		NV_ERROR(cli, "bad page flags: 0x%08x\n", req->info.tile_flags);
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index ebbdf41..8f77a35 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -152,7 +152,7 @@ static struct {
 static void evict_entry(struct drm_gem_object *obj,
 		enum tiler_fmt fmt, struct usergart_entry *entry)
 {
-	if (obj->dev->dev_mapping) {
+	if (obj->dev->anon_inode->i_mapping) {
 		struct omap_gem_object *omap_obj = to_omap_bo(obj);
 		int n = usergart[fmt].height;
 		size_t size = PAGE_SIZE * n;
@@ -163,12 +163,13 @@ static void evict_entry(struct drm_gem_object *obj,
 			int i;
 			/* if stride > than PAGE_SIZE then sparse mapping: */
 			for (i = n; i > 0; i--) {
-				unmap_mapping_range(obj->dev->dev_mapping,
+				unmap_mapping_range(obj->dev->anon_inode->i_mapping,
 						off, PAGE_SIZE, 1);
 				off += PAGE_SIZE * m;
 			}
 		} else {
-			unmap_mapping_range(obj->dev->dev_mapping, off, size, 1);
+			unmap_mapping_range(obj->dev->anon_inode->i_mapping,
+					    off, size, 1);
 		}
 	}
 
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 1191fe7..df2f03a 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -82,7 +82,7 @@ int qxl_bo_create(struct qxl_device *qdev,
 	int r;
 
 	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
-		qdev->mman.bdev.dev_mapping = qdev->ddev->dev_mapping;
+		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
 	if (kernel)
 		type = ttm_bo_type_kernel;
 	else
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 489cb8c..0f62d35 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -515,7 +515,7 @@ int qxl_ttm_init(struct qxl_device *qdev)
 	DRM_INFO("qxl: %luM of IO pages memory ready (VRAM domain)\n",
 		 ((unsigned)num_io_pages * PAGE_SIZE) / (1024 * 1024));
 	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
-		qdev->mman.bdev.dev_mapping = qdev->ddev->dev_mapping;
+		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
 	r = qxl_ttm_debugfs_init(qdev);
 	if (r) {
 		DRM_ERROR("Failed to init debugfs\n");
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 0219d26..f79bc77 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -120,7 +120,7 @@ int radeon_bo_create(struct radeon_device *rdev,
 
 	size = ALIGN(size, PAGE_SIZE);
 
-	rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
+	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
 	if (kernel) {
 		type = ttm_bo_type_kernel;
 	} else if (sg) {
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6c0ce89..6071ecb 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -735,7 +735,7 @@ int radeon_ttm_init(struct radeon_device *rdev)
 	}
 	DRM_INFO("radeon: %uM of GTT memory ready.\n",
 		 (unsigned)(rdev->mc.gtt_size / (1024 * 1024)));
-	rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
+	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
 
 	r = radeon_ttm_debugfs_init(rdev);
 	if (r) {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 78e2164..5b9bf2b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -760,7 +760,7 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
 		goto out_no_tfile;
 
 	file_priv->driver_priv = vmw_fp;
-	dev_priv->bdev.dev_mapping = dev->dev_mapping;
+	dev_priv->bdev.dev_mapping = dev->anon_inode->i_mapping;
 
 	return 0;
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 12083dc..51bf85e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1198,7 +1198,7 @@ struct drm_device {
 	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
 	void *dev_private;		/**< device private data */
 	void *mm_private;
-	struct address_space *dev_mapping;
+	struct inode *anon_inode;
 	struct drm_sigdata sigdata;	   /**< For block_all_signals */
 	sigset_t sigmask;
 
-- 
1.8.3.2

Re: [PATCH 2/2] DRM: use anon_inode instead of delayed inode init

[Daniel Vetter]

On Thu, Jul 11, 2013 at 01:45:30AM +0200, David Herrmann wrote:
> Instead of delaying inode initialization until first ->open(), we can use
> an anonymous inode. This avoids modifying FS internal inode fields and
> provides us a private address_space right during initialization.
> 
> Delayed TTM dev_mapping initialization is currently left untouched to keep
> this simple. But we could now safely provide the address_space during
> ttm_bo_device_init() instead of delaying until first buffer ->mmap().
> 
> Note that this also fixes several bugs:
>  - We currently call iput(container_of(..dev_mapping..)) before
>    drm_lastclose(), but we reset dev_mapping to zero at the end of
>    drm_lastclose(). This fails if dev_mapping points to an address_space
>    other than the current inode and the char-dev got already removed.
>  - We also drop dev_mapping during any drm_lastclose() call. So if
>    user-space still has VMAs to our buffers, we will be unable to unmap
>    them if the next ->firstopen() is on another inode. dev_mapping will
>    then point to a new address_space and we leak mappings that we no
>    longer control.

It's certainly ugly, but I don't think we have a real problem here. vma
grabs a reference to the open file at mmap time and we grab a reference to
the underlying gem object. So it shouldn't be possible to observe a
non-NULL dev_mapping while the inode refcount has already reached 0
anywhere we actually care. At least in drm/i915 since we never call
unmap_mapping_range if we know that there's no ptes around pointing at
this specific object (which we accurately keep track of in our fault
handler).

TTM might be different, and it's certainly good to rid us of this code.

>  - We ignore inode->i_mapping completely. It is unlikely that a FS uses it
>    to overwrite inode->i_data for char-devs, but it definitely doesn't
>    look very nice to ignore it silently.

Tbh I have no idea what the rules are here ... But since core vfs code
uses the filp->f_mapping at mmap time not frobbing inode->i_mapping looks
like the sane to do.

> Regarding legacy drivers: We no longer reset the address_space during
> drm_lastclose() to avoid re-allocating inodes all the time. However,
> legacy UMS drivers are weird and it is not clear to me whether some of the
> old drivers might depend on this (for what reason?), but I remember Daniel
> told me that i810 might.

i915 in gem+ums mode might. i810 is a different level of horror show
entirely, but since it doesn't do gem we can ignore it here.

> 
> Tested with nouveau on x86_64.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I guess it makes sense to merge that after your drm vma offset manager
changes with the little pte shootdown helper since that'll reduce the
diff?
-Daniel

> ---
>  drivers/gpu/drm/drm_drv.c              |  1 -
>  drivers/gpu/drm/drm_fops.c             | 24 +++---------------------
>  drivers/gpu/drm/drm_stub.c             | 12 +++++++++++-
>  drivers/gpu/drm/i915/i915_gem.c        |  4 ++--
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  2 +-
>  drivers/gpu/drm/omapdrm/omap_gem.c     |  7 ++++---
>  drivers/gpu/drm/qxl/qxl_object.c       |  2 +-
>  drivers/gpu/drm/qxl/qxl_ttm.c          |  2 +-
>  drivers/gpu/drm/radeon/radeon_object.c |  2 +-
>  drivers/gpu/drm/radeon/radeon_ttm.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
>  include/drm/drmP.h                     |  2 +-
>  12 files changed, 27 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 99fcd7c..9797613 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -232,7 +232,6 @@ int drm_lastclose(struct drm_device * dev)
>  	    !drm_core_check_feature(dev, DRIVER_MODESET))
>  		drm_dma_takedown(dev);
>  
> -	dev->dev_mapping = NULL;
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	DRM_DEBUG("lastclose completed\n");
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 3a24385..6752f59 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -122,8 +122,6 @@ int drm_open(struct inode *inode, struct file *filp)
>  	struct drm_minor *minor;
>  	int retcode = 0;
>  	int need_setup = 0;
> -	struct address_space *old_mapping;
> -	struct address_space *old_imapping;
>  
>  	minor = idr_find(&drm_minors_idr, minor_id);
>  	if (!minor)
> @@ -137,16 +135,9 @@ int drm_open(struct inode *inode, struct file *filp)
>  
>  	if (!dev->open_count++)
>  		need_setup = 1;
> -	mutex_lock(&dev->struct_mutex);
> -	old_imapping = inode->i_mapping;
> -	old_mapping = dev->dev_mapping;
> -	if (old_mapping == NULL)
> -		dev->dev_mapping = &inode->i_data;
> -	/* ihold ensures nobody can remove inode with our i_data */
> -	ihold(container_of(dev->dev_mapping, struct inode, i_data));
> -	inode->i_mapping = dev->dev_mapping;
> -	filp->f_mapping = dev->dev_mapping;
> -	mutex_unlock(&dev->struct_mutex);
> +
> +	/* set address_space for shared mappings */
> +	filp->f_mapping = dev->anon_inode->i_mapping;
>  
>  	retcode = drm_open_helper(inode, filp, dev);
>  	if (retcode)
> @@ -160,12 +151,6 @@ int drm_open(struct inode *inode, struct file *filp)
>  	return 0;
>  
>  err_undo:
> -	mutex_lock(&dev->struct_mutex);
> -	filp->f_mapping = old_imapping;
> -	inode->i_mapping = old_imapping;
> -	iput(container_of(dev->dev_mapping, struct inode, i_data));
> -	dev->dev_mapping = old_mapping;
> -	mutex_unlock(&dev->struct_mutex);
>  	dev->open_count--;
>  	return retcode;
>  }
> @@ -543,9 +528,6 @@ int drm_release(struct inode *inode, struct file *filp)
>  		}
>  	}
>  
> -	BUG_ON(dev->dev_mapping == NULL);
> -	iput(container_of(dev->dev_mapping, struct inode, i_data));
> -
>  	/* drop the reference held my the file priv */
>  	drm_master_put(&file_priv->master);
>  	file_priv->is_master = 0;
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 327ca19..45804f1 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -31,6 +31,7 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
>  
> +#include <linux/anon_inodes.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/slab.h>
> @@ -267,8 +268,14 @@ int drm_fill_in_dev(struct drm_device *dev,
>  	mutex_init(&dev->struct_mutex);
>  	mutex_init(&dev->ctxlist_mutex);
>  
> +	/* create private address_space on anon inode */
> +	dev->anon_inode = anon_inode_new();
> +	if (IS_ERR(dev->anon_inode))
> +		return PTR_ERR(dev->anon_inode);
> +
>  	if (drm_ht_create(&dev->map_hash, 12)) {
> -		return -ENOMEM;
> +		retcode = -ENOMEM;
> +		goto err_inode;
>  	}
>  
>  	/* the DRM has 6 basic counters */
> @@ -309,6 +316,8 @@ int drm_fill_in_dev(struct drm_device *dev,
>  
>        error_out_unreg:
>  	drm_lastclose(dev);
> +err_inode:
> +	iput(dev->anon_inode);
>  	return retcode;
>  }
>  EXPORT_SYMBOL(drm_fill_in_dev);
> @@ -478,6 +487,7 @@ void drm_put_dev(struct drm_device *dev)
>  
>  	drm_put_minor(&dev->primary);
>  
> +	iput(dev->anon_inode);
>  	list_del(&dev->driver_item);
>  	kfree(dev->devname);
>  	kfree(dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 769f752..37c73e3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1425,8 +1425,8 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
>  	if (!obj->fault_mappable)
>  		return;
>  
> -	if (obj->base.dev->dev_mapping)
> -		unmap_mapping_range(obj->base.dev->dev_mapping,
> +	if (obj->base.dev->anon_inode->i_mapping)
> +		unmap_mapping_range(obj->base.dev->anon_inode->i_mapping,
>  				    (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
>  				    obj->base.size, 1);
>  
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index e72d09c..ceb4d34 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -243,7 +243,7 @@ nouveau_gem_ioctl_new(struct drm_device *dev, void *data,
>  	struct nouveau_bo *nvbo = NULL;
>  	int ret = 0;
>  
> -	drm->ttm.bdev.dev_mapping = drm->dev->dev_mapping;
> +	drm->ttm.bdev.dev_mapping = drm->dev->anon_inode->i_mapping;
>  
>  	if (!pfb->memtype_valid(pfb, req->info.tile_flags)) {
>  		NV_ERROR(cli, "bad page flags: 0x%08x\n", req->info.tile_flags);
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index ebbdf41..8f77a35 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -152,7 +152,7 @@ static struct {
>  static void evict_entry(struct drm_gem_object *obj,
>  		enum tiler_fmt fmt, struct usergart_entry *entry)
>  {
> -	if (obj->dev->dev_mapping) {
> +	if (obj->dev->anon_inode->i_mapping) {
>  		struct omap_gem_object *omap_obj = to_omap_bo(obj);
>  		int n = usergart[fmt].height;
>  		size_t size = PAGE_SIZE * n;
> @@ -163,12 +163,13 @@ static void evict_entry(struct drm_gem_object *obj,
>  			int i;
>  			/* if stride > than PAGE_SIZE then sparse mapping: */
>  			for (i = n; i > 0; i--) {
> -				unmap_mapping_range(obj->dev->dev_mapping,
> +				unmap_mapping_range(obj->dev->anon_inode->i_mapping,
>  						off, PAGE_SIZE, 1);
>  				off += PAGE_SIZE * m;
>  			}
>  		} else {
> -			unmap_mapping_range(obj->dev->dev_mapping, off, size, 1);
> +			unmap_mapping_range(obj->dev->anon_inode->i_mapping,
> +					    off, size, 1);
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
> index 1191fe7..df2f03a 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -82,7 +82,7 @@ int qxl_bo_create(struct qxl_device *qdev,
>  	int r;
>  
>  	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
> -		qdev->mman.bdev.dev_mapping = qdev->ddev->dev_mapping;
> +		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
>  	if (kernel)
>  		type = ttm_bo_type_kernel;
>  	else
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 489cb8c..0f62d35 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -515,7 +515,7 @@ int qxl_ttm_init(struct qxl_device *qdev)
>  	DRM_INFO("qxl: %luM of IO pages memory ready (VRAM domain)\n",
>  		 ((unsigned)num_io_pages * PAGE_SIZE) / (1024 * 1024));
>  	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
> -		qdev->mman.bdev.dev_mapping = qdev->ddev->dev_mapping;
> +		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
>  	r = qxl_ttm_debugfs_init(qdev);
>  	if (r) {
>  		DRM_ERROR("Failed to init debugfs\n");
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 0219d26..f79bc77 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -120,7 +120,7 @@ int radeon_bo_create(struct radeon_device *rdev,
>  
>  	size = ALIGN(size, PAGE_SIZE);
>  
> -	rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
> +	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
>  	if (kernel) {
>  		type = ttm_bo_type_kernel;
>  	} else if (sg) {
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 6c0ce89..6071ecb 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -735,7 +735,7 @@ int radeon_ttm_init(struct radeon_device *rdev)
>  	}
>  	DRM_INFO("radeon: %uM of GTT memory ready.\n",
>  		 (unsigned)(rdev->mc.gtt_size / (1024 * 1024)));
> -	rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
> +	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
>  
>  	r = radeon_ttm_debugfs_init(rdev);
>  	if (r) {
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 78e2164..5b9bf2b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -760,7 +760,7 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
>  		goto out_no_tfile;
>  
>  	file_priv->driver_priv = vmw_fp;
> -	dev_priv->bdev.dev_mapping = dev->dev_mapping;
> +	dev_priv->bdev.dev_mapping = dev->anon_inode->i_mapping;
>  
>  	return 0;
>  
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 12083dc..51bf85e 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1198,7 +1198,7 @@ struct drm_device {
>  	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
>  	void *dev_private;		/**< device private data */
>  	void *mm_private;
> -	struct address_space *dev_mapping;
> +	struct inode *anon_inode;
>  	struct drm_sigdata sigdata;	   /**< For block_all_signals */
>  	sigset_t sigmask;
>  
> -- 
> 1.8.3.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v2 0/4] Anonymous Inode Allocations

[David Herrmann]

Hi

This is v2 of my anon_inodes allocation series. I would really like some
comments on #1 (and maybe #2) from VFS maintainers/developers so we can continue
with this work. Patches 1 and 2 are unchanged, I added 3 and 4 to clean up DRM
delayed initializations based on the new anon_inode.


This implements anon_inodes_new() to create anonymous inodes. Patch #1
describes the changes to anon_inodes.c and why DRM could make great use of this.
Patch #2 converts DRM core to use anon_inodes_new() instead of delayed
dev_mapping initialization. Patches #3 and #4 simplify the various driver bits
to drop unused code paths.

The idea is to get an anonymous backing inode for DRM devices without waiting
for the first char-dev inode. We use it to unmap userspace mappings via
unmap_mapping_range() if we have to evict buffers if GTT runs short or if the FW
framebuffers are destroyed.
We need the inode only once the first user-space mapping was created. But the
delayed initialization causes an ugly code base and keeps inodes of char-devs
around just to preserve the address_space.

We actually only need the address_space, but mm core requires a corresponding
file* and FS core requires mapping->host to be set (although I am not sure
whether we can hit those paths with char-devs). So I went with the whole
anonymous inode approach.

If anyone has an idea how to use an embedded "struct address_space" inside of
"drm_device" and set "dev_mapping->host" to the shared anon_inode_inode, I'd be
happy to implement it. However, I didn't succeed and I am actually not sure that
separate "struct address_space" are actually supported. For instance, DRM core
uses code like:
  container_of(dev_mapping, struct inode, i_data)
which would break in this case.

Cheers
David

David Herrmann (4):
  anon_inodes: allow external inode allocations
  DRM: use anon_inode instead of delayed inode init
  drm: init TTM dev_mapping in ttm_bo_device_init()
  drm/omap: remove useless if() in evict_entry()

 drivers/gpu/drm/ast/ast_ttm.c          |  4 +++-
 drivers/gpu/drm/cirrus/cirrus_ttm.c    |  4 +++-
 drivers/gpu/drm/drm_drv.c              |  1 -
 drivers/gpu/drm/drm_fops.c             | 24 +++--------------------
 drivers/gpu/drm/drm_stub.c             | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_gem.c        |  3 ++-
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  4 +++-
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  2 --
 drivers/gpu/drm/nouveau/nouveau_ttm.c  |  4 +++-
 drivers/gpu/drm/omapdrm/omap_gem.c     | 33 +++++++++++++++----------------
 drivers/gpu/drm/qxl/qxl_object.c       |  2 --
 drivers/gpu/drm/qxl/qxl_ttm.c          |  6 +++---
 drivers/gpu/drm/radeon/radeon_object.c |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c    |  5 +++--
 drivers/gpu/drm/ttm/ttm_bo.c           |  3 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  5 +++--
 fs/anon_inodes.c                       | 36 +++++++++++++++++++++++++++-------
 include/drm/drmP.h                     |  2 +-
 include/drm/drm_vma_manager.h          |  6 +++---
 include/drm/ttm/ttm_bo_driver.h        |  2 ++
 include/linux/anon_inodes.h            |  1 +
 21 files changed, 91 insertions(+), 69 deletions(-)

-- 
1.8.3.4

[PATCH v2 1/4] anon_inodes: allow external inode allocations

[David Herrmann]

DRM core shares a single address_space across all inodes that belong to
the same DRM device. This allows efficient unmapping of user-space
mappings during buffer eviction. However, there is no easy way to get a
shared address_space for DRM devices during initialization. Therefore, we
currently delay this step until the first ->open() and save the given
inode for future use.

This causes ugly delayed initialization throughout the DRM core. TTM
devices end up without a dev_mapping pointer and we have to carefully
respect any underlying filesystem implementation so we don't corrupt the
inode->i_mapping and inode->i_data fields.

We can avoid this if we were allowed to allocate an anonymous inode for
each DRM device. We only have to set file->f_mapping during ->open()
and no longer need to adjust inode mappings. As fs/anon_inodes.c already
provides a minimal internal FS mount, we extend it to also provide
anonymous inodes for use in drivers like DRM.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Wanted-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 fs/anon_inodes.c            | 36 +++++++++++++++++++++++++++++-------
 include/linux/anon_inodes.h |  1 +
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 47a65df..7d8a80a 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -25,6 +25,7 @@
 static struct vfsmount *anon_inode_mnt __read_mostly;
 static struct inode *anon_inode_inode;
 static const struct file_operations anon_inode_fops;
+static struct dentry *anon_inode_root;
 
 /*
  * anon_inodefs_dname() is called from d_path().
@@ -87,19 +88,18 @@ static struct inode *anon_inode_mkinode(struct super_block *s)
 static struct dentry *anon_inodefs_mount(struct file_system_type *fs_type,
 				int flags, const char *dev_name, void *data)
 {
-	struct dentry *root;
-	root = mount_pseudo(fs_type, "anon_inode:", NULL,
+	anon_inode_root = mount_pseudo(fs_type, "anon_inode:", NULL,
 			&anon_inodefs_dentry_operations, ANON_INODE_FS_MAGIC);
-	if (!IS_ERR(root)) {
-		struct super_block *s = root->d_sb;
+	if (!IS_ERR(anon_inode_root)) {
+		struct super_block *s = anon_inode_root->d_sb;
 		anon_inode_inode = anon_inode_mkinode(s);
 		if (IS_ERR(anon_inode_inode)) {
-			dput(root);
+			dput(anon_inode_root);
 			deactivate_locked_super(s);
-			root = ERR_CAST(anon_inode_inode);
+			anon_inode_root = ERR_CAST(anon_inode_inode);
 		}
 	}
-	return root;
+	return anon_inode_root;
 }
 
 static struct file_system_type anon_inode_fs_type = {
@@ -219,6 +219,28 @@ err_put_unused_fd:
 }
 EXPORT_SYMBOL_GPL(anon_inode_getfd);
 
+/**
+ * anon_inode_new - create private anonymous inode
+ *
+ * Creates a new inode on the anonymous inode FS for driver's use. The inode has
+ * it's own address_space compared to the shared anon_inode_inode. It can be
+ * used in situations where user-space mappings have to be shared across
+ * different files but no backing inode is available.
+ *
+ * Call iput(inode) to release the inode.
+ *
+ * RETURNS:
+ * New inode on success, error pointer on failure.
+ */
+struct inode *anon_inode_new(void)
+{
+	if (IS_ERR(anon_inode_root))
+		return ERR_CAST(anon_inode_root);
+
+	return anon_inode_mkinode(anon_inode_root->d_sb);
+}
+EXPORT_SYMBOL_GPL(anon_inode_new);
+
 static int __init anon_inode_init(void)
 {
 	int error;
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index 8013a45..ddbd67f 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -15,6 +15,7 @@ struct file *anon_inode_getfile(const char *name,
 				void *priv, int flags);
 int anon_inode_getfd(const char *name, const struct file_operations *fops,
 		     void *priv, int flags);
+struct inode *anon_inode_new(void);
 
 #endif /* _LINUX_ANON_INODES_H */
 
-- 
1.8.3.4

[PATCH v2 2/4] DRM: use anon_inode instead of delayed inode init

[David Herrmann]

Instead of delaying inode initialization until first ->open(), we can use
an anonymous inode. This avoids modifying FS internal inode fields and
provides us a private address_space right during initialization.

Delayed TTM dev_mapping initialization is currently left untouched to keep
this simple. But we could now safely provide the address_space during
ttm_bo_device_init() instead of delaying until first buffer ->mmap().

Note that this also fixes several bugs:
 - We currently call iput(container_of(..dev_mapping..)) before
   drm_lastclose(), but we reset dev_mapping to zero at the end of
   drm_lastclose(). This fails if dev_mapping points to an address_space
   other than the current inode and the char-dev got already removed.
 - We also drop dev_mapping during any drm_lastclose() call. So if
   user-space still has VMAs to our buffers, we will be unable to unmap
   them if the next ->firstopen() is on another inode. dev_mapping will
   then point to a new address_space and we leak mappings that we no
   longer control.
 - We ignore inode->i_mapping completely. It is unlikely that a FS uses it
   to overwrite inode->i_data for char-devs, but it definitely doesn't
   look very nice to ignore it silently.

Regarding legacy drivers: We no longer reset the address_space during
drm_lastclose() to avoid re-allocating inodes all the time. However,
legacy UMS drivers are weird and it is not clear to me whether some of the
old drivers might depend on this (for what reason?), but I remember Daniel
told me that i810 might.

Tested with nouveau on x86_64.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_drv.c              |  1 -
 drivers/gpu/drm/drm_fops.c             | 24 +++---------------------
 drivers/gpu/drm/drm_stub.c             | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_gem.c        |  3 ++-
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c     |  7 ++++---
 drivers/gpu/drm/qxl/qxl_object.c       |  2 +-
 drivers/gpu/drm/qxl/qxl_ttm.c          |  2 +-
 drivers/gpu/drm/radeon/radeon_object.c |  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
 include/drm/drmP.h                     |  2 +-
 12 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index dddd799..7dcad34 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -213,7 +213,6 @@ int drm_lastclose(struct drm_device * dev)
 	    !drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_dma_takedown(dev);
 
-	dev->dev_mapping = NULL;
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG("lastclose completed\n");
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 72acae9..300d3b3 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -112,8 +112,6 @@ int drm_open(struct inode *inode, struct file *filp)
 	struct drm_minor *minor;
 	int retcode = 0;
 	int need_setup = 0;
-	struct address_space *old_mapping;
-	struct address_space *old_imapping;
 
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (!minor)
@@ -127,16 +125,9 @@ int drm_open(struct inode *inode, struct file *filp)
 
 	if (!dev->open_count++)
 		need_setup = 1;
-	mutex_lock(&dev->struct_mutex);
-	old_imapping = inode->i_mapping;
-	old_mapping = dev->dev_mapping;
-	if (old_mapping == NULL)
-		dev->dev_mapping = &inode->i_data;
-	/* ihold ensures nobody can remove inode with our i_data */
-	ihold(container_of(dev->dev_mapping, struct inode, i_data));
-	inode->i_mapping = dev->dev_mapping;
-	filp->f_mapping = dev->dev_mapping;
-	mutex_unlock(&dev->struct_mutex);
+
+	/* set address_space for shared mappings */
+	filp->f_mapping = dev->anon_inode->i_mapping;
 
 	retcode = drm_open_helper(inode, filp, dev);
 	if (retcode)
@@ -150,12 +141,6 @@ int drm_open(struct inode *inode, struct file *filp)
 	return 0;
 
 err_undo:
-	mutex_lock(&dev->struct_mutex);
-	filp->f_mapping = old_imapping;
-	inode->i_mapping = old_imapping;
-	iput(container_of(dev->dev_mapping, struct inode, i_data));
-	dev->dev_mapping = old_mapping;
-	mutex_unlock(&dev->struct_mutex);
 	dev->open_count--;
 	return retcode;
 }
@@ -533,9 +518,6 @@ int drm_release(struct inode *inode, struct file *filp)
 		}
 	}
 
-	BUG_ON(dev->dev_mapping == NULL);
-	iput(container_of(dev->dev_mapping, struct inode, i_data));
-
 	/* drop the reference held my the file priv */
 	drm_master_put(&file_priv->master);
 	file_priv->is_master = 0;
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index d663f7d..8f086f0 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -31,6 +31,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/anon_inodes.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/slab.h>
@@ -267,8 +268,14 @@ int drm_fill_in_dev(struct drm_device *dev,
 	mutex_init(&dev->struct_mutex);
 	mutex_init(&dev->ctxlist_mutex);
 
+	/* create private address_space on anon inode */
+	dev->anon_inode = anon_inode_new();
+	if (IS_ERR(dev->anon_inode))
+		return PTR_ERR(dev->anon_inode);
+
 	if (drm_ht_create(&dev->map_hash, 12)) {
-		return -ENOMEM;
+		retcode = -ENOMEM;
+		goto err_inode;
 	}
 
 	/* the DRM has 6 basic counters */
@@ -309,6 +316,8 @@ int drm_fill_in_dev(struct drm_device *dev,
 
       error_out_unreg:
 	drm_lastclose(dev);
+err_inode:
+	iput(dev->anon_inode);
 	return retcode;
 }
 EXPORT_SYMBOL(drm_fill_in_dev);
@@ -473,6 +482,7 @@ void drm_put_dev(struct drm_device *dev)
 
 	drm_put_minor(&dev->primary);
 
+	iput(dev->anon_inode);
 	list_del(&dev->driver_item);
 	kfree(dev->devname);
 	kfree(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 26c5f80..02c77d4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1420,7 +1420,8 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
 	if (!obj->fault_mappable)
 		return;
 
-	drm_vma_node_unmap(&obj->base.vma_node, obj->base.dev->dev_mapping);
+	drm_vma_node_unmap(&obj->base.vma_node,
+			   obj->base.dev->anon_inode->i_mapping);
 	obj->fault_mappable = false;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 487242f..220c406 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -237,7 +237,7 @@ nouveau_gem_ioctl_new(struct drm_device *dev, void *data,
 	struct nouveau_bo *nvbo = NULL;
 	int ret = 0;
 
-	drm->ttm.bdev.dev_mapping = drm->dev->dev_mapping;
+	drm->ttm.bdev.dev_mapping = drm->dev->anon_inode->i_mapping;
 
 	if (!pfb->memtype_valid(pfb, req->info.tile_flags)) {
 		NV_ERROR(cli, "bad page flags: 0x%08x\n", req->info.tile_flags);
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index b1f1970..8219940 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -153,7 +153,7 @@ static struct {
 static void evict_entry(struct drm_gem_object *obj,
 		enum tiler_fmt fmt, struct usergart_entry *entry)
 {
-	if (obj->dev->dev_mapping) {
+	if (obj->dev->anon_inode->i_mapping) {
 		struct omap_gem_object *omap_obj = to_omap_bo(obj);
 		int n = usergart[fmt].height;
 		size_t size = PAGE_SIZE * n;
@@ -164,12 +164,13 @@ static void evict_entry(struct drm_gem_object *obj,
 			int i;
 			/* if stride > than PAGE_SIZE then sparse mapping: */
 			for (i = n; i > 0; i--) {
-				unmap_mapping_range(obj->dev->dev_mapping,
+				unmap_mapping_range(obj->dev->anon_inode->i_mapping,
 						off, PAGE_SIZE, 1);
 				off += PAGE_SIZE * m;
 			}
 		} else {
-			unmap_mapping_range(obj->dev->dev_mapping, off, size, 1);
+			unmap_mapping_range(obj->dev->anon_inode->i_mapping,
+					    off, size, 1);
 		}
 	}
 
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 1191fe7..df2f03a 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -82,7 +82,7 @@ int qxl_bo_create(struct qxl_device *qdev,
 	int r;
 
 	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
-		qdev->mman.bdev.dev_mapping = qdev->ddev->dev_mapping;
+		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
 	if (kernel)
 		type = ttm_bo_type_kernel;
 	else
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 489cb8c..0f62d35 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -515,7 +515,7 @@ int qxl_ttm_init(struct qxl_device *qdev)
 	DRM_INFO("qxl: %luM of IO pages memory ready (VRAM domain)\n",
 		 ((unsigned)num_io_pages * PAGE_SIZE) / (1024 * 1024));
 	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
-		qdev->mman.bdev.dev_mapping = qdev->ddev->dev_mapping;
+		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
 	r = qxl_ttm_debugfs_init(qdev);
 	if (r) {
 		DRM_ERROR("Failed to init debugfs\n");
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 2020bf4..d535092 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -120,7 +120,7 @@ int radeon_bo_create(struct radeon_device *rdev,
 
 	size = ALIGN(size, PAGE_SIZE);
 
-	rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
+	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
 	if (kernel) {
 		type = ttm_bo_type_kernel;
 	} else if (sg) {
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6c0ce89..6071ecb 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -735,7 +735,7 @@ int radeon_ttm_init(struct radeon_device *rdev)
 	}
 	DRM_INFO("radeon: %uM of GTT memory ready.\n",
 		 (unsigned)(rdev->mc.gtt_size / (1024 * 1024)));
-	rdev->mman.bdev.dev_mapping = rdev->ddev->dev_mapping;
+	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
 
 	r = radeon_ttm_debugfs_init(rdev);
 	if (r) {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 5086150..375110d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -760,7 +760,7 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
 		goto out_no_tfile;
 
 	file_priv->driver_priv = vmw_fp;
-	dev_priv->bdev.dev_mapping = dev->dev_mapping;
+	dev_priv->bdev.dev_mapping = dev->anon_inode->i_mapping;
 
 	return 0;
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3ecdde6..8a8973d 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1187,7 +1187,7 @@ struct drm_device {
 	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
 	void *dev_private;		/**< device private data */
 	void *mm_private;
-	struct address_space *dev_mapping;
+	struct inode *anon_inode;
 	struct drm_sigdata sigdata;	   /**< For block_all_signals */
 	sigset_t sigmask;
 
-- 
1.8.3.4

[PATCH v2 3/4] drm: init TTM dev_mapping in ttm_bo_device_init()

[David Herrmann]

With dev->anon_inode we have a global address_space ready for operation
right from the beginning. Therefore, there is no need to do a delayed
setup with TTM. Instead, set dev_mapping during initialization in
ttm_bo_device_init() and remove any if (dev_mapping) conditions.

Cc: Dave Airlie <airlied@redhat.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/ast/ast_ttm.c          | 4 +++-
 drivers/gpu/drm/cirrus/cirrus_ttm.c    | 4 +++-
 drivers/gpu/drm/mgag200/mgag200_ttm.c  | 4 +++-
 drivers/gpu/drm/nouveau/nouveau_gem.c  | 2 --
 drivers/gpu/drm/nouveau/nouveau_ttm.c  | 4 +++-
 drivers/gpu/drm/qxl/qxl_object.c       | 2 --
 drivers/gpu/drm/qxl/qxl_ttm.c          | 6 +++---
 drivers/gpu/drm/radeon/radeon_object.c | 1 -
 drivers/gpu/drm/radeon/radeon_ttm.c    | 5 +++--
 drivers/gpu/drm/ttm/ttm_bo.c           | 3 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    | 5 +++--
 include/drm/drm_vma_manager.h          | 6 +++---
 include/drm/ttm/ttm_bo_driver.h        | 2 ++
 13 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 98d6708..2ba6a53 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -257,7 +257,9 @@ int ast_mm_init(struct ast_private *ast)
 
 	ret = ttm_bo_device_init(&ast->ttm.bdev,
 				 ast->ttm.bo_global_ref.ref.object,
-				 &ast_bo_driver, DRM_FILE_PAGE_OFFSET,
+				 &ast_bo_driver,
+				 dev->anon_inode->i_mapping,
+				 DRM_FILE_PAGE_OFFSET,
 				 true);
 	if (ret) {
 		DRM_ERROR("Error initialising bo driver; %d\n", ret);
diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 0047012..9acfc77 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -257,7 +257,9 @@ int cirrus_mm_init(struct cirrus_device *cirrus)
 
 	ret = ttm_bo_device_init(&cirrus->ttm.bdev,
 				 cirrus->ttm.bo_global_ref.ref.object,
-				 &cirrus_bo_driver, DRM_FILE_PAGE_OFFSET,
+				 &cirrus_bo_driver,
+				 dev->anon_inode->i_mapping,
+				 DRM_FILE_PAGE_OFFSET,
 				 true);
 	if (ret) {
 		DRM_ERROR("Error initialising bo driver; %d\n", ret);
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3acb2b0..90a5833 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -257,7 +257,9 @@ int mgag200_mm_init(struct mga_device *mdev)
 
 	ret = ttm_bo_device_init(&mdev->ttm.bdev,
 				 mdev->ttm.bo_global_ref.ref.object,
-				 &mgag200_bo_driver, DRM_FILE_PAGE_OFFSET,
+				 &mgag200_bo_driver,
+				 dev->anon_inode->i_mapping,
+				 DRM_FILE_PAGE_OFFSET,
 				 true);
 	if (ret) {
 		DRM_ERROR("Error initialising bo driver; %d\n", ret);
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 220c406..605099f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -237,8 +237,6 @@ nouveau_gem_ioctl_new(struct drm_device *dev, void *data,
 	struct nouveau_bo *nvbo = NULL;
 	int ret = 0;
 
-	drm->ttm.bdev.dev_mapping = drm->dev->anon_inode->i_mapping;
-
 	if (!pfb->memtype_valid(pfb, req->info.tile_flags)) {
 		NV_ERROR(cli, "bad page flags: 0x%08x\n", req->info.tile_flags);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 19e3757..d486f25 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -375,7 +375,9 @@ nouveau_ttm_init(struct nouveau_drm *drm)
 
 	ret = ttm_bo_device_init(&drm->ttm.bdev,
 				  drm->ttm.bo_global_ref.ref.object,
-				  &nouveau_bo_driver, DRM_FILE_PAGE_OFFSET,
+				  &nouveau_bo_driver,
+				  dev->anon_inode->i_mapping,
+				  DRM_FILE_PAGE_OFFSET,
 				  bits <= 32 ? true : false);
 	if (ret) {
 		NV_ERROR(drm, "error initialising bo driver, %d\n", ret);
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index df2f03a..e9b4af8 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -81,8 +81,6 @@ int qxl_bo_create(struct qxl_device *qdev,
 	enum ttm_bo_type type;
 	int r;
 
-	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
-		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
 	if (kernel)
 		type = ttm_bo_type_kernel;
 	else
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 0f62d35..1a69a33 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -491,7 +491,9 @@ int qxl_ttm_init(struct qxl_device *qdev)
 	/* No others user of address space so set it to 0 */
 	r = ttm_bo_device_init(&qdev->mman.bdev,
 			       qdev->mman.bo_global_ref.ref.object,
-			       &qxl_bo_driver, DRM_FILE_PAGE_OFFSET, 0);
+			       &qxl_bo_driver,
+			       qdev->ddev->anon_inode->i_mapping,
+			       DRM_FILE_PAGE_OFFSET, 0);
 	if (r) {
 		DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
 		return r;
@@ -514,8 +516,6 @@ int qxl_ttm_init(struct qxl_device *qdev)
 		 (unsigned)qdev->vram_size / (1024 * 1024));
 	DRM_INFO("qxl: %luM of IO pages memory ready (VRAM domain)\n",
 		 ((unsigned)num_io_pages * PAGE_SIZE) / (1024 * 1024));
-	if (unlikely(qdev->mman.bdev.dev_mapping == NULL))
-		qdev->mman.bdev.dev_mapping = qdev->ddev->anon_inode->i_mapping;
 	r = qxl_ttm_debugfs_init(qdev);
 	if (r) {
 		DRM_ERROR("Failed to init debugfs\n");
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index d535092..d96b47b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -120,7 +120,6 @@ int radeon_bo_create(struct radeon_device *rdev,
 
 	size = ALIGN(size, PAGE_SIZE);
 
-	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
 	if (kernel) {
 		type = ttm_bo_type_kernel;
 	} else if (sg) {
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6071ecb..200cb54 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -697,7 +697,9 @@ int radeon_ttm_init(struct radeon_device *rdev)
 	/* No others user of address space so set it to 0 */
 	r = ttm_bo_device_init(&rdev->mman.bdev,
 			       rdev->mman.bo_global_ref.ref.object,
-			       &radeon_bo_driver, DRM_FILE_PAGE_OFFSET,
+			       &radeon_bo_driver,
+			       rdev->ddev->anon_inode->i_mapping,
+			       DRM_FILE_PAGE_OFFSET,
 			       rdev->need_dma32);
 	if (r) {
 		DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
@@ -735,7 +737,6 @@ int radeon_ttm_init(struct radeon_device *rdev)
 	}
 	DRM_INFO("radeon: %uM of GTT memory ready.\n",
 		 (unsigned)(rdev->mc.gtt_size / (1024 * 1024)));
-	rdev->mman.bdev.dev_mapping = rdev->ddev->anon_inode->i_mapping;
 
 	r = radeon_ttm_debugfs_init(rdev);
 	if (r) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index f1a857e..0652043 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1428,6 +1428,7 @@ EXPORT_SYMBOL(ttm_bo_device_release);
 int ttm_bo_device_init(struct ttm_bo_device *bdev,
 		       struct ttm_bo_global *glob,
 		       struct ttm_bo_driver *driver,
+		       struct address_space *mapping,
 		       uint64_t file_page_offset,
 		       bool need_dma32)
 {
@@ -1449,7 +1450,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 				    0x10000000);
 	INIT_DELAYED_WORK(&bdev->wq, ttm_bo_delayed_workqueue);
 	INIT_LIST_HEAD(&bdev->ddestroy);
-	bdev->dev_mapping = NULL;
+	bdev->dev_mapping = mapping;
 	bdev->glob = glob;
 	bdev->need_dma32 = need_dma32;
 	bdev->val_seq = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 375110d..17d1ba2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -543,7 +543,9 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 
 	ret = ttm_bo_device_init(&dev_priv->bdev,
 				 dev_priv->bo_global_ref.ref.object,
-				 &vmw_bo_driver, VMWGFX_FILE_PAGE_OFFSET,
+				 &vmw_bo_driver,
+				 dev->anon_inode->i_mapping,
+				 VMWGFX_FILE_PAGE_OFFSET,
 				 false);
 	if (unlikely(ret != 0)) {
 		DRM_ERROR("Failed initializing TTM buffer object driver.\n");
@@ -760,7 +762,6 @@ static int vmw_driver_open(struct drm_device *dev, struct drm_file *file_priv)
 		goto out_no_tfile;
 
 	file_priv->driver_priv = vmw_fp;
-	dev_priv->bdev.dev_mapping = dev->anon_inode->i_mapping;
 
 	return 0;
 
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 22eedac..f0e96cb 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -206,8 +206,8 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
  * @file_mapping: Address space to unmap @node from
  *
  * Unmap all userspace mappings for a given offset node. The mappings must be
- * associated with the @file_mapping address-space. If no offset exists or
- * the address-space is invalid, nothing is done.
+ * associated with the @file_mapping address-space. If no offset exists
+ * nothing is done.
  *
  * This call is unlocked. The caller must guarantee that drm_vma_offset_remove()
  * is not called on this node concurrently.
@@ -215,7 +215,7 @@ static inline __u64 drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
 static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
 				      struct address_space *file_mapping)
 {
-	if (file_mapping && drm_vma_node_has_offset(node))
+	if (drm_vma_node_has_offset(node))
 		unmap_mapping_range(file_mapping,
 				    drm_vma_node_offset_addr(node),
 				    drm_vma_node_size(node) << PAGE_SHIFT, 1);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 8639c85..9261e42 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -738,6 +738,7 @@ extern int ttm_bo_device_release(struct ttm_bo_device *bdev);
  * @bdev: A pointer to a struct ttm_bo_device to initialize.
  * @glob: A pointer to an initialized struct ttm_bo_global.
  * @driver: A pointer to a struct ttm_bo_driver set up by the caller.
+ * @mapping: The address space to use for this bo.
  * @file_page_offset: Offset into the device address space that is available
  * for buffer data. This ensures compatibility with other users of the
  * address space.
@@ -749,6 +750,7 @@ extern int ttm_bo_device_release(struct ttm_bo_device *bdev);
 extern int ttm_bo_device_init(struct ttm_bo_device *bdev,
 			      struct ttm_bo_global *glob,
 			      struct ttm_bo_driver *driver,
+			      struct address_space *mapping,
 			      uint64_t file_page_offset, bool need_dma32);
 
 /**
-- 
1.8.3.4

[PATCH v2 4/4] drm/omap: remove useless if() in evict_entry()

[David Herrmann]

anon_inode->i_mapping is always set. This was needed before we introduced
anon_inodes. We can safely remove it now.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/omapdrm/omap_gem.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index 8219940..0f714a8 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -153,25 +153,23 @@ static struct {
 static void evict_entry(struct drm_gem_object *obj,
 		enum tiler_fmt fmt, struct usergart_entry *entry)
 {
-	if (obj->dev->anon_inode->i_mapping) {
-		struct omap_gem_object *omap_obj = to_omap_bo(obj);
-		int n = usergart[fmt].height;
-		size_t size = PAGE_SIZE * n;
-		loff_t off = mmap_offset(obj) +
-				(entry->obj_pgoff << PAGE_SHIFT);
-		const int m = 1 + ((omap_obj->width << fmt) / PAGE_SIZE);
-		if (m > 1) {
-			int i;
-			/* if stride > than PAGE_SIZE then sparse mapping: */
-			for (i = n; i > 0; i--) {
-				unmap_mapping_range(obj->dev->anon_inode->i_mapping,
-						off, PAGE_SIZE, 1);
-				off += PAGE_SIZE * m;
-			}
-		} else {
+	struct omap_gem_object *omap_obj = to_omap_bo(obj);
+	int n = usergart[fmt].height;
+	size_t size = PAGE_SIZE * n;
+	loff_t off = mmap_offset(obj) + (entry->obj_pgoff << PAGE_SHIFT);
+	const int m = 1 + ((omap_obj->width << fmt) / PAGE_SIZE);
+
+	if (m > 1) {
+		int i;
+		/* if stride > than PAGE_SIZE then sparse mapping: */
+		for (i = n; i > 0; i--) {
 			unmap_mapping_range(obj->dev->anon_inode->i_mapping,
-					    off, size, 1);
+					    off, PAGE_SIZE, 1);
+			off += PAGE_SIZE * m;
 		}
+	} else {
+		unmap_mapping_range(obj->dev->anon_inode->i_mapping,
+				    off, size, 1);
 	}
 
 	entry->obj = NULL;
-- 
1.8.3.4