| Message ID | 20170821183503.12246-2-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andrew, This patch has been floating around for a while now Acked and without further comments. It is blocking us from merging huge page support to drm/i915. Would you mind merging it, or prodding the right people to get it in? Regards, Joonas On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > 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. > > 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: Dave Hansen <dave.hansen@intel.com> > Cc: Kirill A. Shutemov <kirill@shutemov.name> > Cc: Hugh Dickins <hughd@google.com> > Cc: linux-mm@kvack.org > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > include/linux/shmem_fs.h | 2 ++ > mm/shmem.c | 30 ++++++++++++++++++++++-------- > 2 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h > index a7d6bd2a918f..27de676f0b63 100644 > --- a/include/linux/shmem_fs.h > +++ b/include/linux/shmem_fs.h > @@ -53,6 +53,8 @@ extern struct file *shmem_file_setup(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); > +extern struct file *shmem_file_setup_with_mnt(struct vfsmount *mnt, > + const char *name, loff_t size, unsigned long flags); > extern int shmem_zero_setup(struct vm_area_struct *); > extern unsigned long shmem_get_unmapped_area(struct file *, unsigned long addr, > unsigned long len, unsigned long pgoff, unsigned long flags); > diff --git a/mm/shmem.c b/mm/shmem.c > index 6540e5982444..0975e65ea61c 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -4141,7 +4141,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; > @@ -4150,8 +4150,8 @@ 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 (IS_ERR(mnt)) > + return ERR_CAST(mnt); > > if (size < 0 || size > MAX_LFS_FILESIZE) > return ERR_PTR(-EINVAL); > @@ -4163,8 +4163,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; > @@ -4209,7 +4209,7 @@ 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(shm_mnt, name, size, flags, S_PRIVATE); > } > > /** > @@ -4220,11 +4220,25 @@ struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned lon > */ > struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags) > { > - return __shmem_file_setup(name, size, flags, 0); > + return __shmem_file_setup(shm_mnt, name, size, flags, 0); > } > EXPORT_SYMBOL_GPL(shmem_file_setup); > > /** > + * shmem_file_setup_with_mnt - 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_with_mnt(struct vfsmount *mnt, const char *name, > + loff_t size, unsigned long flags) > +{ > + return __shmem_file_setup(mnt, name, size, flags, 0); > +} > +EXPORT_SYMBOL_GPL(shmem_file_setup_with_mnt); > + > +/** > * shmem_zero_setup - setup a shared anonymous mapping > * @vma: the vma to be mmapped is prepared by do_mmap_pgoff > */ > @@ -4239,7 +4253,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); >
On Wed, 23 Aug 2017 12:31:28 +0300 Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > This patch has been floating around for a while now Acked and without > further comments. It is blocking us from merging huge page support to > drm/i915. > > Would you mind merging it, or prodding the right people to get it in? > > Regards, Joonas > > On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > > 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. hm, it's a bit odd. I'm having trouble locating the code which handles huge=within_size (and any other options?). What other approaches were considered? Was it not feasible to add i915-specific mount options to mm/shmem.c (for example?).
On 23 August 2017 at 23:34, Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 23 Aug 2017 12:31:28 +0300 Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > >> This patch has been floating around for a while now Acked and without >> further comments. It is blocking us from merging huge page support to >> drm/i915. >> >> Would you mind merging it, or prodding the right people to get it in? >> >> Regards, Joonas >> >> On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: >> > 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. > > hm, it's a bit odd. I'm having trouble locating the code which handles > huge=within_size (and any other options?). See here https://patchwork.freedesktop.org/patch/172771/, currently we only care about huge=within_size. > What other approaches were considered? We also tried https://patchwork.freedesktop.org/patch/156528/, where it was suggested that we mount our own tmpfs instance. Following from that we now have our own tmps mnt mounted with huge=within_size. With this patch we avoid having to roll our own __shmem_file_setup like in https://patchwork.freedesktop.org/patch/163024/. > Was it not feasible to add i915-specific mount options to > mm/shmem.c (for example?). Hmm, I think within_size should suffice for our needs. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 24 Aug 2017 13:04:09 +0100 Matthew Auld <matthew.william.auld@gmail.com> wrote: > On 23 August 2017 at 23:34, Andrew Morton <akpm@linux-foundation.org> wrote: > > On Wed, 23 Aug 2017 12:31:28 +0300 Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > > > >> This patch has been floating around for a while now Acked and without > >> further comments. It is blocking us from merging huge page support to > >> drm/i915. > >> > >> Would you mind merging it, or prodding the right people to get it in? > >> > >> Regards, Joonas > >> > >> On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > >> > 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. > > > > hm, it's a bit odd. I'm having trouble locating the code which handles > > huge=within_size (and any other options?). > > See here https://patchwork.freedesktop.org/patch/172771/, currently we > only care about huge=within_size. > > > What other approaches were considered? > > We also tried https://patchwork.freedesktop.org/patch/156528/, where > it was suggested that we mount our own tmpfs instance. > > Following from that we now have our own tmps mnt mounted with > huge=within_size. With this patch we avoid having to roll our own > __shmem_file_setup like in > https://patchwork.freedesktop.org/patch/163024/. > > > Was it not feasible to add i915-specific mount options to > > mm/shmem.c (for example?). > > Hmm, I think within_size should suffice for our needs. hm, ok, well, unless someone can think of something cleaner, please add my ack and include it in the appropriate drm tree.
On Fri, 2017-08-25 at 13:49 -0700, Andrew Morton wrote: > On Thu, 24 Aug 2017 13:04:09 +0100 Matthew Auld <matthew.william.auld@gmail.com> wrote: > > > On 23 August 2017 at 23:34, Andrew Morton <akpm@linux-foundation.org> wrote: > > > On Wed, 23 Aug 2017 12:31:28 +0300 Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote: > > > > > > > This patch has been floating around for a while now Acked and without > > > > further comments. It is blocking us from merging huge page support to > > > > drm/i915. > > > > > > > > Would you mind merging it, or prodding the right people to get it in? > > > > > > > > Regards, Joonas > > > > > > > > On Mon, 2017-08-21 at 19:34 +0100, Matthew Auld wrote: > > > > > 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. > > > > > > hm, it's a bit odd. I'm having trouble locating the code which handles > > > huge=within_size (and any other options?). > > > > See here https://patchwork.freedesktop.org/patch/172771/, currently we > > only care about huge=within_size. > > > > > What other approaches were considered? > > > > We also tried https://patchwork.freedesktop.org/patch/156528/, where > > it was suggested that we mount our own tmpfs instance. > > > > Following from that we now have our own tmps mnt mounted with > > huge=within_size. With this patch we avoid having to roll our own > > __shmem_file_setup like in > > https://patchwork.freedesktop.org/patch/163024/. > > > > > Was it not feasible to add i915-specific mount options to > > > mm/shmem.c (for example?). > > > > Hmm, I think within_size should suffice for our needs. > > hm, ok, well, unless someone can think of something cleaner, please add > my ack and include it in the appropriate drm tree. Thanks, I will do that. It'll first get incorporated into drm-tip ( https://cgit.freedesktop.org/drm-tip) once the kselftests are finalized (now that we know we're not facing third rewrite for core MM dependency). And eventually into drm-next through a pull request to Dave Airlie. Regards, Joonas
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h index a7d6bd2a918f..27de676f0b63 100644 --- a/include/linux/shmem_fs.h +++ b/include/linux/shmem_fs.h @@ -53,6 +53,8 @@ extern struct file *shmem_file_setup(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); +extern struct file *shmem_file_setup_with_mnt(struct vfsmount *mnt, + const char *name, loff_t size, unsigned long flags); extern int shmem_zero_setup(struct vm_area_struct *); extern unsigned long shmem_get_unmapped_area(struct file *, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags); diff --git a/mm/shmem.c b/mm/shmem.c index 6540e5982444..0975e65ea61c 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -4141,7 +4141,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; @@ -4150,8 +4150,8 @@ 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 (IS_ERR(mnt)) + return ERR_CAST(mnt); if (size < 0 || size > MAX_LFS_FILESIZE) return ERR_PTR(-EINVAL); @@ -4163,8 +4163,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; @@ -4209,7 +4209,7 @@ 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(shm_mnt, name, size, flags, S_PRIVATE); } /** @@ -4220,11 +4220,25 @@ struct file *shmem_kernel_file_setup(const char *name, loff_t size, unsigned lon */ struct file *shmem_file_setup(const char *name, loff_t size, unsigned long flags) { - return __shmem_file_setup(name, size, flags, 0); + return __shmem_file_setup(shm_mnt, name, size, flags, 0); } EXPORT_SYMBOL_GPL(shmem_file_setup); /** + * shmem_file_setup_with_mnt - 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_with_mnt(struct vfsmount *mnt, const char *name, + loff_t size, unsigned long flags) +{ + return __shmem_file_setup(mnt, name, size, flags, 0); +} +EXPORT_SYMBOL_GPL(shmem_file_setup_with_mnt); + +/** * shmem_zero_setup - setup a shared anonymous mapping * @vma: the vma to be mmapped is prepared by do_mmap_pgoff */ @@ -4239,7 +4253,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);