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:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     Ira Weiny <ira.weiny@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Will Deacon <will@kernel.org>,
        Peter Collingbourne <pcc@google.com>,
        Vlastimil Babka <vbabka@suse.cz>, linux-kernel@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org,
        outreachy@lists.linux.dev,
        "Acked-by : Mike Rapoport" <rppt@linux.ibm.com>
Subject: Re: [PATCH v2 1/4] mm/highmem: Fix kernel-doc warnings in highmem*.h
Date:   Tue, 26 Apr 2022 11:43:03 +0200
Message-ID: <4396926.LvFx2qVVIh@leap>
In-Reply-To: <YmeYzKT8Ikq5SfdE@linutronix.de>
References: <20220425162400.11334-1-fmdefrancesco@gmail.com> <20220425162400.11334-2-fmdefrancesco@gmail.com> <YmeYzKT8Ikq5SfdE@linutronix.de>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
Precedence: bulk

On marted=C3=AC 26 aprile 2022 09:01:32 CEST Sebastian Andrzej Siewior wrot=
e:
> On 2022-04-25 18:23:57 [+0200], Fabio M. De Francesco wrote:
> > index a77be5630209..aa22daeed617 100644
> > --- a/include/linux/highmem-internal.h
> > +++ b/include/linux/highmem-internal.h
> > @@ -236,9 +236,17 @@ static inline unsigned long totalhigh_pages(void)=
=20
{ return 0UL; }
> > =20
> >  #endif /* CONFIG_HIGHMEM */
> > =20
> > -/*
> > - * Prevent people trying to call kunmap_atomic() as if it were=20
kunmap()
> > - * kunmap_atomic() should get the return value of kmap_atomic, not the=
=20
page.
> > +/**
> > + * kunmap_atomic - Unmap the virtual address mapped by kmap_atomic()
> > + * @__addr:       Virtual address to be unmapped
> > + *
> > + * Unmaps an address previously mapped by kmap_atomic() and re-enables
> > + * pagefaults and preemption. Mappings should be unmapped in the=20
reverse
>=20
> You mind adding "Deprecated!" like kmap_atomic() has?=20

I might add "Deprecated!", however Ira Weiny asked me to rephrase an=20
earlier version of one of the patch which is is this series. I wrote that=20
"The use of kmap_atomic() is deprecated in favor of kmap_local_page()." and=
=20
Ira replied "I'm not sure deprecated is the right word. [] This series=20
should end up indicating the desire to stop growing kmap() and
kmap_atomic() call sites and that their deprecation is on the horizon.".

What Ira suggested is exactly what I'm doing in v2.=20

@Ira: what about adding "Deprecated!" for consistency with kmap_atomic()=20
kdoc?

> The part about
> disabling/ enabling preemption is true for !PREEMPT_RT.

To me it looks that this is not what Thomas Gleixner wrote in the cover=20
letter of his series ("[patch V2 00/18] mm/highmem: Preemptible variant of=
=20
kmap_atomic & friends") at=20
https://lore.kernel.org/lkml/20201029221806.189523375@linutronix.de/

=46or your convenience:

"[] there is not a real reason anymore to confine migration disabling to=20
RT. [] Removing the RT dependency from migrate_disable/enable()".

Is there anything I'm still missing?

> The part that
> worries me is that people use it and rely on disabled preemption like
> some did in the past.=20

This is something I'd prefer to hear also from other developers who are=20
CC'ed for this patch :)=20

> I've been told this API is about to be removed (or so I have been told)
> so I hope that it will be gone soon ;)
>=20
> > + * order that they were mapped. See kmap_local_page() for details.
> > + * @__addr can be any address within the mapped page, so there is no=20
need
> > + * to subtract any offset that has been added. In contrast to=20
kunmap(),
> > + * this function takes the address returned from kmap_atomic(), not=20
the
> > + * page passed to it. The compiler will warn you if you pass the page.
> >   */
> >  #define kunmap_atomic(__addr)				=09
\
> >  do {							=09
\
>=20
> Sebastian
>=20

Thanks for your review,

=46abio