Since
commit 96667f8a4382db9ed042332ca6ee165ae9b91307 Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Fri Nov 27 17:41:21 2020 +0100
mm: Close race in generic_access_phys
it is race-free and can therefore be safely used for dynamic mappings like we have too.
Cc: Jon Bloomfield jon.bloomfield@intel.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: "Thomas Hellström" thomas.hellstrom@linux.intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Andrew Morton akpm@linux-foundation.org Cc: "Christian König" christian.koenig@amd.com Cc: "Ville Syrjälä" ville.syrjala@linux.intel.com Cc: Michel Lespinasse walken@google.com --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 60 +++--------------------- 1 file changed, 6 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index f6fe5cb01438..717798293044 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -414,58 +414,6 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf) return i915_error_to_vmf_fault(ret); }
-static int -vm_access(struct vm_area_struct *area, unsigned long addr, - void *buf, int len, int write) -{ - struct i915_mmap_offset *mmo = area->vm_private_data; - struct drm_i915_gem_object *obj = mmo->obj; - struct i915_gem_ww_ctx ww; - void *vaddr; - int err = 0; - - if (i915_gem_object_is_readonly(obj) && write) - return -EACCES; - - addr -= area->vm_start; - if (addr >= obj->base.size) - return -EINVAL; - - i915_gem_ww_ctx_init(&ww, true); -retry: - err = i915_gem_object_lock(obj, &ww); - if (err) - goto out; - - /* As this is primarily for debugging, let's focus on simplicity */ - vaddr = i915_gem_object_pin_map(obj, I915_MAP_FORCE_WC); - if (IS_ERR(vaddr)) { - err = PTR_ERR(vaddr); - goto out; - } - - if (write) { - memcpy(vaddr + addr, buf, len); - __i915_gem_object_flush_map(obj, addr, len); - } else { - memcpy(buf, vaddr + addr, len); - } - - i915_gem_object_unpin_map(obj); -out: - if (err == -EDEADLK) { - err = i915_gem_ww_ctx_backoff(&ww); - if (!err) - goto retry; - } - i915_gem_ww_ctx_fini(&ww); - - if (err) - return err; - - return len; -} - void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj) { struct i915_vma *vma; @@ -801,14 +749,18 @@ static void vm_close(struct vm_area_struct *vma)
static const struct vm_operations_struct vm_ops_gtt = { .fault = vm_fault_gtt, - .access = vm_access, +#ifdef CONFIG_HAVE_IOREMAP_PROT + .access = generic_access_phys +#endif .open = vm_open, .close = vm_close, };
static const struct vm_operations_struct vm_ops_cpu = { .fault = vm_fault_cpu, - .access = vm_access, +#ifdef CONFIG_HAVE_IOREMAP_PROT + .access = generic_access_phys +#endif .open = vm_open, .close = vm_close, };
Since
commit 96667f8a4382db9ed042332ca6ee165ae9b91307 Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Fri Nov 27 17:41:21 2020 +0100
mm: Close race in generic_access_phys
it is race-free and can therefore be safely used for dynamic mappings like we have too.
v2 git commit --amend
*sigh*
Cc: Jon Bloomfield jon.bloomfield@intel.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: "Thomas Hellström" thomas.hellstrom@linux.intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Andrew Morton akpm@linux-foundation.org Cc: "Christian König" christian.koenig@amd.com Cc: "Ville Syrjälä" ville.syrjala@linux.intel.com Cc: Michel Lespinasse walken@google.com --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 60 +++--------------------- 1 file changed, 6 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index f6fe5cb01438..16a059d54bda 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -414,58 +414,6 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf) return i915_error_to_vmf_fault(ret); }
-static int -vm_access(struct vm_area_struct *area, unsigned long addr, - void *buf, int len, int write) -{ - struct i915_mmap_offset *mmo = area->vm_private_data; - struct drm_i915_gem_object *obj = mmo->obj; - struct i915_gem_ww_ctx ww; - void *vaddr; - int err = 0; - - if (i915_gem_object_is_readonly(obj) && write) - return -EACCES; - - addr -= area->vm_start; - if (addr >= obj->base.size) - return -EINVAL; - - i915_gem_ww_ctx_init(&ww, true); -retry: - err = i915_gem_object_lock(obj, &ww); - if (err) - goto out; - - /* As this is primarily for debugging, let's focus on simplicity */ - vaddr = i915_gem_object_pin_map(obj, I915_MAP_FORCE_WC); - if (IS_ERR(vaddr)) { - err = PTR_ERR(vaddr); - goto out; - } - - if (write) { - memcpy(vaddr + addr, buf, len); - __i915_gem_object_flush_map(obj, addr, len); - } else { - memcpy(buf, vaddr + addr, len); - } - - i915_gem_object_unpin_map(obj); -out: - if (err == -EDEADLK) { - err = i915_gem_ww_ctx_backoff(&ww); - if (!err) - goto retry; - } - i915_gem_ww_ctx_fini(&ww); - - if (err) - return err; - - return len; -} - void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj) { struct i915_vma *vma; @@ -801,14 +749,18 @@ static void vm_close(struct vm_area_struct *vma)
static const struct vm_operations_struct vm_ops_gtt = { .fault = vm_fault_gtt, - .access = vm_access, +#ifdef CONFIG_HAVE_IOREMAP_PROT + .access = generic_access_phys, +#endif .open = vm_open, .close = vm_close, };
static const struct vm_operations_struct vm_ops_cpu = { .fault = vm_fault_cpu, - .access = vm_access, +#ifdef CONFIG_HAVE_IOREMAP_PROT + .access = generic_access_phys, +#endif .open = vm_open, .close = vm_close, };
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip v5.13-rc3 next-20210526] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Use-generic_... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-a001-20210526 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/80157be9e8542ce9a835e6f159408b951590... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/drm-i915-Use-generic_access_phys/20210526-231425 git checkout 80157be9e8542ce9a835e6f159408b951590b578 # save the attached .config to linux build tree make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gem/i915_gem_mman.c:755:2: error: request for member 'open' in something not a structure or union
755 | .open = vm_open, | ^ drivers/gpu/drm/i915/gem/i915_gem_mman.c:764:2: error: request for member 'open' in something not a structure or union 764 | .open = vm_open, | ^
vim +/open +755 drivers/gpu/drm/i915/gem/i915_gem_mman.c
cc662126b4134e2 Abdiel Janulgue 2019-12-04 749 cc662126b4134e2 Abdiel Janulgue 2019-12-04 750 static const struct vm_operations_struct vm_ops_gtt = { cc662126b4134e2 Abdiel Janulgue 2019-12-04 751 .fault = vm_fault_gtt, 80157be9e8542ce Daniel Vetter 2021-05-26 752 #ifdef CONFIG_HAVE_IOREMAP_PROT 80157be9e8542ce Daniel Vetter 2021-05-26 753 .access = generic_access_phys 80157be9e8542ce Daniel Vetter 2021-05-26 754 #endif cc662126b4134e2 Abdiel Janulgue 2019-12-04 @755 .open = vm_open, cc662126b4134e2 Abdiel Janulgue 2019-12-04 756 .close = vm_close, cc662126b4134e2 Abdiel Janulgue 2019-12-04 757 }; cc662126b4134e2 Abdiel Janulgue 2019-12-04 758
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip v5.13-rc3 next-20210526] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Use-generic_... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-r025-20210526 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/80157be9e8542ce9a835e6f159408b951590... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/drm-i915-Use-generic_access_phys/20210526-231425 git checkout 80157be9e8542ce9a835e6f159408b951590b578 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gem/i915_gem_mman.c:755:2: error: member reference base type 'int (struct vm_area_struct *, unsigned long, void *, int, int)' is not a structure or union
.open = vm_open, ^~~~~ drivers/gpu/drm/i915/gem/i915_gem_mman.c:764:2: error: member reference base type 'int (struct vm_area_struct *, unsigned long, void *, int, int)' is not a structure or union .open = vm_open, ^~~~~ 2 errors generated.
vim +755 drivers/gpu/drm/i915/gem/i915_gem_mman.c
cc662126b4134e2 Abdiel Janulgue 2019-12-04 749 cc662126b4134e2 Abdiel Janulgue 2019-12-04 750 static const struct vm_operations_struct vm_ops_gtt = { cc662126b4134e2 Abdiel Janulgue 2019-12-04 751 .fault = vm_fault_gtt, 80157be9e8542ce Daniel Vetter 2021-05-26 752 #ifdef CONFIG_HAVE_IOREMAP_PROT 80157be9e8542ce Daniel Vetter 2021-05-26 753 .access = generic_access_phys 80157be9e8542ce Daniel Vetter 2021-05-26 754 #endif cc662126b4134e2 Abdiel Janulgue 2019-12-04 @755 .open = vm_open, cc662126b4134e2 Abdiel Janulgue 2019-12-04 756 .close = vm_close, cc662126b4134e2 Abdiel Janulgue 2019-12-04 757 }; cc662126b4134e2 Abdiel Janulgue 2019-12-04 758
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, May 26, 2021 at 05:11:06PM +0200, Daniel Vetter wrote:
Since
commit 96667f8a4382db9ed042332ca6ee165ae9b91307 Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Fri Nov 27 17:41:21 2020 +0100
mm: Close race in generic_access_physit is race-free and can therefore be safely used for dynamic mappings like we have too.
Cc: Jon Bloomfield jon.bloomfield@intel.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: "Thomas Hellström" thomas.hellstrom@linux.intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Andrew Morton akpm@linux-foundation.org Cc: "Christian König" christian.koenig@amd.com Cc: "Ville Syrjälä" ville.syrjala@linux.intel.com Cc: Michel Lespinasse walken@google.com
This turned out to be a bad idea, because we stuff normal memory into our io mappings (how well does that work on other archs, no idea). That would be fixable, but then again we have our issue with using multiple inconsistent mmaps, and at that point it's probably a lost cause :-( -Daniel
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 60 +++--------------------- 1 file changed, 6 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index f6fe5cb01438..717798293044 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -414,58 +414,6 @@ static vm_fault_t vm_fault_gtt(struct vm_fault *vmf) return i915_error_to_vmf_fault(ret); }
-static int -vm_access(struct vm_area_struct *area, unsigned long addr,
void *buf, int len, int write)-{
- struct i915_mmap_offset *mmo = area->vm_private_data;
- struct drm_i915_gem_object *obj = mmo->obj;
- struct i915_gem_ww_ctx ww;
- void *vaddr;
- int err = 0;
- if (i915_gem_object_is_readonly(obj) && write)
return -EACCES;- addr -= area->vm_start;
- if (addr >= obj->base.size)
return -EINVAL;- i915_gem_ww_ctx_init(&ww, true);
-retry:
- err = i915_gem_object_lock(obj, &ww);
- if (err)
goto out;- /* As this is primarily for debugging, let's focus on simplicity */
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_FORCE_WC);
- if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);goto out;- }
- if (write) {
memcpy(vaddr + addr, buf, len);__i915_gem_object_flush_map(obj, addr, len);- } else {
memcpy(buf, vaddr + addr, len);- }
- i915_gem_object_unpin_map(obj);
-out:
- if (err == -EDEADLK) {
err = i915_gem_ww_ctx_backoff(&ww);if (!err)goto retry;- }
- i915_gem_ww_ctx_fini(&ww);
- if (err)
return err;- return len;
-}
void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj) { struct i915_vma *vma; @@ -801,14 +749,18 @@ static void vm_close(struct vm_area_struct *vma)
static const struct vm_operations_struct vm_ops_gtt = { .fault = vm_fault_gtt,
- .access = vm_access,
+#ifdef CONFIG_HAVE_IOREMAP_PROT
- .access = generic_access_phys
+#endif .open = vm_open, .close = vm_close, };
static const struct vm_operations_struct vm_ops_cpu = { .fault = vm_fault_cpu,
- .access = vm_access,
+#ifdef CONFIG_HAVE_IOREMAP_PROT
- .access = generic_access_phys
+#endif .open = vm_open, .close = vm_close, }; -- 2.31.0
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
kernel test robot