linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
@ 2019-08-08 17:22 Chris Wilson
  2019-08-08 21:56 ` Hugh Dickins
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chris Wilson @ 2019-08-08 17:22 UTC (permalink / raw)
  To: intel-gfx
  Cc: linux-fsdevel, linux-kernel, Chris Wilson, Sergey Senozhatsky,
	Sedat Dilek, Hugh Dickins, Matthew Auld, Al Viro

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 <sergey.senozhatsky@gmail.com>
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
Suggested-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
  2019-08-08 17:22 [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint Chris Wilson
@ 2019-08-08 21:56 ` Hugh Dickins
  2019-08-09 18:47 ` [Intel-gfx] " Matthew Auld
  2019-08-12  7:11 ` Sergey Senozhatsky
  2 siblings, 0 replies; 7+ messages in thread
From: Hugh Dickins @ 2019-08-08 21:56 UTC (permalink / raw)
  To: Chris Wilson
  Cc: intel-gfx, linux-fsdevel, linux-kernel, Sergey Senozhatsky,
	Sedat Dilek, Hugh Dickins, Matthew Auld, Al Viro

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
  2019-08-08 17:22 [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint Chris Wilson
  2019-08-08 21:56 ` Hugh Dickins
@ 2019-08-09 18:47 ` Matthew Auld
  2019-08-09 18:52   ` Chris Wilson
  2019-08-12  7:11 ` Sergey Senozhatsky
  2 siblings, 1 reply; 7+ messages in thread
From: Matthew Auld @ 2019-08-09 18:47 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Intel Graphics Development, Sedat Dilek, Hugh Dickins,
	kernel list, Sergey Senozhatsky, Matthew Auld, linux-fsdevel,
	Al Viro

On Thu, 8 Aug 2019 at 18:23, Chris Wilson <chris@chris-wilson.co.uk> 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 <sergey.senozhatsky@gmail.com>
> Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> Suggested-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
  2019-08-09 18:47 ` [Intel-gfx] " Matthew Auld
@ 2019-08-09 18:52   ` Chris Wilson
  2019-08-09 20:11     ` Sedat Dilek
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2019-08-09 18:52 UTC (permalink / raw)
  To: Matthew Auld
  Cc: Intel Graphics Development, Sedat Dilek, Hugh Dickins,
	kernel list, Sergey Senozhatsky, Matthew Auld, linux-fsdevel,
	Al Viro

Quoting Matthew Auld (2019-08-09 19:47:02)
> On Thu, 8 Aug 2019 at 18:23, Chris Wilson <chris@chris-wilson.co.uk> 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 <sergey.senozhatsky@gmail.com>
> > Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> > Suggested-by: Hugh Dickins <hughd@google.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Thanks, picked up with the s/within/within_size/ fix.
-Chris

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
  2019-08-09 18:52   ` Chris Wilson
@ 2019-08-09 20:11     ` Sedat Dilek
  0 siblings, 0 replies; 7+ messages in thread
From: Sedat Dilek @ 2019-08-09 20:11 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Matthew Auld, Intel Graphics Development, Hugh Dickins,
	kernel list, Sergey Senozhatsky, Matthew Auld, linux-fsdevel,
	Al Viro

On Fri, Aug 9, 2019 at 8:52 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Matthew Auld (2019-08-09 19:47:02)
> > On Thu, 8 Aug 2019 at 18:23, Chris Wilson <chris@chris-wilson.co.uk> 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 <sergey.senozhatsky@gmail.com>
> > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com>
> > > Suggested-by: Hugh Dickins <hughd@google.com>
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Hugh Dickins <hughd@google.com>
> > > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > > Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
  2019-08-08 17:22 [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint Chris Wilson
  2019-08-08 21:56 ` Hugh Dickins
  2019-08-09 18:47 ` [Intel-gfx] " Matthew Auld
@ 2019-08-12  7:11 ` Sergey Senozhatsky
  2019-08-12  7:13   ` Chris Wilson
  2 siblings, 1 reply; 7+ messages in thread
From: Sergey Senozhatsky @ 2019-08-12  7:11 UTC (permalink / raw)
  To: Chris Wilson
  Cc: intel-gfx, linux-fsdevel, linux-kernel, Sergey Senozhatsky,
	Sedat Dilek, Hugh Dickins, Matthew Auld, Al Viro

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint
  2019-08-12  7:11 ` Sergey Senozhatsky
@ 2019-08-12  7:13   ` Chris Wilson
  0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2019-08-12  7:13 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: intel-gfx, linux-fsdevel, linux-kernel, Sergey Senozhatsky,
	Sedat Dilek, Hugh Dickins, Matthew Auld, Al Viro

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-08-12  7:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 17:22 [PATCH] drm/i915: Stop reconfiguring our shmemfs mountpoint Chris Wilson
2019-08-08 21:56 ` Hugh Dickins
2019-08-09 18:47 ` [Intel-gfx] " Matthew Auld
2019-08-09 18:52   ` Chris Wilson
2019-08-09 20:11     ` Sedat Dilek
2019-08-12  7:11 ` Sergey Senozhatsky
2019-08-12  7:13   ` Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).