| Message ID | 20230329073220.3982460-1-zhao1.liu@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915: Replace kmap_atomic() with kmap_local_page() | expand |
On mercoledì 29 marzo 2023 09:32:11 CEST Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > Hi list, > > Sorry for a long delay since v1 [1]. This patchset is based on 197b6b6 > (Linux 6.3-rc4). > > Welcome and thanks for your review and comments! > > > # Purpose of this patchset > > The purpose of this pacthset is to replace all uses of kmap_atomic() in > i915 with kmap_local_page() because the use of kmap_atomic() is being > deprecated in favor of kmap_local_page()[1]. And 92b64bd (mm/highmem: > add notes about conversions from kmap{,_atomic}()) has declared the > deprecation of kmap_atomic(). > > > # Motivation for deprecating kmap_atomic() and using kmap_local_page() > > The main difference between atomic and local mappings is that local > mappings doesn't disable page faults or preemption (the preemption is > disabled for !PREEMPT_RT case, otherwise it only disables migration). > > With kmap_local_page(), we can avoid the often unwanted side effect of > unnecessary page faults and preemption disables. > > > # Patch summary > > Patch 1, 4-6 and 8-9 replace kamp_atomic()/kunmap_atomic() with > kmap_local_page()/kunmap_local() directly. With thses local > mappings, the page faults and preemption are allowed. > > Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace > kamp_atomic()/kunmap_atomic(). These two variants of memcpy() > are based on the local mapping, so page faults and preemption > are also allowed in these two interfaces. > > Patch 3 replaces kamp_atomic()/kunmap_atomic() with kmap_local_page()/ > kunmap_local() and also diable page fault since the for special > handling (pls see the commit message). > > > # Changes since v1 > > * Dropped hot plug related description in commit message since it has > nothing to do with kmap_local_page(). > * Emphasized the motivation for using kmap_local_page() in commit > message. > * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to > keep the "idx" variable of type pgoff_t here. > * Used memcpy_from_page() and memcpy_to_page() to replace > kmap_local_page() + memcpy() in patch 2. > > > # Reference > > [1]: > https://lore.kernel.org/lkml/20221017093726.2070674-1-zhao1.liu@linux.intel.c > om/ [1]: > https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com --- > Zhao Liu (9): > drm/i915: Use kmap_local_page() in gem/i915_gem_object.c > drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c > drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c > drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c > drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c > drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c > drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c > drm/i915: Use kmap_local_page() in i915_cmd_parser.c > drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c > I _think_ that the "long delay" you mentioned in the first sentence has paid off in full. I don't see things to improve (except all those "kamp_atomic()" typo in the patches summary; however, typos are only in the cover so I'm sure they won't hurt anybody). Each of the nine patches listed above looks good to me, so they are all… Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Thanks! Fabio PS: Obviously there was no need to reconfirm my tag for patch 3/9. A single tag that catches all patches is easier for a lazy person like me :-) > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++----- > drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++----- > drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++-------- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++-- > drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++--- > .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++-------- > .../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++---- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +---- > drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++-- > 9 files changed, 28 insertions(+), 41 deletions(-) > > -- > 2.34.1
Hi Fabio, On Wed, Mar 29, 2023 at 06:03:38PM +0200, Fabio M. De Francesco wrote: > Date: Wed, 29 Mar 2023 18:03:38 +0200 > From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com> > Subject: Re: [PATCH v2 0/9] drm/i915: Replace kmap_atomic() with > kmap_local_page() > > On mercoledì 29 marzo 2023 09:32:11 CEST Zhao Liu wrote: > > From: Zhao Liu <zhao1.liu@intel.com> > > > > Hi list, > > > > Sorry for a long delay since v1 [1]. This patchset is based on 197b6b6 > > (Linux 6.3-rc4). > > > > Welcome and thanks for your review and comments! > > > > > > # Purpose of this patchset > > > > The purpose of this pacthset is to replace all uses of kmap_atomic() in > > i915 with kmap_local_page() because the use of kmap_atomic() is being > > deprecated in favor of kmap_local_page()[1]. And 92b64bd (mm/highmem: > > add notes about conversions from kmap{,_atomic}()) has declared the > > deprecation of kmap_atomic(). > > > > > > # Motivation for deprecating kmap_atomic() and using kmap_local_page() > > > > The main difference between atomic and local mappings is that local > > mappings doesn't disable page faults or preemption (the preemption is > > disabled for !PREEMPT_RT case, otherwise it only disables migration). > > > > With kmap_local_page(), we can avoid the often unwanted side effect of > > unnecessary page faults and preemption disables. > > > > > > # Patch summary > > > > Patch 1, 4-6 and 8-9 replace kamp_atomic()/kunmap_atomic() with > > kmap_local_page()/kunmap_local() directly. With thses local > > mappings, the page faults and preemption are allowed. > > > > Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace > > kamp_atomic()/kunmap_atomic(). These two variants of memcpy() > > are based on the local mapping, so page faults and preemption > > are also allowed in these two interfaces. > > > > Patch 3 replaces kamp_atomic()/kunmap_atomic() with kmap_local_page()/ > > kunmap_local() and also diable page fault since the for special > > handling (pls see the commit message). > > > > > > # Changes since v1 > > > > * Dropped hot plug related description in commit message since it has > > nothing to do with kmap_local_page(). > > * Emphasized the motivation for using kmap_local_page() in commit > > message. > > * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to > > keep the "idx" variable of type pgoff_t here. > > * Used memcpy_from_page() and memcpy_to_page() to replace > > kmap_local_page() + memcpy() in patch 2. > > > > > > # Reference > > > > [1]: > > https://lore.kernel.org/lkml/20221017093726.2070674-1-zhao1.liu@linux.intel.c > > om/ [1]: > > https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com --- > > Zhao Liu (9): > > drm/i915: Use kmap_local_page() in gem/i915_gem_object.c > > drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c > > drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c > > drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c > > drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c > > drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c > > drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c > > drm/i915: Use kmap_local_page() in i915_cmd_parser.c > > drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c > > > > I _think_ that the "long delay" you mentioned in the first sentence has paid > off in full. > > I don't see things to improve (except all those "kamp_atomic()" typo in the > patches summary; however, typos are only in the cover so I'm sure they won't > hurt anybody). Thanks a lot for your patience and your help! :-) > > Each of the nine patches listed above looks good to me, so they are all… > > Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > Thanks! > > Fabio > > PS: Obviously there was no need to reconfirm my tag for patch 3/9. A single > tag that catches all patches is easier for a lazy person like me :-) The typos and this description still can be improved. I'll pay attention in the future! Thanks, Zhao > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++----- > > drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++----- > > drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++-------- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++-- > > drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++--- > > .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++-------- > > .../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++---- > > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +---- > > drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++-- > > 9 files changed, 28 insertions(+), 41 deletions(-) > > > > -- > > 2.34.1 > > > >
From: Zhao Liu <zhao1.liu@intel.com> Hi list, Sorry for a long delay since v1 [1]. This patchset is based on 197b6b6 (Linux 6.3-rc4). Welcome and thanks for your review and comments! # Purpose of this patchset The purpose of this pacthset is to replace all uses of kmap_atomic() in i915 with kmap_local_page() because the use of kmap_atomic() is being deprecated in favor of kmap_local_page()[1]. And 92b64bd (mm/highmem: add notes about conversions from kmap{,_atomic}()) has declared the deprecation of kmap_atomic(). # Motivation for deprecating kmap_atomic() and using kmap_local_page() The main difference between atomic and local mappings is that local mappings doesn't disable page faults or preemption (the preemption is disabled for !PREEMPT_RT case, otherwise it only disables migration). With kmap_local_page(), we can avoid the often unwanted side effect of unnecessary page faults and preemption disables. # Patch summary Patch 1, 4-6 and 8-9 replace kamp_atomic()/kunmap_atomic() with kmap_local_page()/kunmap_local() directly. With thses local mappings, the page faults and preemption are allowed. Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace kamp_atomic()/kunmap_atomic(). These two variants of memcpy() are based on the local mapping, so page faults and preemption are also allowed in these two interfaces. Patch 3 replaces kamp_atomic()/kunmap_atomic() with kmap_local_page()/ kunmap_local() and also diable page fault since the for special handling (pls see the commit message). # Changes since v1 * Dropped hot plug related description in commit message since it has nothing to do with kmap_local_page(). * Emphasized the motivation for using kmap_local_page() in commit message. * Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to keep the "idx" variable of type pgoff_t here. * Used memcpy_from_page() and memcpy_to_page() to replace kmap_local_page() + memcpy() in patch 2. # Reference [1]: https://lore.kernel.org/lkml/20221017093726.2070674-1-zhao1.liu@linux.intel.com/ [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com --- Zhao Liu (9): drm/i915: Use kmap_local_page() in gem/i915_gem_object.c drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c drm/i915: Use kmap_local_page() in i915_cmd_parser.c drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +++++----- drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++----- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++-------- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++++-- drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++--- .../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12 ++++-------- .../gpu/drm/i915/gem/selftests/i915_gem_context.c | 8 ++++---- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +---- drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++-- 9 files changed, 28 insertions(+), 41 deletions(-)