[PATCH 01/22] mm/shmem: support passing mnt to shmem_file_setup

[PATCH 01/22] mm/shmem: support passing mnt to shmem_file_setup

[Matthew Auld]

We are planning to use our own tmpfs mnt in i915 in place of the
shm_mnt such that we can control the mount options, in particular
huge=, which we require to support huge-gtt-pages. So rather than roll
our own version of __shmem_file_setup, it would be preferred if we could
just give shmem our mnt, and let it do the rest.

Based on the patch: https://patchwork.freedesktop.org/patch/178433/

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Arve Hjønnevåg" <arve@android.com>
Cc: Riley Andrews <riandrews@android.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mm@kvack.org
Cc: devel@driverdev.osuosl.org
---
 drivers/gpu/drm/drm_gem.c        |  2 +-
 drivers/gpu/drm/ttm/ttm_tt.c     |  2 +-
 drivers/staging/android/ashmem.c |  3 ++-
 include/linux/shmem_fs.h         |  4 +++-
 mm/shmem.c                       | 24 ++++++++++++++----------
 5 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 7199bba68c37..12e5fdc90a6f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -138,7 +138,7 @@ int drm_gem_object_init(struct drm_device *dev,
 
 	drm_gem_private_object_init(dev, obj, size);
 
-	filp = shmem_file_setup("drm mm object", size, VM_NORESERVE);
+	filp = shmem_file_setup(TMPFS_MNT, "drm mm object", size, VM_NORESERVE);
 	if (IS_ERR(filp))
 		return PTR_ERR(filp);
 
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 8ebc8d3560c3..4eef07d8b8ed 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -336,7 +336,7 @@ int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage)
 	BUG_ON(ttm->caching_state != tt_cached);
 
 	if (!persistent_swap_storage) {
-		swap_storage = shmem_file_setup("ttm swap",
+		swap_storage = shmem_file_setup(TMPFS_MNT, "ttm swap",
 						ttm->num_pages << PAGE_SHIFT,
 						0);
 		if (IS_ERR(swap_storage)) {
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 0f695df14c9d..184fcbb6d127 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -391,7 +391,8 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
 			name = asma->name;
 
 		/* ... and allocate the backing shmem file */
-		vmfile = shmem_file_setup(name, asma->size, vma->vm_flags);
+		vmfile = shmem_file_setup(TMPFS_MNT, name, asma->size,
+					  vma->vm_flags);
 		if (IS_ERR(vmfile)) {
 			ret = PTR_ERR(vmfile);
 			goto out;
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index b6c3540e07bc..00b199d804ca 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -44,12 +44,14 @@ static inline struct shmem_inode_info *SHMEM_I(struct inode *inode)
 	return container_of(inode, struct shmem_inode_info, vfs_inode);
 }
 
+#define TMPFS_MNT NULL
+
 /*
  * Functions in mm/shmem.c called directly from elsewhere:
  */
 extern int shmem_init(void);
 extern int shmem_fill_super(struct super_block *sb, void *data, int silent);
-extern struct file *shmem_file_setup(const char *name,
+extern struct file *shmem_file_setup(struct vfsmount *mnt, const char *name,
 					loff_t size, unsigned long flags);
 extern struct file *shmem_kernel_file_setup(const char *name, loff_t size,
 					    unsigned long flags);
diff --git a/mm/shmem.c b/mm/shmem.c
index 07a1d22807be..ae2e46291ffa 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3715,7 +3715,7 @@ SYSCALL_DEFINE2(memfd_create,
 					(flags >> MFD_HUGE_SHIFT) &
 					MFD_HUGE_MASK);
 	} else
-		file = shmem_file_setup(name, 0, VM_NORESERVE);
+		file = shmem_file_setup(TMPFS_MNT, name, 0, VM_NORESERVE);
 	if (IS_ERR(file)) {
 		error = PTR_ERR(file);
 		goto err_fd;
@@ -4183,7 +4183,7 @@ static const struct dentry_operations anon_ops = {
 	.d_dname = simple_dname
 };
 
-static struct file *__shmem_file_setup(const char *name, loff_t size,
+static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, loff_t size,
 				       unsigned long flags, unsigned int i_flags)
 {
 	struct file *res;
@@ -4192,8 +4192,11 @@ static struct file *__shmem_file_setup(const char *name, loff_t size,
 	struct super_block *sb;
 	struct qstr this;
 
-	if (IS_ERR(shm_mnt))
-		return ERR_CAST(shm_mnt);
+	if (mnt == TMPFS_MNT)
+		mnt = shm_mnt;
+
+	if (IS_ERR(mnt))
+		return ERR_CAST(mnt);
 
 	if (size < 0 || size > MAX_LFS_FILESIZE)
 		return ERR_PTR(-EINVAL);
@@ -4205,8 +4208,8 @@ static struct file *__shmem_file_setup(const char *name, loff_t size,
 	this.name = name;
 	this.len = strlen(name);
 	this.hash = 0; /* will go */
-	sb = shm_mnt->mnt_sb;
-	path.mnt = mntget(shm_mnt);
+	sb = mnt->mnt_sb;
+	path.mnt = mntget(mnt);
 	path.dentry = d_alloc_pseudo(sb, &this);
 	if (!path.dentry)
 		goto put_memory;
@@ -4251,18 +4254,19 @@ static struct file *__shmem_file_setup(const char *name, loff_t size,
  */
 struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(name, size, flags, S_PRIVATE);
+	return __shmem_file_setup(TMPFS_MNT, name, size, flags, S_PRIVATE);
 }
 
 /**
  * shmem_file_setup - get an unlinked file living in tmpfs
+ * @mnt: the tmpfs mount where the file will be created
  * @name: name for dentry (to be seen in /proc/<pid>/maps
  * @size: size to be set for the file
  * @flags: VM_NORESERVE suppresses pre-accounting of the entire object size
  */
-struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags)
+struct file *shmem_file_setup(struct vfsmount *mnt, const char *name, loff_t size, unsigned long flags)
 {
-	return __shmem_file_setup(name, size, flags, 0);
+	return __shmem_file_setup(mnt, name, size, flags, 0);
 }
 EXPORT_SYMBOL_GPL(shmem_file_setup);
 
@@ -4281,7 +4285,7 @@ int shmem_zero_setup(struct vm_area_struct *vma)
 	 * accessible to the user through its mapping, use S_PRIVATE flag to
 	 * bypass file security, in the same way as shmem_kernel_file_setup().
 	 */
-	file = __shmem_file_setup("dev/zero", size, vma->vm_flags, S_PRIVATE);
+	file = shmem_kernel_file_setup("dev/zero", size, vma->vm_flags);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-- 
2.13.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 02/22] drm/i915: introduce simple gemfs

[Matthew Auld]

Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
moves us away from the shmemfs shm_mnt, and gives us the much needed
flexibility to do things like set our own mount options, namely huge=
which should allow us to enable the use of transparent-huge-pages for
our shmem backed objects.

v2: various improvements suggested by Joonas

v3: move gemfs instance to i915.mm and simplify now that we have
file_setup_with_mnt

v4: fallback to tmpfs shm_mnt upon failure to setup gemfs

v5: make tmpfs fallback kinder

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Arve Hjønnevåg" <arve@android.com>
Cc: Riley Andrews <riandrews@android.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mm@kvack.org
Cc: devel@driverdev.osuosl.org
---
 drivers/gpu/drm/i915/Makefile                    |  1 +
 drivers/gpu/drm/i915/i915_drv.h                  |  5 +++
 drivers/gpu/drm/i915/i915_gem.c                  | 26 +++++++++++-
 drivers/gpu/drm/i915/i915_gemfs.c                | 52 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gemfs.h                | 34 ++++++++++++++++
 drivers/gpu/drm/i915/selftests/mock_gem_device.c |  4 ++
 6 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gemfs.c
 create mode 100644 drivers/gpu/drm/i915/i915_gemfs.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5182e3d5557d..980c41568f46 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -47,6 +47,7 @@ i915-y += i915_cmd_parser.o \
 	  i915_gem_tiling.o \
 	  i915_gem_timeline.o \
 	  i915_gem_userptr.o \
+	  i915_gemfs.o \
 	  i915_trace_points.o \
 	  i915_vma.o \
 	  intel_breadcrumbs.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 61a4be9c2d37..d2079c863a9f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1501,6 +1501,11 @@ struct i915_gem_mm {
 	/** Usable portion of the GTT for GEM */
 	dma_addr_t stolen_base; /* limited to low memory (32-bit) */
 
+	/**
+	 * tmpfs instance used for shmem backed objects
+	 */
+	struct vfsmount *gemfs;
+
 	/** PPGTT used for aliasing the PPGTT with the GTT */
 	struct i915_hw_ppgtt *aliasing_ppgtt;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73eeb6b1f1cd..26a3ad2668d7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -35,6 +35,7 @@
 #include "intel_drv.h"
 #include "intel_frontbuffer.h"
 #include "intel_mocs.h"
+#include "i915_gemfs.h"
 #include <linux/dma-fence-array.h>
 #include <linux/kthread.h>
 #include <linux/reservation.h>
@@ -4251,6 +4252,24 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
 	.pwrite = i915_gem_object_pwrite_gtt,
 };
 
+static int i915_gem_object_create_shmem(struct drm_device *dev,
+					struct drm_gem_object *obj,
+					size_t size)
+{
+	struct drm_i915_private *i915 = to_i915(dev);
+	struct file *filp;
+
+	drm_gem_private_object_init(dev, obj, size);
+
+	filp = shmem_file_setup(i915->mm.gemfs, "i915", size, VM_NORESERVE);
+	if (IS_ERR(filp))
+		return PTR_ERR(filp);
+
+	obj->filp = filp;
+
+	return 0;
+}
+
 struct drm_i915_gem_object *
 i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size)
 {
@@ -4275,7 +4294,7 @@ i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size)
 	if (obj == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	ret = drm_gem_object_init(&dev_priv->drm, &obj->base, size);
+	ret = i915_gem_object_create_shmem(&dev_priv->drm, &obj->base, size);
 	if (ret)
 		goto fail;
 
@@ -4914,6 +4933,9 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
 
 	spin_lock_init(&dev_priv->fb_tracking.lock);
 
+	if (i915_gemfs_init(dev_priv))
+		DRM_NOTE("Unable to create a private tmpfs mountpoint, hugepage support will be disabled.\n");
+
 	return 0;
 
 err_priorities:
@@ -4952,6 +4974,8 @@ void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
 
 	/* And ensure that our DESTROY_BY_RCU slabs are truly destroyed */
 	rcu_barrier();
+
+	i915_gemfs_fini(dev_priv);
 }
 
 int i915_gem_freeze(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_gemfs.c b/drivers/gpu/drm/i915/i915_gemfs.c
new file mode 100644
index 000000000000..168d0bd98f60
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gemfs.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <linux/fs.h>
+#include <linux/mount.h>
+
+#include "i915_drv.h"
+#include "i915_gemfs.h"
+
+int i915_gemfs_init(struct drm_i915_private *i915)
+{
+	struct file_system_type *type;
+	struct vfsmount *gemfs;
+
+	type = get_fs_type("tmpfs");
+	if (!type)
+		return -ENODEV;
+
+	gemfs = kern_mount(type);
+	if (IS_ERR(gemfs))
+		return PTR_ERR(gemfs);
+
+	i915->mm.gemfs = gemfs;
+
+	return 0;
+}
+
+void i915_gemfs_fini(struct drm_i915_private *i915)
+{
+	kern_unmount(i915->mm.gemfs);
+}
diff --git a/drivers/gpu/drm/i915/i915_gemfs.h b/drivers/gpu/drm/i915/i915_gemfs.h
new file mode 100644
index 000000000000..cca8bdc5b93e
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gemfs.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __I915_GEMFS_H__
+#define __I915_GEMFS_H__
+
+struct drm_i915_private;
+
+int i915_gemfs_init(struct drm_i915_private *i915);
+
+void i915_gemfs_fini(struct drm_i915_private *i915);
+
+#endif
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 2388424a14da..ed3407b078e7 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -83,6 +83,8 @@ static void mock_device_release(struct drm_device *dev)
 	kmem_cache_destroy(i915->vmas);
 	kmem_cache_destroy(i915->objects);
 
+	i915_gemfs_fini(i915);
+
 	drm_dev_fini(&i915->drm);
 	put_device(&i915->drm.pdev->dev);
 }
@@ -239,6 +241,8 @@ struct drm_i915_private *mock_gem_device(void)
 	if (!i915->kernel_context)
 		goto err_engine;
 
+	WARN_ON(i915_gemfs_init(i915));
+
 	return i915;
 
 err_engine:
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Greg Kroah-Hartman]

On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> moves us away from the shmemfs shm_mnt, and gives us the much needed
> flexibility to do things like set our own mount options, namely huge=
> which should allow us to enable the use of transparent-huge-pages for
> our shmem backed objects.
> 
> v2: various improvements suggested by Joonas
> 
> v3: move gemfs instance to i915.mm and simplify now that we have
> file_setup_with_mnt
> 
> v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> 
> v5: make tmpfs fallback kinder

Why do this only for one specific driver?  Shouldn't the drm core handle
this for you, for all other drivers as well?  Otherwise trying to figure
out how to "contain" this type of thing is going to be a pain (mount
options, selinux options, etc.)

thanks,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Joonas Lahtinen]

On Tue, 2017-09-26 at 09:52 +0200, Greg Kroah-Hartman wrote:
> On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> > moves us away from the shmemfs shm_mnt, and gives us the much needed
> > flexibility to do things like set our own mount options, namely huge=
> > which should allow us to enable the use of transparent-huge-pages for
> > our shmem backed objects.
> > 
> > v2: various improvements suggested by Joonas
> > 
> > v3: move gemfs instance to i915.mm and simplify now that we have
> > file_setup_with_mnt
> > 
> > v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> > 
> > v5: make tmpfs fallback kinder
> 
> Why do this only for one specific driver?  Shouldn't the drm core handle
> this for you, for all other drivers as well?  Otherwise trying to figure
> out how to "contain" this type of thing is going to be a pain (mount
> options, selinux options, etc.)

We actually started quite grande by making stripped down version of
shmemfs for drm core, but kept running into nacks about how we were
implementing it (after getting a recommendation to try implementing it
some way). After a few iterations and massive engineering time, we have
been progressively reducing the amount of changes outside i915 in the
hopes to get this merged.

And all the while clock is ticking, so we thought the best way to get
something to support our future work is to implement this first locally
with minimal external changes outside i915 and then once we have
something working, it'll be easier to generalize it for the drm core.
Otherwise we'll never get to work with the huge page support, for which
gemfs is the stepping stone here.

So we're not planning on sitting on top of it, we'll just incubate it
under i915/ so that it'll then be less pain for others to adopt when
the biggest hurdles with core MM interactions are sorted out.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Greg Kroah-Hartman]

On Tue, Sep 26, 2017 at 04:21:47PM +0300, Joonas Lahtinen wrote:
> On Tue, 2017-09-26 at 09:52 +0200, Greg Kroah-Hartman wrote:
> > On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> > > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> > > moves us away from the shmemfs shm_mnt, and gives us the much needed
> > > flexibility to do things like set our own mount options, namely huge=
> > > which should allow us to enable the use of transparent-huge-pages for
> > > our shmem backed objects.
> > > 
> > > v2: various improvements suggested by Joonas
> > > 
> > > v3: move gemfs instance to i915.mm and simplify now that we have
> > > file_setup_with_mnt
> > > 
> > > v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> > > 
> > > v5: make tmpfs fallback kinder
> > 
> > Why do this only for one specific driver?  Shouldn't the drm core handle
> > this for you, for all other drivers as well?  Otherwise trying to figure
> > out how to "contain" this type of thing is going to be a pain (mount
> > options, selinux options, etc.)
> 
> We actually started quite grande by making stripped down version of
> shmemfs for drm core, but kept running into nacks about how we were
> implementing it (after getting a recommendation to try implementing it
> some way). After a few iterations and massive engineering time, we have
> been progressively reducing the amount of changes outside i915 in the
> hopes to get this merged.
> 
> And all the while clock is ticking, so we thought the best way to get
> something to support our future work is to implement this first locally
> with minimal external changes outside i915 and then once we have
> something working, it'll be easier to generalize it for the drm core.
> Otherwise we'll never get to work with the huge page support, for which
> gemfs is the stepping stone here.
> 
> So we're not planning on sitting on top of it, we'll just incubate it
> under i915/ so that it'll then be less pain for others to adopt when
> the biggest hurdles with core MM interactions are sorted out.

But by doing this, you are now creating a new user/kernel api that you
have to support for forever, right?  Will it not change if you make it
"generic" to the drm core eventually?

Worse case, name it a generic name that everyone will end up using in
the future, and then you can just claim that all other drivers need to
implement it :)

