The filesystem reconfigure API is undergoing a transition, breaking our
current code. As we only set the default options, we can simply remove
the call to s_op->remount_fs(). In the future, when HW permits, we can
try re-enabling huge page support, albeit as suggested with new per-file
controls.
Reported-by: Sergey Senozhatsky <[email protected]>
Reported-by: Sedat Dilek <[email protected]>
Suggested-by: Hugh Dickins <[email protected]>
Signed-off-by: Chris Wilson <[email protected]>
Cc: Matthew Auld <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
---
drivers/gpu/drm/i915/gem/i915_gemfs.c | 31 ++++++++-------------------
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
index 099f3397aada..be94598cb304 100644
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
@@ -20,31 +20,18 @@ int i915_gemfs_init(struct drm_i915_private *i915)
if (!type)
return -ENODEV;
- gemfs = kern_mount(type);
- if (IS_ERR(gemfs))
- return PTR_ERR(gemfs);
-
/*
- * Enable huge-pages for objects that are at least HPAGE_PMD_SIZE, most
- * likely 2M. Note that within_size may overallocate huge-pages, if say
- * we allocate an object of size 2M + 4K, we may get 2M + 2M, but under
- * memory pressure shmem should split any huge-pages which can be
- * shrunk.
+ * By creating our own shmemfs mountpoint, we can pass in
+ * mount flags that better match our usecase.
+ *
+ * One example, although it is probably better with a per-file
+ * control, is selecting huge page allocations ("huge=within").
+ * Currently unused due to bandwidth issues (slow reads) on Broadwell+.
*/
- if (has_transparent_hugepage()) {
- struct super_block *sb = gemfs->mnt_sb;
- /* FIXME: Disabled until we get W/A for read BW issue. */
- char options[] = "huge=never";
- int flags = 0;
- int err;
-
- err = sb->s_op->remount_fs(sb, &flags, options);
- if (err) {
- kern_unmount(gemfs);
- return err;
- }
- }
+ gemfs = kern_mount(type);
+ if (IS_ERR(gemfs))
+ return PTR_ERR(gemfs);
i915->mm.gemfs = gemfs;
--
2.23.0.rc1
On Thu, 8 Aug 2019, Chris Wilson wrote:
> + * By creating our own shmemfs mountpoint, we can pass in
> + * mount flags that better match our usecase.
> + *
> + * One example, although it is probably better with a per-file
> + * control, is selecting huge page allocations ("huge=within").
s/within/within_size/
Not that either of us is recommending that direction,
but since it's mentioned, better to give the correct name.
> + * Currently unused due to bandwidth issues (slow reads) on Broadwell+.
Thanks,
Hugh
On Thu, 8 Aug 2019 at 18:23, Chris Wilson <[email protected]> wrote:
>
> The filesystem reconfigure API is undergoing a transition, breaking our
> current code. As we only set the default options, we can simply remove
> the call to s_op->remount_fs(). In the future, when HW permits, we can
> try re-enabling huge page support, albeit as suggested with new per-file
> controls.
>
> Reported-by: Sergey Senozhatsky <[email protected]>
> Reported-by: Sedat Dilek <[email protected]>
> Suggested-by: Hugh Dickins <[email protected]>
> Signed-off-by: Chris Wilson <[email protected]>
> Cc: Matthew Auld <[email protected]>
> Cc: Hugh Dickins <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Sergey Senozhatsky <[email protected]>
Reviewed-by: Matthew Auld <[email protected]>
Quoting Matthew Auld (2019-08-09 19:47:02)
> On Thu, 8 Aug 2019 at 18:23, Chris Wilson <[email protected]> wrote:
> >
> > The filesystem reconfigure API is undergoing a transition, breaking our
> > current code. As we only set the default options, we can simply remove
> > the call to s_op->remount_fs(). In the future, when HW permits, we can
> > try re-enabling huge page support, albeit as suggested with new per-file
> > controls.
> >
> > Reported-by: Sergey Senozhatsky <[email protected]>
> > Reported-by: Sedat Dilek <[email protected]>
> > Suggested-by: Hugh Dickins <[email protected]>
> > Signed-off-by: Chris Wilson <[email protected]>
> > Cc: Matthew Auld <[email protected]>
> > Cc: Hugh Dickins <[email protected]>
> > Cc: Al Viro <[email protected]>
> > Cc: Sergey Senozhatsky <[email protected]>
> Reviewed-by: Matthew Auld <[email protected]>
Thanks, picked up with the s/within/within_size/ fix.
-Chris
On Fri, Aug 9, 2019 at 8:52 PM Chris Wilson <[email protected]> wrote:
>
> Quoting Matthew Auld (2019-08-09 19:47:02)
> > On Thu, 8 Aug 2019 at 18:23, Chris Wilson <[email protected]> wrote:
> > >
> > > The filesystem reconfigure API is undergoing a transition, breaking our
> > > current code. As we only set the default options, we can simply remove
> > > the call to s_op->remount_fs(). In the future, when HW permits, we can
> > > try re-enabling huge page support, albeit as suggested with new per-file
> > > controls.
> > >
> > > Reported-by: Sergey Senozhatsky <[email protected]>
> > > Reported-by: Sedat Dilek <[email protected]>
> > > Suggested-by: Hugh Dickins <[email protected]>
> > > Signed-off-by: Chris Wilson <[email protected]>
> > > Cc: Matthew Auld <[email protected]>
> > > Cc: Hugh Dickins <[email protected]>
> > > Cc: Al Viro <[email protected]>
> > > Cc: Sergey Senozhatsky <[email protected]>
> > Reviewed-by: Matthew Auld <[email protected]>
>
> Thanks, picked up with the s/within/within_size/ fix.
> -Chris
For the records and followers:
[1] https://cgit.freedesktop.org/drm-intel/commit/?h=for-linux-next&id=72e67f04637432f91e4cc5e8e4f7eb4e38461e8e
On (08/08/19 18:22), Chris Wilson wrote:
[..]
> @@ -20,31 +20,18 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> if (!type)
> return -ENODEV;
[..]
> + gemfs = kern_mount(type);
> + if (IS_ERR(gemfs))
> + return PTR_ERR(gemfs);
>
> i915->mm.gemfs = gemfs;
We still have to put_filesystem(). Right?
-ss
Quoting Sergey Senozhatsky (2019-08-12 08:11:48)
> On (08/08/19 18:22), Chris Wilson wrote:
> [..]
> > @@ -20,31 +20,18 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> > if (!type)
> > return -ENODEV;
> [..]
> > + gemfs = kern_mount(type);
> > + if (IS_ERR(gemfs))
> > + return PTR_ERR(gemfs);
> >
> > i915->mm.gemfs = gemfs;
>
> We still have to put_filesystem(). Right?
Yes. We still your patches for EXPORT_SYMBOL.
-Chris