| Message ID | 20221017093726.2070674-4-zhao1.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Replace kmap_atomic() with kmap_local_page() | expand |
On Mon, Oct 17, 2022 at 05:37:19PM +0800, Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > The use of kmap_atomic() is being deprecated in favor of > kmap_local_page()[1]. > > The main difference between atomic and local mappings is that local > mappings doesn't disable page faults or preemption. > > In drm/i915/gem/i915_gem_shmem.c, the function shmem_pwrite() need to > disable pagefault to eliminate the potential recursion fault[2]. But > here __copy_from_user_inatomic() doesn't need to disable preemption and > local mapping is valid for sched in/out. > > So it can use kmap_local_page() / kunmap_local() with > pagefault_disable() / pagefault_enable() to replace atomic mapping. > > [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com > [2]: https://patchwork.freedesktop.org/patch/295840/ > > Suggested-by: Ira Weiny <ira.weiny@intel.com> Reviewed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > Suggested by credits: > Ira: Referred to his suggestions about keeping pagefault_disable(). > --- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index f42ca1179f37..e279a3e30c02 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > @@ -472,11 +472,13 @@ shmem_pwrite(struct drm_i915_gem_object *obj, > if (err < 0) > return err; > > - vaddr = kmap_atomic(page); > + vaddr = kmap_local_page(page); > + pagefault_disable(); > unwritten = __copy_from_user_inatomic(vaddr + pg, > user_data, > len); > - kunmap_atomic(vaddr); > + pagefault_enable(); > + kunmap_local(vaddr); > > err = aops->write_end(obj->base.filp, mapping, offset, len, > len - unwritten, page, data); > -- > 2.34.1 >
On lunedì 17 ottobre 2022 11:37:19 CET Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > The use of kmap_atomic() is being deprecated in favor of > kmap_local_page()[1]. > > The main difference between atomic and local mappings is that local > mappings doesn't disable page faults or preemption. > > In drm/i915/gem/i915_gem_shmem.c, the function shmem_pwrite() need to > disable pagefault to eliminate the potential recursion fault[2]. But > here __copy_from_user_inatomic() doesn't need to disable preemption and > local mapping is valid for sched in/out. > So it can use kmap_local_page() / kunmap_local() with > pagefault_disable() / pagefault_enable() to replace atomic mapping. > > [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com > [2]: https://patchwork.freedesktop.org/patch/295840/ > > Suggested-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > Suggested by credits: > Ira: Referred to his suggestions about keeping pagefault_disable(). > --- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) The commit message explains clearly and the changes to the code look good. The necessity to reuse pagefault_disable() / pagefault_enable() from the main kmap_atomic() side effect is a nice catch. Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Thanks! > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index f42ca1179f37..e279a3e30c02 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > @@ -472,11 +472,13 @@ shmem_pwrite(struct drm_i915_gem_object *obj, > if (err < 0) > return err; > > - vaddr = kmap_atomic(page); > + vaddr = kmap_local_page(page); > + pagefault_disable(); > unwritten = __copy_from_user_inatomic(vaddr + pg, > user_data, > len); > - kunmap_atomic(vaddr); > + pagefault_enable(); > + kunmap_local(vaddr); > > err = aops->write_end(obj->base.filp, mapping, offset, len, > len - unwritten, page, data);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index f42ca1179f37..e279a3e30c02 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -472,11 +472,13 @@ shmem_pwrite(struct drm_i915_gem_object *obj, if (err < 0) return err; - vaddr = kmap_atomic(page); + vaddr = kmap_local_page(page); + pagefault_disable(); unwritten = __copy_from_user_inatomic(vaddr + pg, user_data, len); - kunmap_atomic(vaddr); + pagefault_enable(); + kunmap_local(vaddr); err = aops->write_end(obj->base.filp, mapping, offset, len, len - unwritten, page, data);