thanks,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Joonas Lahtinen]

On Tue, 2017-09-26 at 23:34 +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 26, 2017 at 04:21:47PM +0300, Joonas Lahtinen wrote:
> > On Tue, 2017-09-26 at 09:52 +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> > > > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> > > > moves us away from the shmemfs shm_mnt, and gives us the much needed
> > > > flexibility to do things like set our own mount options, namely huge=
> > > > which should allow us to enable the use of transparent-huge-pages for
> > > > our shmem backed objects.
> > > > 
> > > > v2: various improvements suggested by Joonas
> > > > 
> > > > v3: move gemfs instance to i915.mm and simplify now that we have
> > > > file_setup_with_mnt
> > > > 
> > > > v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> > > > 
> > > > v5: make tmpfs fallback kinder
> > > 
> > > Why do this only for one specific driver?  Shouldn't the drm core handle
> > > this for you, for all other drivers as well?  Otherwise trying to figure
> > > out how to "contain" this type of thing is going to be a pain (mount
> > > options, selinux options, etc.)
> > 
> > We actually started quite grande by making stripped down version of
> > shmemfs for drm core, but kept running into nacks about how we were
> > implementing it (after getting a recommendation to try implementing it
> > some way). After a few iterations and massive engineering time, we have
> > been progressively reducing the amount of changes outside i915 in the
> > hopes to get this merged.
> > 
> > And all the while clock is ticking, so we thought the best way to get
> > something to support our future work is to implement this first locally
> > with minimal external changes outside i915 and then once we have
> > something working, it'll be easier to generalize it for the drm core.
> > Otherwise we'll never get to work with the huge page support, for which
> > gemfs is the stepping stone here.
> > 
> > So we're not planning on sitting on top of it, we'll just incubate it
> > under i915/ so that it'll then be less pain for others to adopt when
> > the biggest hurdles with core MM interactions are sorted out.
> 
> But by doing this, you are now creating a new user/kernel api that you
> have to support for forever, right?  Will it not change if you make it
> "generic" to the drm core eventually?

Nope, this series is actually just for the driver to get some THPs,
regardless of whether user asked for them or not. It's an opportunistic
feature in this form, no new API introduced. We will also take
advantage if we happen to get 4-order pages (64KB).

What comes to the API anyway, the differences between each GPU driver
are big enough that we each have our own GEM buffer create IOCTLs for
example (I915_GEM_CREATE for i915). And then those are internally
calling DRM core functions which do bulk of the work with the backing
storage. So if we provide an interface for the user to enforce getting
huge pages, we'll simply have our own bit in the IOCTL which will then
be translated to some DRM core flag or function call.

> Worse case, name it a generic name that everyone will end up using in
> the future, and then you can just claim that all other drivers need to
> implement it :)

"gem" is the DRM core memory manager (well, the other of them), so
"gemfs" is not an accidental name :) We're definitely driving it there.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx