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, 3 Nov 2022 09:49:38 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Zhao Liu <zhao1.liu@linux.intel.com>
CC:     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>,
        "Fabio M . De Francesco" <fmdefrancesco@gmail.com>,
        Zhenyu Wang <zhenyu.z.wang@intel.com>,
        Zhao Liu <zhao1.liu@intel.com>
Subject: Re: [PATCH 3/9] drm/i915: Use kmap_local_page() in
 gem/i915_gem_shmem.c
Message-ID: <Y2PxImhTMtLijeab@iweiny-desk3>
References: <20221017093726.2070674-1-zhao1.liu@linux.intel.com>
 <20221017093726.2070674-4-zhao1.liu@linux.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221017093726.2070674-4-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?V8BPV7LFzscsihnWiQnA2YfRDqcKya2rtbpwxyb0ltfuzVFZHI+yC/ewyQHG?=
 =?us-ascii?Q?j00pV7hI6PjuXGabHuLKUNfeYHJbTJ4tXxkyVMdVDbvtCg+u30/TYvDbhEM7?=
 =?us-ascii?Q?Ci9FifgH06j0IIwifKbul8BP8RrMhbtM3Uq9+U+tseh3ahHQpxb3web1Yu71?=
 =?us-ascii?Q?ajvqzqo0QsafIAinhW/sN6/OYwLnYBE5VTz/AhBWA8q7U/cAHeqG8m5ZFG6z?=
 =?us-ascii?Q?xur1xzn39pUMw+LVbfyvcLdvbR2orsNLeGjl5mSxlX6aANT0Rx8jYz/tNsoJ?=
 =?us-ascii?Q?qc22e2gK3cnhI4tuAFXgHfTkd0Yd/eGxWBdgkvSB5vmujSScqNU0HWpMrpl+?=
 =?us-ascii?Q?l2+CcY3Si0jk1clNoRb6mDi7WojAqAjxGWlBRrNLBAoId/RLTD1iDuR/hOui?=
 =?us-ascii?Q?aVE5CvY0QA8MhBoBc9kCSqOwxRGLE87ZxABjp3wOZ5Pn7poA+8fhNDI6WIcp?=
 =?us-ascii?Q?14nGISEgW2sR7ZSrhu3h1rHCPXgfKdsgs+IWQLrCppub3UvHCYaSjgXbZv2F?=
 =?us-ascii?Q?ZN1W3K86Wnobyo0Qlk5NINFqAWZHr3d1P2HH+S10VYgZjWIsztt1qZLZk1nc?=
 =?us-ascii?Q?xe7wZlMMxGLzJgVH2wq7sz6nPKHTLIYPFF76yeErNk3+1GsAYvv0vqhDiLEk?=
 =?us-ascii?Q?ubYpr0uFpnG9xSyOyfxZ8iTyFAQ6DXKMyCkBeqJCsDRatMrFVbDdlBfg1f6J?=
 =?us-ascii?Q?/7zgvORo14mhMkW1eIfezfSyhc5HxhOD9BjDki1cHy26zH8Er1lYg/tWREDF?=
 =?us-ascii?Q?1RMiMbvPLtUsrYt4R2hStsXnLcjY+zPpWNHtt1VjpYFIK6cE5Y4O5xdhcVan?=
 =?us-ascii?Q?hJyNXOwinUtyS+5633iJsyxWJcmPrXTbUIqZWxZ3In+QFevyrRh4j2rIs93J?=
 =?us-ascii?Q?9l4q7FIm9l/jvU7luwtj7OZ1kwixQGKX6nCu8jkZCTo5XSO9B7B5GKKWUeVQ?=
 =?us-ascii?Q?VMlnXa8udr+b1a584FQFfLElv0X6yriQ8eYvEVDiOK5UMmJXjTd/vf8CZyLC?=
 =?us-ascii?Q?g0UDQPKcr8fS0H8GVNzq6N9v2byiMFyyZ137VU7sJ5N1nYnATr6lHPR8ahiR?=
 =?us-ascii?Q?SSU2M7QAZjdoTOZB643Q/tz9nIBGUCu7eOALPDrRuMrciSV3izEVbUVORCF4?=
 =?us-ascii?Q?UmkVR15woOGI7IWc3A3bnvsc9XYoScWdHZxZA9QVZaET5yOq6SXXujY0Z+Xa?=
 =?us-ascii?Q?B8egsGCmOlAa0l2yGp7vX2Zf4ov061On9TCapz6FxrTpOJiS8HvMMyv1jr/o?=
 =?us-ascii?Q?fylqSXRaBkxd7SXPs0xr6LHzazcC0wMKuE7olH5qJPRFb/gWOo/skiFSP9WP?=
 =?us-ascii?Q?/0xlwDU6IyYbzDoQjHih60aLvjIw6+Wtmo5VXHuatN1wJqXUtdG0D/wjLQ2A?=
 =?us-ascii?Q?8Qb3nPvzUyf+vh8sWDuRAGzuWp0FcP5nxd8ZUJla5AQzehs3i4+gqOz9zsoJ?=
 =?us-ascii?Q?DFEhHKNq0VbnkqeDgMd4FXEmzMk1GULpGdI0tMJtcsmbLfSWycXCoq8mPF7d?=
 =?us-ascii?Q?vVMr01EOh0xF8XrmwAGFso+NoTPxAAr/VUOmKZRrcMVe4jfs1ODudgLHvXgi?=
 =?us-ascii?Q?W1zEiPIb38e8OdPKaIT0jqWeawWtTv+z15ssufBi?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6e30eb40-7fe3-4c8e-72dc-08dabdbb68a8
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2022 16:49:44.0625
 (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: Az6nSjTcst9otVl99n0BOnqeMMw3zMSjV7sWzS/ygNfbWez1PIlDjg1faJtKhA9Tzplz2CTXevez/I4yid/PfQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB7150
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham 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

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
>