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:   Fri, 30 Dec 2022 14:31:17 -0800
From:   Ira Weiny <ira.weiny@intel.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
CC:     Nicolas Pitre <nico@fluxnic.net>,
        Christian Brauner <brauner@kernel.org>,
        <linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>,
        "Venkataramanan, Anirudh" <anirudh.venkataramanan@intel.com>
Subject: Re: [PATCH] fs/cramfs: Convert kmap() to kmap_local_data()
Message-ID: <Y69mtbmmbDcXhiln@iweiny-desk3>
References: <20221012091356.16632-1-fmdefrancesco@gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20221012091356.16632-1-fmdefrancesco@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NnpLNDlYdXJoOEdaSHlreE1xZmhMcnZScGNkSkxldGJCc0lnT3Y0Nk9EWFFH?=
 =?utf-8?B?QjBNNHVHT1RzSGt3TGZEQXhKZXROWWY1Ny9QLzRXbXdtSnN3WmRnMkY3MWVi?=
 =?utf-8?B?L0RVczFwSW1VdmVNcUlOZTNpSnd3NWRmTmpvZk9NVnFDU1RrTVBocFVPdnZ0?=
 =?utf-8?B?Y3RzOFNhS0VWN3N4RW1vSy9DTjVRZHhNMS85U0dLRWxQUXVkZVg3YTIzYUJo?=
 =?utf-8?B?RVcraDhyb0FiNitUSkJpU0wwdU5yMUhDejE1N2E1UmkreDRtU1FBUHFpK0RT?=
 =?utf-8?B?TmtWTXBMWVZIdVVTMno3QldJMVhTaEx3QitaNjQrWVhrd0hYL1NDeWRBR1hu?=
 =?utf-8?B?aFEvd2V2WXFaWWJUTE05YnlwUmtMVk95V1c3MnlBZk1mTEFRSmRQYUI0aVdr?=
 =?utf-8?B?eU1Hd2R0S3NOQmtockJjRnBBOWhsMU54TEFtRlNWRDVod2lGcWIwY21DaXNN?=
 =?utf-8?B?cUNoUWIzVWhITGZPS0RrTWk1N2p6YVRFRGJVNko3QnlZTjRBZ3FkdWJmZVJE?=
 =?utf-8?B?cjJpa0UrQXIvUXExbjZHUXVoREF2dDRsS1J3Mk4xdmovZE9Ra0ZySXhwbUxT?=
 =?utf-8?B?TkQ3K0U4c0tuWk1WdmZlWFdHZU5weDNHUEpqc1pqTEhwNGpxMG92SU03b3N3?=
 =?utf-8?B?a2c4bThDQlFJWldhMEhEUlp5L0pHeUhpUFpYYlAybjk0YTUrak9uTmxSbnBj?=
 =?utf-8?B?SEh3MDFLbjhiV2xpbUgwMUJqczI2b0JRYnJVc0pjNFo1S0REZWYyRmNDa3Zq?=
 =?utf-8?B?aU1pSzM1a1ZobkQyS2pIa0lZTW9tQW4wVzdMOVd6YWQrUHhsdVNaamNHd1Fk?=
 =?utf-8?B?eDlqeE8rN3FQMDZhUVZwdkJVYnkydVhIdWEyYUFjdkFFZTNvdGN4NWl3UU9R?=
 =?utf-8?B?cER1VE5WdnFmRlE3RG5mYzExdDFlZzN6M3NNTWRSRWZrbnhTZW5NSWpySlRO?=
 =?utf-8?B?V0REVUVMYzN5OW5zUFBzN3FNNU0vOHBMSndXQzVVTkkzTWNWQWJacjdaeG1z?=
 =?utf-8?B?YnF1THRxUEo1amhqUE1mZ0Rsa1Ura0dJWERKY0htaWdHR2xXNDRqTkxUMXl5?=
 =?utf-8?B?WWVFa1FCa1daWWZiSU04OE5ZMDRTNnF2WXF1UWE5SFJiUzdkakFVdjliUDgr?=
 =?utf-8?B?V28waHNwSGxpQkFRNFBOVmFuS2NJNGxXZFNLdytiUmpMK3Boa0RkWWtQakR1?=
 =?utf-8?B?MllUNEMrK2lxS25SaG4rbGl1Zk91aTZOQUxIVTdDZ3pPMUxVcUxDQ0hBTDVL?=
 =?utf-8?B?Y1ZrKytQbW1GQUg1ejhxZlVpQyt4TDVMT3grejVZU2tBU3k2dzZmZGVoWlo5?=
 =?utf-8?B?bHU3MGRaT0JpS0tvMWlXOWtPSjFMTC94RkZkRVE1SlNVaVk5VnFWUWpERDVp?=
 =?utf-8?B?NG9URkkxalI1NXhUQm1wQzc1ZnVQa2sxem5kT1dKbDNDWjNEbUNzYzVseUxO?=
 =?utf-8?B?aitKZkZINGExanNtQVR3VDhPcmc1RlZ1VUpJTnVaZW5qdHJGekxudnZYLy9Q?=
 =?utf-8?B?WkxXbElrMjRJblRFcWFWTnpDeFMrUG9QM0Z6b0tPTllCWmNJMkdFNXBsYWxD?=
 =?utf-8?B?dmJFdGJYNW5JcHhySXJTNkJNemZ4NGFUMEtrMWdOYmhrTEZOYmtpMWRteVFY?=
 =?utf-8?B?SUh2ZlVuU0xseEtxZzNLSjNVdjRqeWFXNy9NRXZ0R0VYWVhVL3kvcEo5bktU?=
 =?utf-8?B?T1BidEkyWVR0aWNoRmdIcFdhS2tvaHZFTi9RbmNGS3pwTW1HaGkwem1ZaHhS?=
 =?utf-8?B?dkFUb3JsUnQ4SDNVNGc2U1lUcmtYU2p4cnhKbTdpbEY0UGlHWUhVTUtnVWZ4?=
 =?utf-8?B?QklUL1ZFMnJNQXI0Qlh5cExBMDN3d3U2dkFmOWJFMmg1eUpsUis3eTlBOUJY?=
 =?utf-8?B?TDVHK0ROcW1RSEVZLzFtaG1Sa21qYjc5ZXl2bC8vL2tTOGwyaGZoSW16VExq?=
 =?utf-8?B?UjR4amhwcVJkUkZBSDRoWTUxSTBTcW5WUWFEczRQYWRaZzJlY2xJN1lJUzQ4?=
 =?utf-8?B?TEJERVNZaVAvOUV2eWE2WmoxemN4aFNTVGpzckF5NXBxR0FKKzJsTE12NWtQ?=
 =?utf-8?B?U0JWSVRzNnRnbndRZXdrZ0RLVGNpa1N0RXNlbC9wVWsrR0J5L0k4anN4aU8z?=
 =?utf-8?Q?oxA2X8c301N8+UC7CGKqVCTUY?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3147af90-fb0a-41ca-e12c-08daeab5939e
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Dec 2022 22:31:21.4239
 (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: 6RzvdTM5qGhy7FHm84IlKS5bDWg+MPATzCgpbMHI0HUjq2WEkSy8fkQ+J0uKFdb7ph5NXcI0s6eHrFyTLjqgfg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6203
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.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 Wed, Oct 12, 2022 at 11:13:56AM +0200, Fabio M. De Francesco wrote:
> The use of kmap() is being deprecated in favor of kmap_local_page().
> 
> There are two main problems with kmap(): (1) It comes with an overhead as
> the mapping space is restricted and protected by a global lock for
> synchronization and (2) it also requires global TLB invalidation when the
> kmap’s pool wraps and it might block when the mapping space is fully
> utilized until a slot becomes available.
> 
> With kmap_local_page() the mappings are per thread, CPU local, can take
> page faults, and can be called from any context (including interrupts).
> It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
> the tasks can be preempted and, when they are scheduled to run again, the
> kernel virtual addresses are restored and still valid.
> 
> Since its use in fs/cramfs is safe everywhere, it should be preferred.
> 
> Therefore, replace kmap() with kmap_local_page() in fs/cramfs. Instead
> of open-coding kmap_local_page() + memcpy(), use memcpy_from_page().
> 
> Cc: "Venkataramanan, Anirudh" <anirudh.venkataramanan@intel.com>
> Suggested-by: Ira Weiny <ira.weiny@intel.com>

Nicolas, I was auditing the kmap conversion status and I see you reviewed this
but did not merge it.  Are you still the maintainer for fs/cramfs?  Should
Fabio send this to someone else?

FWIW LGTM

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

> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>  fs/cramfs/inode.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> index 61ccf7722fc3..c17cbba5d45e 100644
> --- a/fs/cramfs/inode.c
> +++ b/fs/cramfs/inode.c
> @@ -238,8 +238,7 @@ static void *cramfs_blkdev_read(struct super_block *sb, unsigned int offset,
>  		struct page *page = pages[i];
>  
>  		if (page) {
> -			memcpy(data, kmap(page), PAGE_SIZE);
> -			kunmap(page);
> +			memcpy_from_page(data, page, 0, PAGE_SIZE);
>  			put_page(page);
>  		} else
>  			memset(data, 0, PAGE_SIZE);
> @@ -815,7 +814,7 @@ static int cramfs_read_folio(struct file *file, struct folio *folio)
>  
>  	maxblock = (inode->i_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>  	bytes_filled = 0;
> -	pgdata = kmap(page);
> +	pgdata = kmap_local_page(page);
>  
>  	if (page->index < maxblock) {
>  		struct super_block *sb = inode->i_sb;
> @@ -903,13 +902,13 @@ static int cramfs_read_folio(struct file *file, struct folio *folio)
>  
>  	memset(pgdata + bytes_filled, 0, PAGE_SIZE - bytes_filled);
>  	flush_dcache_page(page);
> -	kunmap(page);
> +	kunmap_local(pgdata);
>  	SetPageUptodate(page);
>  	unlock_page(page);
>  	return 0;
>  
>  err:
> -	kunmap(page);
> +	kunmap_local(pgdata);
>  	ClearPageUptodate(page);
>  	SetPageError(page);
>  	unlock_page(page);
> -- 
> 2.37.3
>