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:     Baoquan He <bhe@redhat.com>
Cc:     Eric Biederman <ebiederm@xmission.com>, kexec@lists.infradead.org,
        akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
        Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
Date:   Tue, 02 Aug 2022 13:02:25 +0200
Message-ID: <5847084.alqRGMn8q6@opensuse>
In-Reply-To: <YujTHv0xVG8DirkW@MiWiFi-R3L-srv>
References: <20220707231550.1484-1-fmdefrancesco@gmail.com> <1923146.PYKUYFuaPT@opensuse> <YujTHv0xVG8DirkW@MiWiFi-R3L-srv>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
Precedence: bulk

On marted=C3=AC 2 agosto 2022 09:32:46 CEST Baoquan He wrote:
> On 08/02/22 at 08:20am, Fabio M. De Francesco wrote:
> > On marted=C3=AC 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> > > On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > > > The use of kmap() and kmap_atomic() are being deprecated in favor=20
of
> > > > kmap_local_page().
> > > >=20
> > > > With kmap_local_page(), the mappings are per thread, CPU local and=
=20
not
> > > > globally visible. Furthermore, the mappings can be acquired from=20
any
> > > > context (including interrupts).
> > > >=20
> > > > Therefore, use kmap_local_page() in kexec_core.c because these=20
mappings=20
> > are
> > > > per thread, CPU local, and not globally visible.
> > > >=20
> > > > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > > > enabled.
> > >=20
> > > Wondering what arch you tested with.
> >=20
> > I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
> > This is usually an information that I add in the commit messages of all=
=20
the=20
> > recent conversions I'm working on across the entire kernel.
> >=20
> > Another important information (overlooked again this time) is that (1)=
=20
> > kmap() comes with an overhead as mapping space is restricted and=20
protected=20
> > by a global lock for synchronization and (2) it also requires global=20
TLB=20
> > invalidation when the kmap=E2=80=99s pool wraps and it might block when=
 the=20
mapping=20
> > space is fully utilized until a slot becomes available.
>=20
> Thanks a lot for update with more details, Fabio.

You're welcome.

> Maybe these can be
> added into log with v3 if you think they are worth adding, and with my
> Ack since no code change related. You decide.

=46or weeks I've been thinking that these details were not necessary in the=
=20
commit message of these conversions. Later I realized that a fair number of=
=20
maintainers weren't aware of why we should change something that had been=20
working "properly" for years. Perhaps they thought that we would face too=20
many risks of breaking things versus little rewards.

After discussing with Greg K-H (who was initially not fond of these changes=
=20
to core parts of firmware loading mechanism), he suggested that a proper=20
commit log with the necessary information could have helped himself and the=
=20
other maintainers who may not yet had time to deepen this topics.

This patch to "kexec" slipped in with no such additions.

Therefore, I agree with you in full: I'll add these information and send a=
=20
new version ASAP.

> Thanks again for the effort.

Thanks so much for the "Acked-by:" tag from you.

=46abio =20

[snip]