From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> To: Jerome Glisse <jglisse@redhat.com> Cc: "Ralph Campbell" <rcampbell@nvidia.com>, "Jan Kara" <jack@suse.cz>, "Arnd Bergmann" <arnd@arndb.de>, kvm@vger.kernel.org, "Matthew Wilcox" <mawilcox@microsoft.com>, linux-rdma@vger.kernel.org, "John Hubbard" <jhubbard@nvidia.com>, "Felix Kuehling" <Felix.Kuehling@amd.com>, "Radim Krčmář" <rkrcmar@redhat.com>, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, "Michal Hocko" <mhocko@kernel.org>, linux-mm@kvack.org, "Jason Gunthorpe" <jgg@mellanox.com>, "Christian König" <christian.koenig@amd.com>, "Ross Zwisler" <zwisler@kernel.org>, linux-fsdevel@vger.kernel.org, "Paolo Bonzini" <pbonzini@redhat.com>, "Dan Williams" <dan.j.williams@intel.com>, "Andrew Morton" <akpm@linux-foundation.org> Subject: Re: [PATCH v4 8/9] gpu/drm/i915: optimize out the case when a range is updated to read only Date: Tue, 29 Jan 2019 16:20:00 +0200 [thread overview] Message-ID: <154877159986.4387.16328989441685542244@jlahtine-desk.ger.corp.intel.com> (raw) In-Reply-To: <20190124153032.GA5030@redhat.com> Quoting Jerome Glisse (2019-01-24 17:30:32) > On Thu, Jan 24, 2019 at 02:09:12PM +0200, Joonas Lahtinen wrote: > > Hi Jerome, > > > > This patch seems to have plenty of Cc:s, but none of the right ones :) > > So sorry, i am bad with git commands. > > > For further iterations, I guess you could use git option --cc to make > > sure everyone gets the whole series, and still keep the Cc:s in the > > patches themselves relevant to subsystems. > > Will do. > > > This doesn't seem to be on top of drm-tip, but on top of your previous > > patches(?) that I had some comments about. Could you take a moment to > > first address the couple of question I had, before proceeding to discuss > > what is built on top of that base. > > It is on top of Linus tree so roughly ~ rc3 it does not depend on any > of the previous patch i posted. You actually managed to race a point in time just when Chris rewrote much of the userptr code in drm-tip, which I didn't remember of. My bad. Still interested to hearing replies to my questions in the previous thread, if the series is still relevant. Trying to get my head around how the different aspects of HMM pan out for devices without fault handling. > I still intended to propose to remove > GUP from i915 once i get around to implement the equivalent of GUP_fast > for HMM and other bonus cookies with it. > > The plan is once i have all mm bits properly upstream then i can propose > patches to individual driver against the proper driver tree ie following > rules of each individual device driver sub-system and Cc only people > there to avoid spamming the mm folks :) Makes sense, as we're having tons of changes in this field in i915, the churn to rebase on top of them will be substantial. Regards, Joonas PS. Are you by any chance attending FOSDEM? Would be nice to chat about this. > > > > > > My reply's Message-ID is: > > 154289518994.19402.3481838548028068213@jlahtine-desk.ger.corp.intel.com > > > > Regards, Joonas > > > > PS. Please keep me Cc:d in the following patches, I'm keen on > > understanding the motive and benefits. > > > > Quoting jglisse@redhat.com (2019-01-24 00:23:14) > > > From: Jérôme Glisse <jglisse@redhat.com> > > > > > > When range of virtual address is updated read only and corresponding > > > user ptr object are already read only it is pointless to do anything. > > > Optimize this case out. > > > > > > Signed-off-by: Jérôme Glisse <jglisse@redhat.com> > > > Cc: Christian König <christian.koenig@amd.com> > > > Cc: Jan Kara <jack@suse.cz> > > > Cc: Felix Kuehling <Felix.Kuehling@amd.com> > > > Cc: Jason Gunthorpe <jgg@mellanox.com> > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Cc: Matthew Wilcox <mawilcox@microsoft.com> > > > Cc: Ross Zwisler <zwisler@kernel.org> > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > Cc: Radim Krčmář <rkrcmar@redhat.com> > > > Cc: Michal Hocko <mhocko@kernel.org> > > > Cc: Ralph Campbell <rcampbell@nvidia.com> > > > Cc: John Hubbard <jhubbard@nvidia.com> > > > Cc: kvm@vger.kernel.org > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: linux-rdma@vger.kernel.org > > > Cc: linux-fsdevel@vger.kernel.org > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > --- > > > drivers/gpu/drm/i915/i915_gem_userptr.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c > > > index 9558582c105e..23330ac3d7ea 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_userptr.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c > > > @@ -59,6 +59,7 @@ struct i915_mmu_object { > > > struct interval_tree_node it; > > > struct list_head link; > > > struct work_struct work; > > > + bool read_only; > > > bool attached; > > > }; > > > > > > @@ -119,6 +120,7 @@ static int i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > > > container_of(_mn, struct i915_mmu_notifier, mn); > > > struct i915_mmu_object *mo; > > > struct interval_tree_node *it; > > > + bool update_to_read_only; > > > LIST_HEAD(cancelled); > > > unsigned long end; > > > > > > @@ -128,6 +130,8 @@ static int i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > > > /* interval ranges are inclusive, but invalidate range is exclusive */ > > > end = range->end - 1; > > > > > > + update_to_read_only = mmu_notifier_range_update_to_read_only(range); > > > + > > > spin_lock(&mn->lock); > > > it = interval_tree_iter_first(&mn->objects, range->start, end); > > > while (it) { > > > @@ -145,6 +149,17 @@ static int i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > > > * object if it is not in the process of being destroyed. > > > */ > > > mo = container_of(it, struct i915_mmu_object, it); > > > + > > > + /* > > > + * If it is already read only and we are updating to > > > + * read only then we do not need to change anything. > > > + * So save time and skip this one. > > > + */ > > > + if (update_to_read_only && mo->read_only) { > > > + it = interval_tree_iter_next(it, range->start, end); > > > + continue; > > > + } > > > + > > > if (kref_get_unless_zero(&mo->obj->base.refcount)) > > > queue_work(mn->wq, &mo->work); > > > > > > @@ -270,6 +285,7 @@ i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, > > > mo->mn = mn; > > > mo->obj = obj; > > > mo->it.start = obj->userptr.ptr; > > > + mo->read_only = i915_gem_object_is_readonly(obj); > > > mo->it.last = obj->userptr.ptr + obj->base.size - 1; > > > INIT_WORK(&mo->work, cancel_userptr); > > > > > > -- > > > 2.17.2 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> To: Jerome Glisse <jglisse@redhat.com> Cc: linux-mm@kvack.org, "Ralph Campbell" <rcampbell@nvidia.com>, "Jan Kara" <jack@suse.cz>, "Arnd Bergmann" <arnd@arndb.de>, kvm@vger.kernel.org, "Matthew Wilcox" <mawilcox@microsoft.com>, linux-rdma@vger.kernel.org, "John Hubbard" <jhubbard@nvidia.com>, "Felix Kuehling" <Felix.Kuehling@amd.com>, "Radim Krčmář" <rkrcmar@redhat.com>, "Dan Williams" <dan.j.williams@intel.com>, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, "Michal Hocko" <mhocko@kernel.org>, "Jason Gunthorpe" <jgg@mellanox.com>, "Ross Zwisler" <zwisler@kernel.org>, linux-fsdevel@vger.kernel.org, "Paolo Bonzini" <pbonzini@redhat.com>, "Andrew Morton" <akpm@linux-foundation.org>, "Christian König" <christian.koenig@amd.com> Subject: Re: [PATCH v4 8/9] gpu/drm/i915: optimize out the case when a range is updated to read only Date: Tue, 29 Jan 2019 16:20:00 +0200 [thread overview] Message-ID: <154877159986.4387.16328989441685542244@jlahtine-desk.ger.corp.intel.com> (raw) In-Reply-To: <20190124153032.GA5030@redhat.com> Quoting Jerome Glisse (2019-01-24 17:30:32) > On Thu, Jan 24, 2019 at 02:09:12PM +0200, Joonas Lahtinen wrote: > > Hi Jerome, > > > > This patch seems to have plenty of Cc:s, but none of the right ones :) > > So sorry, i am bad with git commands. > > > For further iterations, I guess you could use git option --cc to make > > sure everyone gets the whole series, and still keep the Cc:s in the > > patches themselves relevant to subsystems. > > Will do. > > > This doesn't seem to be on top of drm-tip, but on top of your previous > > patches(?) that I had some comments about. Could you take a moment to > > first address the couple of question I had, before proceeding to discuss > > what is built on top of that base. > > It is on top of Linus tree so roughly ~ rc3 it does not depend on any > of the previous patch i posted. You actually managed to race a point in time just when Chris rewrote much of the userptr code in drm-tip, which I didn't remember of. My bad. Still interested to hearing replies to my questions in the previous thread, if the series is still relevant. Trying to get my head around how the different aspects of HMM pan out for devices without fault handling. > I still intended to propose to remove > GUP from i915 once i get around to implement the equivalent of GUP_fast > for HMM and other bonus cookies with it. > > The plan is once i have all mm bits properly upstream then i can propose > patches to individual driver against the proper driver tree ie following > rules of each individual device driver sub-system and Cc only people > there to avoid spamming the mm folks :) Makes sense, as we're having tons of changes in this field in i915, the churn to rebase on top of them will be substantial. Regards, Joonas PS. Are you by any chance attending FOSDEM? Would be nice to chat about this. > > > > > > My reply's Message-ID is: > > 154289518994.19402.3481838548028068213@jlahtine-desk.ger.corp.intel.com > > > > Regards, Joonas > > > > PS. Please keep me Cc:d in the following patches, I'm keen on > > understanding the motive and benefits. > > > > Quoting jglisse@redhat.com (2019-01-24 00:23:14) > > > From: Jérôme Glisse <jglisse@redhat.com> > > > > > > When range of virtual address is updated read only and corresponding > > > user ptr object are already read only it is pointless to do anything. > > > Optimize this case out. > > > > > > Signed-off-by: Jérôme Glisse <jglisse@redhat.com> > > > Cc: Christian König <christian.koenig@amd.com> > > > Cc: Jan Kara <jack@suse.cz> > > > Cc: Felix Kuehling <Felix.Kuehling@amd.com> > > > Cc: Jason Gunthorpe <jgg@mellanox.com> > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Cc: Matthew Wilcox <mawilcox@microsoft.com> > > > Cc: Ross Zwisler <zwisler@kernel.org> > > > Cc: Dan Williams <dan.j.williams@intel.com> > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > Cc: Radim Krčmář <rkrcmar@redhat.com> > > > Cc: Michal Hocko <mhocko@kernel.org> > > > Cc: Ralph Campbell <rcampbell@nvidia.com> > > > Cc: John Hubbard <jhubbard@nvidia.com> > > > Cc: kvm@vger.kernel.org > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: linux-rdma@vger.kernel.org > > > Cc: linux-fsdevel@vger.kernel.org > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > --- > > > drivers/gpu/drm/i915/i915_gem_userptr.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c > > > index 9558582c105e..23330ac3d7ea 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_userptr.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c > > > @@ -59,6 +59,7 @@ struct i915_mmu_object { > > > struct interval_tree_node it; > > > struct list_head link; > > > struct work_struct work; > > > + bool read_only; > > > bool attached; > > > }; > > > > > > @@ -119,6 +120,7 @@ static int i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > > > container_of(_mn, struct i915_mmu_notifier, mn); > > > struct i915_mmu_object *mo; > > > struct interval_tree_node *it; > > > + bool update_to_read_only; > > > LIST_HEAD(cancelled); > > > unsigned long end; > > > > > > @@ -128,6 +130,8 @@ static int i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > > > /* interval ranges are inclusive, but invalidate range is exclusive */ > > > end = range->end - 1; > > > > > > + update_to_read_only = mmu_notifier_range_update_to_read_only(range); > > > + > > > spin_lock(&mn->lock); > > > it = interval_tree_iter_first(&mn->objects, range->start, end); > > > while (it) { > > > @@ -145,6 +149,17 @@ static int i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, > > > * object if it is not in the process of being destroyed. > > > */ > > > mo = container_of(it, struct i915_mmu_object, it); > > > + > > > + /* > > > + * If it is already read only and we are updating to > > > + * read only then we do not need to change anything. > > > + * So save time and skip this one. > > > + */ > > > + if (update_to_read_only && mo->read_only) { > > > + it = interval_tree_iter_next(it, range->start, end); > > > + continue; > > > + } > > > + > > > if (kref_get_unless_zero(&mo->obj->base.refcount)) > > > queue_work(mn->wq, &mo->work); > > > > > > @@ -270,6 +285,7 @@ i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj, > > > mo->mn = mn; > > > mo->obj = obj; > > > mo->it.start = obj->userptr.ptr; > > > + mo->read_only = i915_gem_object_is_readonly(obj); > > > mo->it.last = obj->userptr.ptr + obj->base.size - 1; > > > INIT_WORK(&mo->work, cancel_userptr); > > > > > > -- > > > 2.17.2 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-01-29 14:20 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-23 22:23 [PATCH v4 0/9] mmu notifier provide context informations jglisse 2019-01-23 22:23 ` [PATCH v4 1/9] mm/mmu_notifier: contextual information for event enums jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 2/9] mm/mmu_notifier: contextual information for event triggering invalidation jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 3/9] mm/mmu_notifier: use correct mmu_notifier events for each invalidation jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 4/9] mm/mmu_notifier: pass down vma and reasons why mmu notifier is happening jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 5/9] mm/mmu_notifier: mmu_notifier_range_update_to_read_only() helper jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 6/9] gpu/drm/radeon: optimize out the case when a range is updated to read only jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 7/9] gpu/drm/amdgpu: " jglisse 2019-01-23 22:23 ` jglisse 2019-01-23 22:23 ` [PATCH v4 8/9] gpu/drm/i915: " jglisse 2019-01-23 22:23 ` jglisse 2019-01-24 12:09 ` Joonas Lahtinen 2019-01-24 12:09 ` Joonas Lahtinen 2019-01-24 12:09 ` Joonas Lahtinen 2019-01-24 12:09 ` Joonas Lahtinen 2019-01-24 15:30 ` Jerome Glisse 2019-01-24 15:30 ` Jerome Glisse 2019-01-29 14:20 ` Joonas Lahtinen [this message] 2019-01-29 14:20 ` Joonas Lahtinen 2019-01-29 16:21 ` Jerome Glisse 2019-01-23 22:23 ` [PATCH v4 9/9] RDMA/umem_odp: " jglisse 2019-01-23 22:32 ` Jason Gunthorpe 2019-01-23 22:32 ` Jason Gunthorpe 2019-01-23 22:46 ` Jerome Glisse 2019-01-23 22:46 ` Jerome Glisse 2019-01-23 22:54 ` [PATCH v4 0/9] mmu notifier provide context informations Dan Williams 2019-01-23 22:54 ` Dan Williams 2019-01-23 23:04 ` Jerome Glisse 2019-01-23 23:04 ` Jerome Glisse 2019-01-24 0:00 ` Dan Williams 2019-01-24 0:00 ` Dan Williams 2019-01-31 16:10 ` Jerome Glisse 2019-01-31 16:10 ` Jerome Glisse 2019-01-31 19:55 ` Andrew Morton 2019-01-31 20:33 ` Jerome Glisse 2019-02-01 12:24 ` Christian König 2019-02-01 12:24 ` Christian König 2019-02-01 21:02 ` Jan Kara 2019-02-01 21:02 ` Jan Kara 2019-02-11 18:54 ` Jerome Glisse
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=154877159986.4387.16328989441685542244@jlahtine-desk.ger.corp.intel.com \ --to=joonas.lahtinen@linux.intel.com \ --cc=Felix.Kuehling@amd.com \ --cc=akpm@linux-foundation.org \ --cc=arnd@arndb.de \ --cc=christian.koenig@amd.com \ --cc=dan.j.williams@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=jack@suse.cz \ --cc=jgg@mellanox.com \ --cc=jglisse@redhat.com \ --cc=jhubbard@nvidia.com \ --cc=kvm@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-rdma@vger.kernel.org \ --cc=mawilcox@microsoft.com \ --cc=mhocko@kernel.org \ --cc=pbonzini@redhat.com \ --cc=rcampbell@nvidia.com \ --cc=rkrcmar@redhat.com \ --cc=zwisler@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.