On Thu, Nov 03, 2022 at 08:22:04PM +0100, Fabio M. De Francesco wrote: > Date: Thu, 03 Nov 2022 20:22:04 +0100 > From: "Fabio M. De Francesco" <fmdefrancesco@xxxxxxxxx> > Subject: Re: [PATCH 1/9] drm/i915: Use kmap_local_page() in > gem/i915_gem_object.c > > On gioved? 3 novembre 2022 17:51:23 CET Ira Weiny wrote: > > On Sat, Oct 29, 2022 at 01:17:03PM +0200, Fabio M. De Francesco wrote: > > > On luned? 17 ottobre 2022 11:37:17 CEST Zhao Liu wrote: > > > > From: Zhao Liu <zhao1.liu@xxxxxxxxx> > > > > > > > > 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. > > > > > > You are right about about page faults which are never disabled by > > > kmap_local_page(). However kmap_atomic might not disable preemption. It > > > depends on CONFIG_PREEMPT_RT. > > > > > > Please refer to how kmap_atomic_prot() works (this function is called by > > > kmap_atomic() when kernels have HIGHMEM enabled). > > > > > > > There're 2 reasons why i915_gem_object_read_from_page_kmap() doesn't > > > > need to disable pagefaults and preemption for mapping: > > > > > > > > 1. The flush operation is safe for CPU hotplug when preemption is not > > > > disabled. > > > > > > I'm confused here. Why are you talking about CPU hotplug? > > > > I agree with Fabio here. I'm not making the connection between cpu hotplug > and > > this code path. > > > > Ira > > @Zhao, > > I'd like to add that I was about to put my reviewed-by tag. The other things I > objected are minor nits. Please just clarify this connection. Thanks Fabio for your comments! Sorry I missed the mails that day. This connection is my misunderstanding. Other thoughts please refer to my reply to your first email in this thread. Thanks, Zhao