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:   Wed, 12 Oct 2022 14:54:03 -0400 (EDT)
From:   Nicolas Pitre <nico@fluxnic.net>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
cc:     Christian Brauner <brauner@kernel.org>,
        linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
        "Venkataramanan, Anirudh" <anirudh.venkataramanan@intel.com>,
        Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH] fs/cramfs: Convert kmap() to kmap_local_data()
In-Reply-To: <20221012091356.16632-1-fmdefrancesco@gmail.com>
Message-ID: <87672pr-o821-qns6-11rp-902sp1s34r3@syhkavp.arg>
References: <20221012091356.16632-1-fmdefrancesco@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1044884447-1665600843=:1258"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1044884447-1665600843=:1258
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Wed, 12 Oct 2022, Fabio M. De Francesco wrote:

> The use of kmap() is being deprecated in favor of kmap_local_page().
>=20
> 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 t=
he
> kmap=E2=80=99s pool wraps and it might block when the mapping space is =
fully
> utilized until a slot becomes available.
>=20
> 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, t=
he
> kernel virtual addresses are restored and still valid.
>=20
> Since its use in fs/cramfs is safe everywhere, it should be preferred.
>=20
> Therefore, replace kmap() with kmap_local_page() in fs/cramfs. Instead
> of open-coding kmap_local_page() + memcpy(), use memcpy_from_page().
>=20
> Cc: "Venkataramanan, Anirudh" <anirudh.venkataramanan@intel.com>
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Reviewed-by: Nicolas Pitre <nico@fluxnic.net>

> ---
>  fs/cramfs/inode.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>=20
> 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 =3D pages[i];
> =20
>  		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, str=
uct folio *folio)
> =20
>  	maxblock =3D (inode->i_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
>  	bytes_filled =3D 0;
> -	pgdata =3D kmap(page);
> +	pgdata =3D kmap_local_page(page);
> =20
>  	if (page->index < maxblock) {
>  		struct super_block *sb =3D inode->i_sb;
> @@ -903,13 +902,13 @@ static int cramfs_read_folio(struct file *file, s=
truct folio *folio)
> =20
>  	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;
> =20
>  err:
> -	kunmap(page);
> +	kunmap_local(pgdata);
>  	ClearPageUptodate(page);
>  	SetPageError(page);
>  	unlock_page(page);
> --=20
> 2.37.3
>=20
>=20
--8323328-1044884447-1665600843=:1258--