Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 30 Mar 2023 20:36:44 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Zhao Liu <zhao1.liu@linux.intel.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Matthew Auld <matthew.auld@intel.com>,
        Thomas =?iso-8859-1?Q?Hellstr=F6m?= 
        <thomas.hellstrom@linux.intel.com>,
        Nirmoy Das <nirmoy.das@intel.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Chris Wilson <chris@chris-wilson.co.uk>,
        Christian =?iso-8859-1?Q?K=F6nig?= <christian.koenig@amd.com>,
        <intel-gfx@lists.freedesktop.org>,
        <dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
CC:     Ira Weiny <ira.weiny@intel.com>,
        "Fabio M . De Francesco" <fmdefrancesco@gmail.com>,
        Zhenyu Wang <zhenyu.z.wang@intel.com>,
        Zhao Liu <zhao1.liu@intel.com>,
        Dave Hansen <dave.hansen@intel.com>
Subject: Re: [PATCH v2 8/9] drm/i915: Use kmap_local_page() in
 i915_cmd_parser.c
Message-ID: <6426554c3db9a_375f7e294af@iweiny-mobl.notmuch>
References: <20230329073220.3982460-1-zhao1.liu@linux.intel.com>
 <20230329073220.3982460-9-zhao1.liu@linux.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230329073220.3982460-9-zhao1.liu@linux.intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?zNVHNJctuHkkDWe0lKDFm+AZSt8f/aSe0WfX0bYsKsRiAbdE/PkKZTrBH55B?=
 =?us-ascii?Q?boBsCG9WVAry6apmf6mMhUYv5SGg02673HMsF6tDs8UBz60b09EFFgjxv9d6?=
 =?us-ascii?Q?i1WFao+YwqqFoWZgpayKV0NR0tnVRUVyn0Olt1KZnoL6Z16gN+FXjoEpHV3E?=
 =?us-ascii?Q?0EWy3wzpSMDdzOOraZhartchuKLG6IEbPIcyONLGfp47Hi1v3tSjsJRUXBek?=
 =?us-ascii?Q?4+8mNEhe3IWx3wo6HDompUuPCbSSP6kFB+URng3sS3kWdbMxLeuj5n1coNP8?=
 =?us-ascii?Q?r1LsgDw+VE8gsOY/WT8IJz8OeIaDW6ZdWgMAkdSaHyEoGAYumRFZvKGidB/1?=
 =?us-ascii?Q?kZloObge0/PczNrsNrcZk01b/0sA9/U1foDy8Y5n5LQfmdQERNhzF74ZY/VW?=
 =?us-ascii?Q?TtJ+ZPzMUr3saIy/LFqsGtjLJWUH77mdIUwqxvGj/1TWLXXqQDDb8tztsMiI?=
 =?us-ascii?Q?Udxltf7tQ8ueXEBGaVtftJCP9bRzmnVde1XbGlHMKLQbLkuT5YMP64T4uM26?=
 =?us-ascii?Q?L0Oxu/gTWozADlG+pJ61hTO0cafhdM2jbJNBrzBsF0XRboI65GUCUL6k/842?=
 =?us-ascii?Q?zty4hcnzeQksrFt2BgHtWmLXXAtEAgi0uVhITkzcK9rFrgonewqjWQilH3Th?=
 =?us-ascii?Q?SEEJGPZA1x21cQEDFYssDN83TBC1Ze+RgOmOZqn3JiwlGz67nsnHHAu2DDNk?=
 =?us-ascii?Q?zg1XlGZifB77lbEuicLFc6XRs0GkTEtuUMUYKZt6QCE14zM0BcbuoB6vYl6P?=
 =?us-ascii?Q?T1WAr7YQRZ150H0SCEhEDhACZdBQsRCJXfOHbBt0TlrKeQ16EFJE5LC10/uy?=
 =?us-ascii?Q?uRZsx8uSlTJXDKBb5BEytWwMz9bPgOvF/CF+bCzCFlVUEGUf35EqT3Vzv0+I?=
 =?us-ascii?Q?F8se9QYGVszfnLhBm0g8lYbBDJ5hC2IoanWZ2pqT8uVywjGgqnIKxjIYD1f0?=
 =?us-ascii?Q?3ope22w2AbQlchu29CK0DH9Pmi8vFxQ9qOYZb3k24EFsX0L05R44hIFplfyJ?=
 =?us-ascii?Q?Uaq7l8LxVI3ORjKtzLr9oyxM53PhBw/xJS9atwKeetZEVyFJcLJZrwZbaA0s?=
 =?us-ascii?Q?kqnoIT8J+zfOvLFIhSFQsGyNmtbHln8VhNvJFp3lHfZj67ON0BAryEZujXFm?=
 =?us-ascii?Q?jcAW9GdEJKC3JdUCdiCsTirsmZAP9uhafZPJujFk4BBO5B5MSS7V3oZPLDHp?=
 =?us-ascii?Q?GRgIZM6PH/V3oRRAZG7dnHryCchp9TEkxMOCJ+k1MySRAoG55aJQ1pl92EoQ?=
 =?us-ascii?Q?BlzbPslAJYsCLLNfnquJa333xC4HQMGHEp2mnqB/uQabaP6k9rTRdmZy+/PA?=
 =?us-ascii?Q?+QlEKS4lkTWVElHmbCOc8hyhT3UTlrMVH9/FxzdLXXNHL4o0/NGjJvi5ip4v?=
 =?us-ascii?Q?P16PCQpbF++WceYxMlBq53dorh2vsQoORT5rkPnQ3MZEPMTkymYMutR35+7K?=
 =?us-ascii?Q?UnG0ozJAmc/4Nt7cokEVoly7KKMyx3z4pO3siMvvp4YB9ku41IMJuJ0aoRGz?=
 =?us-ascii?Q?2leGSVeLg4l+4r4LCPsicE3q1n2Ut/jE1GuCeKvO84N9o11VCsvR2ZOthT1M?=
 =?us-ascii?Q?5iTKjzn9X9vQMx0nu2QK3/8zabAIBWdDhElUhZJQ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b01cc12b-6b1c-44c3-1924-08db319928a5
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Mar 2023 03:36:48.7428
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Q3zBTSqK0GlvSmDc3ftQ11OuHn5dizsrMvIEgF0N8NGH3D2Uo3sKH2O4EZvu3uCgehIR0U8j1PfjEE9i/+Ll9w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR11MB6411
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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], and this patch converts the call from
> kmap_atomic() to 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.
> 
> There're 2 reasons why function copy_batch() doesn't need to disable
> pagefaults and preemption for mapping:
> 
> 1. The flush operation is safe. In i915_cmd_parser.c, copy_batch() calls
> drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
> Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu
> in drm_clflush_virt_range(), the flush operation is global.
> 
> 2. Any context switch caused by preemption or page faults (page fault
> may cause sleep) doesn't affect the validity of local mapping.
> 
> Therefore, copy_batch() is a function where the use of
> kmap_local_page() in place of kmap_atomic() is correctly suited.
> 
> Convert the calls of kmap_atomic() / kunmap_atomic() to
> kmap_local_page() / kunmap_local().
> 
> [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com
> 
> v2:
> * Dropped hot plug related description since it has nothing to do with
>   kmap_local_page().
> * No code change since v1, and added description of the motivation of
>   using kmap_local_page().
> 
> Suggested-by: Dave Hansen <dave.hansen@intel.com>
> Suggested-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>