* [PATCH 1/2] i915: convert to new mount API
@ 2019-08-02 12:39 Sergey Senozhatsky
2019-08-02 12:39 ` [PATCH 2/2] i915: do not leak module ref counter Sergey Senozhatsky
2019-08-02 12:54 ` [PATCH 1/2] i915: convert to new mount API Chris Wilson
0 siblings, 2 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2019-08-02 12:39 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Chris Wilson
Cc: David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
tmpfs does not set ->remount_fs() anymore and its users need
to be converted to new mount API.
BUG: kernel NULL pointer dereference, address: 0000000000000000
PF: supervisor instruction fetch in kernel mode
PF: error_code(0x0010) - not-present page
RIP: 0010:0x0
Code: Bad RIP value.
Call Trace:
i915_gemfs_init+0x6e/0xa0 [i915]
i915_gem_init_early+0x76/0x90 [i915]
i915_driver_probe+0x30a/0x1640 [i915]
? kernfs_activate+0x5a/0x80
? kernfs_add_one+0xdd/0x130
pci_device_probe+0x9e/0x110
really_probe+0xce/0x230
driver_probe_device+0x4b/0xc0
device_driver_attach+0x4e/0x60
__driver_attach+0x47/0xb0
? device_driver_attach+0x60/0x60
bus_for_each_dev+0x61/0x90
bus_add_driver+0x167/0x1b0
driver_register+0x67/0xaa
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/gpu/drm/i915/gem/i915_gemfs.c | 28 ++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
index 099f3397aada..cf05ba72df9d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
@@ -7,14 +7,17 @@
#include <linux/fs.h>
#include <linux/mount.h>
#include <linux/pagemap.h>
+#include <linux/fs_context.h>
#include "i915_drv.h"
#include "i915_gemfs.h"
int i915_gemfs_init(struct drm_i915_private *i915)
{
+ struct fs_context *fc = NULL;
struct file_system_type *type;
struct vfsmount *gemfs;
+ bool ok = true;
type = get_fs_type("tmpfs");
if (!type)
@@ -36,18 +39,29 @@ int i915_gemfs_init(struct drm_i915_private *i915)
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;
+ fc = fs_context_for_reconfigure(sb->s_root, 0, 0);
+ if (IS_ERR(fc)) {
+ ok = false;
+ goto out;
}
+
+ if (!fc->ops->parse_monolithic ||
+ fc->ops->parse_monolithic(fc, options)) {
+ ok = false;
+ goto out;
+ }
+
+ if (!fc->ops->reconfigure || fc->ops->reconfigure(fc))
+ ok = false;
}
+out:
+ if (!ok)
+ pr_err("i915 gemfs reconfiguration failed\n");
+ if (!IS_ERR_OR_NULL(fc))
+ put_fs_context(fc);
i915->mm.gemfs = gemfs;
-
return 0;
}
--
2.22.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 12:39 [PATCH 1/2] i915: convert to new mount API Sergey Senozhatsky
@ 2019-08-02 12:39 ` Sergey Senozhatsky
2019-08-02 12:58 ` Chris Wilson
2019-08-02 12:54 ` [PATCH 1/2] i915: convert to new mount API Chris Wilson
1 sibling, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2019-08-02 12:39 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Chris Wilson
Cc: David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
put_filesystem() before i915_gemfs_init() deals with
kern_mount() error.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/gpu/drm/i915/gem/i915_gemfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
index cf05ba72df9d..d437188d1736 100644
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
@@ -24,8 +24,10 @@ int i915_gemfs_init(struct drm_i915_private *i915)
return -ENODEV;
gemfs = kern_mount(type);
- if (IS_ERR(gemfs))
+ if (IS_ERR(gemfs)) {
+ put_filesystem(type);
return PTR_ERR(gemfs);
+ }
/*
* Enable huge-pages for objects that are at least HPAGE_PMD_SIZE, most
--
2.22.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] i915: convert to new mount API
2019-08-02 12:39 [PATCH 1/2] i915: convert to new mount API Sergey Senozhatsky
2019-08-02 12:39 ` [PATCH 2/2] i915: do not leak module ref counter Sergey Senozhatsky
@ 2019-08-02 12:54 ` Chris Wilson
2019-08-02 13:10 ` Sergey Senozhatsky
1 sibling, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-08-02 12:54 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Sergey Senozhatsky
Cc: David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
Quoting Sergey Senozhatsky (2019-08-02 13:39:55)
> tmpfs does not set ->remount_fs() anymore and its users need
> to be converted to new mount API.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> PF: supervisor instruction fetch in kernel mode
> PF: error_code(0x0010) - not-present page
> RIP: 0010:0x0
> Code: Bad RIP value.
> Call Trace:
> i915_gemfs_init+0x6e/0xa0 [i915]
> i915_gem_init_early+0x76/0x90 [i915]
> i915_driver_probe+0x30a/0x1640 [i915]
> ? kernfs_activate+0x5a/0x80
> ? kernfs_add_one+0xdd/0x130
> pci_device_probe+0x9e/0x110
> really_probe+0xce/0x230
> driver_probe_device+0x4b/0xc0
> device_driver_attach+0x4e/0x60
> __driver_attach+0x47/0xb0
> ? device_driver_attach+0x60/0x60
> bus_for_each_dev+0x61/0x90
> bus_add_driver+0x167/0x1b0
> driver_register+0x67/0xaa
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gemfs.c | 28 ++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> index 099f3397aada..cf05ba72df9d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> @@ -7,14 +7,17 @@
> #include <linux/fs.h>
> #include <linux/mount.h>
> #include <linux/pagemap.h>
> +#include <linux/fs_context.h>
>
> #include "i915_drv.h"
> #include "i915_gemfs.h"
>
> int i915_gemfs_init(struct drm_i915_private *i915)
> {
> + struct fs_context *fc = NULL;
> struct file_system_type *type;
> struct vfsmount *gemfs;
> + bool ok = true;
Start with ok = false, we only need to set to true if we succeed in
reconfiguring.
> type = get_fs_type("tmpfs");
> if (!type)
> @@ -36,18 +39,29 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> 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;
Hmm, we could avoid this if we used vfs_kernel_mount() directly rather
than the kern_mount wrapper, as then we pass options through to
parse_monotithic_mount_data(). Or am I barking up the wrong tree?
>
> - err = sb->s_op->remount_fs(sb, &flags, options);
> - if (err) {
> - kern_unmount(gemfs);
> - return err;
> + fc = fs_context_for_reconfigure(sb->s_root, 0, 0);
> + if (IS_ERR(fc)) {
> + ok = false;
> + goto out;
> }
> +
> + if (!fc->ops->parse_monolithic ||
> + fc->ops->parse_monolithic(fc, options)) {
checkpatch.pl will complain that this should line up with the '('
> + ok = false;
> + goto out;
> + }
> +
> + if (!fc->ops->reconfigure || fc->ops->reconfigure(fc))
> + ok = false;
> }
>
> +out:
> + if (!ok)
> + pr_err("i915 gemfs reconfiguration failed\n");
Let's make it a bit more user friendly,
dev_err(i915->drm.dev,
"Unable to reconfigure internal shmemfs for preferred"
" allocation strategy; continuing, but performance may suffer.\n");
Assuming that we can't just use vfs_kern_mount() instead, with the nits
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 12:39 ` [PATCH 2/2] i915: do not leak module ref counter Sergey Senozhatsky
@ 2019-08-02 12:58 ` Chris Wilson
2019-08-02 13:10 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-08-02 12:58 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Sergey Senozhatsky
Cc: David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
Quoting Sergey Senozhatsky (2019-08-02 13:39:56)
> put_filesystem() before i915_gemfs_init() deals with
> kern_mount() error.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gemfs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> index cf05ba72df9d..d437188d1736 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> @@ -24,8 +24,10 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> return -ENODEV;
>
> gemfs = kern_mount(type);
Looking around, it looks like we always need to drop type after
mounting. Should the
put_filesystem(type);
be here instead?
Anyway, nice catch.
> - if (IS_ERR(gemfs))
> + if (IS_ERR(gemfs)) {
> + put_filesystem(type);
> return PTR_ERR(gemfs);
> + }
>
> /*
> * Enable huge-pages for objects that are at least HPAGE_PMD_SIZE, most
> --
> 2.22.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 12:58 ` Chris Wilson
@ 2019-08-02 13:10 ` Chris Wilson
2019-08-02 13:15 ` Sergey Senozhatsky
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-08-02 13:10 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Sergey Senozhatsky
Cc: David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
Quoting Chris Wilson (2019-08-02 13:58:36)
> Quoting Sergey Senozhatsky (2019-08-02 13:39:56)
> > put_filesystem() before i915_gemfs_init() deals with
> > kern_mount() error.
> >
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gemfs.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> > index cf05ba72df9d..d437188d1736 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> > @@ -24,8 +24,10 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> > return -ENODEV;
> >
> > gemfs = kern_mount(type);
>
> Looking around, it looks like we always need to drop type after
> mounting. Should the
> put_filesystem(type);
> be here instead?
>
> Anyway, nice catch.
Sigh. put_filesystem() is part of fs internals. I'd be tempted to add
static void put_filesystem(struct file_system_type *fs)
{
module_put(fs->owner);
}
and cc that for stable. And send a patch to add EXPORT_SYMBOL and queue
it for removal. Or just ignore the stable@ since it's unlikely to be
ever met in the wild and just request the EXPORT_SYMBOL.
-Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] i915: convert to new mount API
2019-08-02 12:54 ` [PATCH 1/2] i915: convert to new mount API Chris Wilson
@ 2019-08-02 13:10 ` Sergey Senozhatsky
0 siblings, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2019-08-02 13:10 UTC (permalink / raw)
To: Chris Wilson
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Sergey Senozhatsky,
David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel
On (08/02/19 13:54), Chris Wilson wrote:
[..]
> > int i915_gemfs_init(struct drm_i915_private *i915)
> > {
> > + struct fs_context *fc = NULL;
> > struct file_system_type *type;
> > struct vfsmount *gemfs;
> > + bool ok = true;
>
> Start with ok = false, we only need to set to true if we succeed in
> reconfiguring.
OK, makes sense.
> > type = get_fs_type("tmpfs");
> > if (!type)
> > @@ -36,18 +39,29 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> > 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;
>
> Hmm, we could avoid this if we used vfs_kernel_mount() directly rather
> than the kern_mount wrapper, as then we pass options through to
> parse_monotithic_mount_data(). Or am I barking up the wrong tree?
Hmm.
Wouldn't this error on !TRANSPARENT_HUGE_PAGECACHE systems?
"huge=never" should be an invalid option when system does
not know about THP.
[..]
> > + if (!fc->ops->parse_monolithic ||
> > + fc->ops->parse_monolithic(fc, options)) {
>
> checkpatch.pl will complain that this should line up with the '('
It doesn't.
-------------------------------------------------
outgoing/0001-i915-convert-to-new-mount-API.patch
-------------------------------------------------
total: 0 errors, 0 warnings, 53 lines checked
outgoing/0001-i915-convert-to-new-mount-API.patch has no obvious style problems and is ready for submission.
-------------------------------------------------------
outgoing/0002-i915-do-not-leak-module-ref-counter.patch
-------------------------------------------------------
total: 0 errors, 0 warnings, 11 lines checked
outgoing/0002-i915-do-not-leak-module-ref-counter.patch has no obvious style problems and is ready for submission.
[..]
> > + if (!ok)
> > + pr_err("i915 gemfs reconfiguration failed\n");
>
> Let's make it a bit more user friendly,
>
> dev_err(i915->drm.dev,
> "Unable to reconfigure internal shmemfs for preferred"
> " allocation strategy; continuing, but performance may suffer.\n");
I guess now checkpatch will complain :)
> Assuming that we can't just use vfs_kern_mount() instead, with the nits
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Thanks.
I'll sit on it for several days, just to see if more feedback will come.
-ss
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 13:10 ` Chris Wilson
@ 2019-08-02 13:15 ` Sergey Senozhatsky
2019-08-02 13:35 ` Sergey Senozhatsky
0 siblings, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2019-08-02 13:15 UTC (permalink / raw)
To: Chris Wilson
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Sergey Senozhatsky,
David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel
On (08/02/19 14:10), Chris Wilson wrote:
> > > drivers/gpu/drm/i915/gem/i915_gemfs.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> > > index cf05ba72df9d..d437188d1736 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> > > @@ -24,8 +24,10 @@ int i915_gemfs_init(struct drm_i915_private *i915)
> > > return -ENODEV;
> > >
> > > gemfs = kern_mount(type);
> >
> > Looking around, it looks like we always need to drop type after
> > mounting. Should the
> > put_filesystem(type);
> > be here instead?
> >
> > Anyway, nice catch.
>
> Sigh. put_filesystem() is part of fs internals. I'd be tempted to add
Good catch!
So we can switch to vfs_kern_mount(), I guess, but pass different options,
depending on has_transparent_hugepage().
-ss
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 13:15 ` Sergey Senozhatsky
@ 2019-08-02 13:35 ` Sergey Senozhatsky
2019-08-02 13:41 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2019-08-02 13:35 UTC (permalink / raw)
To: Chris Wilson
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
On (08/02/19 22:15), Sergey Senozhatsky wrote:
[..]
> > > Looking around, it looks like we always need to drop type after
> > > mounting. Should the
> > > put_filesystem(type);
> > > be here instead?
> > >
> > > Anyway, nice catch.
> >
> > Sigh. put_filesystem() is part of fs internals. I'd be tempted to add
>
> Good catch!
>
> So we can switch to vfs_kern_mount(), I guess, but pass different options,
> depending on has_transparent_hugepage().
Hmm. This doesn't look exactly right. It appears that vfs_kern_mount()
has a slightly different purpose. It's for drivers which register their
own fstype and fs_context/sb callbacks. A typical usage would be
static struct file_system_type nfsd_fs_type = {
.owner→ → = THIS_MODULE,
.name→ → = "nfsd",
.init_fs_context = nfsd_init_fs_context,
.kill_sb→ = nfsd_umount,
};
MODULE_ALIAS_FS("nfsd");
vfs_kern_mount(&nfsd_fs_type, SB_KERNMOUNT, "nfsd", NULL);
i915 is a different beast, it just wants to mount fs and reconfigure
it, it doesn't want to be an fs. So it seems that current kern_mount()
is actually right.
Maybe we need to export put_filesystem() instead.
-ss
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 13:35 ` Sergey Senozhatsky
@ 2019-08-02 13:41 ` Chris Wilson
2019-08-02 13:49 ` Sergey Senozhatsky
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-08-02 13:41 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, intel-gfx, dri-devel, linux-kernel,
Sergey Senozhatsky
Quoting Sergey Senozhatsky (2019-08-02 14:35:03)
> On (08/02/19 22:15), Sergey Senozhatsky wrote:
> [..]
> > > > Looking around, it looks like we always need to drop type after
> > > > mounting. Should the
> > > > put_filesystem(type);
> > > > be here instead?
> > > >
> > > > Anyway, nice catch.
> > >
> > > Sigh. put_filesystem() is part of fs internals. I'd be tempted to add
> >
> > Good catch!
> >
> > So we can switch to vfs_kern_mount(), I guess, but pass different options,
> > depending on has_transparent_hugepage().
>
> Hmm. This doesn't look exactly right. It appears that vfs_kern_mount()
> has a slightly different purpose. It's for drivers which register their
> own fstype and fs_context/sb callbacks. A typical usage would be
>
> static struct file_system_type nfsd_fs_type = {
> .owner→ → = THIS_MODULE,
> .name→ → = "nfsd",
> .init_fs_context = nfsd_init_fs_context,
> .kill_sb→ = nfsd_umount,
> };
> MODULE_ALIAS_FS("nfsd");
>
> vfs_kern_mount(&nfsd_fs_type, SB_KERNMOUNT, "nfsd", NULL);
>
> i915 is a different beast, it just wants to mount fs and reconfigure
> it, it doesn't want to be an fs. So it seems that current kern_mount()
> is actually right.
struct vfsmount *kern_mount(struct file_system_type *type)
{
struct vfsmount *mnt;
mnt = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
if (!IS_ERR(mnt)) {
/*
* it is a longterm mount, don't release mnt until
* we unmount before file sys is unregistered
*/
real_mount(mnt)->mnt_ns = MNT_NS_INTERNAL;
}
return mnt;
}
With the exception of fiddling with MNT_NS_INTERNAL, it seems
amenable for our needs.
-Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 13:41 ` Chris Wilson
@ 2019-08-02 13:49 ` Sergey Senozhatsky
2019-08-02 13:59 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Sergey Senozhatsky @ 2019-08-02 13:49 UTC (permalink / raw)
To: Chris Wilson
Cc: Sergey Senozhatsky, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel
On (08/02/19 14:41), Chris Wilson wrote:
[..]
> struct vfsmount *kern_mount(struct file_system_type *type)
> {
> struct vfsmount *mnt;
> mnt = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
> if (!IS_ERR(mnt)) {
> /*
> * it is a longterm mount, don't release mnt until
> * we unmount before file sys is unregistered
> */
> real_mount(mnt)->mnt_ns = MNT_NS_INTERNAL;
> }
> return mnt;
> }
>
> With the exception of fiddling with MNT_NS_INTERNAL, it seems
> amenable for our needs.
Sorry, not sure I understand. i915 use kern_mount() at the moment.
Since we still need to put_filesystem(), I'd probably add one more
patch
- export put_filesystem()
so then we can put_filesystem() in i915. Wonder what would happen
if someone would do
modprobe i915
rmmod i916
In a loop.
So something like this (this is against current patch set).
---
drivers/gpu/drm/i915/gem/i915_gemfs.c | 5 ++---
fs/filesystems.c | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
index d437188d1736..4ea7a6f750f4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
@@ -24,10 +24,9 @@ int i915_gemfs_init(struct drm_i915_private *i915)
return -ENODEV;
gemfs = kern_mount(type);
- if (IS_ERR(gemfs)) {
- put_filesystem(type);
+ put_filesystem(type);
+ if (IS_ERR(gemfs))
return PTR_ERR(gemfs);
- }
/*
* Enable huge-pages for objects that are at least HPAGE_PMD_SIZE, most
diff --git a/fs/filesystems.c b/fs/filesystems.c
index 9135646e41ac..4837eda748b5 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -45,6 +45,7 @@ void put_filesystem(struct file_system_type *fs)
{
module_put(fs->owner);
}
+EXPORT_SYMBOL(put_filesystem);
static struct file_system_type **find_filesystem(const char *name, unsigned len)
{
@@ -280,5 +281,4 @@ struct file_system_type *get_fs_type(const char *name)
}
return fs;
}
-
EXPORT_SYMBOL(get_fs_type);
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] i915: do not leak module ref counter
2019-08-02 13:49 ` Sergey Senozhatsky
@ 2019-08-02 13:59 ` Chris Wilson
0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-08-02 13:59 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sergey Senozhatsky, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David Airlie, Daniel Vetter, intel-gfx, dri-devel, linux-kernel
Quoting Sergey Senozhatsky (2019-08-02 14:49:55)
> On (08/02/19 14:41), Chris Wilson wrote:
> [..]
> > struct vfsmount *kern_mount(struct file_system_type *type)
> > {
> > struct vfsmount *mnt;
> > mnt = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
> > if (!IS_ERR(mnt)) {
> > /*
> > * it is a longterm mount, don't release mnt until
> > * we unmount before file sys is unregistered
> > */
> > real_mount(mnt)->mnt_ns = MNT_NS_INTERNAL;
> > }
> > return mnt;
> > }
> >
> > With the exception of fiddling with MNT_NS_INTERNAL, it seems
> > amenable for our needs.
>
> Sorry, not sure I understand. i915 use kern_mount() at the moment.
>
> Since we still need to put_filesystem(), I'd probably add one more
> patch
> - export put_filesystem()
>
> so then we can put_filesystem() in i915. Wonder what would happen
> if someone would do
> modprobe i915
> rmmod i916
> In a loop.
>
> So something like this (this is against current patch set).
Yes, that's what I in mind. I was thinking of whether we can replace our
kern_mount + fc->ops->reconfigure() into a single vfs_kern_mount(), and
whether or not that works with get_fs_type("tmpfs").
-Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-08-02 13:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 12:39 [PATCH 1/2] i915: convert to new mount API Sergey Senozhatsky
2019-08-02 12:39 ` [PATCH 2/2] i915: do not leak module ref counter Sergey Senozhatsky
2019-08-02 12:58 ` Chris Wilson
2019-08-02 13:10 ` Chris Wilson
2019-08-02 13:15 ` Sergey Senozhatsky
2019-08-02 13:35 ` Sergey Senozhatsky
2019-08-02 13:41 ` Chris Wilson
2019-08-02 13:49 ` Sergey Senozhatsky
2019-08-02 13:59 ` Chris Wilson
2019-08-02 12:54 ` [PATCH 1/2] i915: convert to new mount API Chris Wilson
2019-08-02 13:10 ` Sergey Senozhatsky
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).