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;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Viacheslav Dubeyko <slava@dubeyko.com>
Cc:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH] hfsplus: Convert kmap() to kmap_local_page() in bitmap.c
Date:   Tue, 26 Jul 2022 14:04:00 +0200
Message-ID: <2050755.bB369e8A3T@opensuse>
In-Reply-To: <A2FB0201-8342-481B-A60C-32A2B0494D33@dubeyko.com>
References: <20220724205007.11765-1-fmdefrancesco@gmail.com> <A2FB0201-8342-481B-A60C-32A2B0494D33@dubeyko.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
Precedence: bulk

On luned=C3=AC 25 luglio 2022 19:17:13 CEST Viacheslav Dubeyko wrote:
>=20
> > On Jul 24, 2022, at 1:50 PM, Fabio M. De Francesco=20
<fmdefrancesco@gmail.com> wrote:
> >=20
> > kmap() is being deprecated in favor of kmap_local_page().
> >=20
> > There are two main problems with kmap(): (1) It comes with an overhead=
=20
as
> > mapping space is restricted and protected by a global lock for
> > synchronization and (2) it also requires global TLB invalidation when=20
the
> > 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,=20
the
> > kernel virtual addresses are restored and are still valid.
> >=20
> > Since its use in bitmap.c is safe everywhere, it should be preferred.
> >=20
> > Therefore, replace kmap() with kmap_local_page() in bnode.c.
> >=20
>=20
> Looks good. Maybe, it makes sense to combine all kmap() related=20
modifications in HFS+ into
> one patchset?
>=20
> Reviewed by: Viacheslav Dubeyko <slava@dubeyko.com>=20

Thanks for your reviews of this and of the other patch to bnode.c.

Actually, I started with the first file I met (bnode.c) because I noticed=20
that maintainers don't need to care about any special ordering for applying=
=20
the patches, since each of them is self-contained.

This is why I haven't thought of making a series of them.

Currently only one file is still left with some kmap() call sites. I'll=20
work on that within the next days.

Again thanks,

=46abio=20

> Thanks,
> Slava.
>=20
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > fs/hfsplus/bitmap.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)

[snip]