On ke, 2017-05-31 at 19:51 +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.
>
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
<SNIP>
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2227,6 +2227,9 @@ struct drm_i915_private {
> DECLARE_HASHTABLE(mm_structs, 7);
> struct mutex mm_lock;
>
> + /* Our tmpfs instance used for shmem backed objects */
> + struct vfsmount *gemfs_mnt;
"gemfs" might be good enough, should not cause any confusion?
> @@ -4169,4 +4172,14 @@ static inline bool i915_gem_object_is_coherent(struct drm_i915_gem_object *obj)
> HAS_LLC(to_i915(obj->base.dev)));
> }
>
> +/* i915_gemfs.c */
i915_gemfs.h please. Lets not bloat i915_drv.h more when effort is in
place to strip it down.
> +struct vfsmount *i915_gemfs_create(void);
Not "int gemfs_init(struct drm_i915_privat *i915)" and _fini?
I doubt we should be creating more of these.
> @@ -4268,7 +4286,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_drm_gem_object_init(&dev_priv->drm, &obj->base, size);
> if (ret)
> goto fail;
As Chris mentioned, smells bit like we could be targeting DRM scope in
the future.
> @@ -4383,6 +4401,9 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
> drm_prime_gem_destroy(&obj->base, NULL);
>
> reservation_object_fini(&obj->__builtin_resv);
> +
For code below, do drop a note here that we want to do part of
drm_gem_object_release's work in advance. Or rather than commenting,
make it explicitly clear by having i915_gem_object_release() with this
hunk of code.
> + if (obj->base.filp)
> + i915_gemfs_unlink(obj->base.filp);
> drm_gem_object_release(&obj->base);
> i915_gem_info_remove_obj(i915, obj->base.size);
>
> @@ -4843,6 +4864,10 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
> {
> int err = -ENOMEM;
>
> + dev_priv->gemfs_mnt = i915_gemfs_create();
> + if (IS_ERR(dev_priv->gemfs_mnt))
> + return PTR_ERR(dev_priv->gemfs_mnt);
err = i915_gemfs_init(dev_priv);
if (err)
return err;
> +
> dev_priv->objects = KMEM_CACHE(drm_i915_gem_object, SLAB_HWCACHE_ALIGN);
> if (!dev_priv->objects)
err = -ENOMEM;
goto err_gemfs;
> @@ -4930,6 +4956,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_destroy(dev_priv->gemfs_mnt);
i915_gemfs_fini();
> +struct file *i915_gemfs_file_setup(struct vfsmount *gemfs_mnt,
> + const char *name, size_t size)
> +{
<SNIP>
> +
> + inode = d_inode(path.dentry);
> + inode->i_size = size;
> +
> + res = alloc_file(&path, FMODE_WRITE | FMODE_READ, inode->i_fop);
shmem is passing their own fops, we don't need to? shmem_mmap seems to
have some transparent huge page code at least which would be missed,
no?
> + if (IS_ERR(res))
> + goto unlink;
> +
> + return res;
> +
> +unlink:
> + dir->i_op->unlink(dir, path.dentry);
> +put_path:
> + path_put(&path);
Might throw newline here.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
