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:     Sumitra Sharma <sumitraartsy@gmail.com>,
        Ira Weiny <ira.weiny@intel.com>,
        Thomas =?ISO-8859-1?Q?Hellstr=F6m_=28Intel=29?= 
        <thomas_os@shipmail.org>
Cc:     Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, Deepak R Varma <drv@mailo.com>
Subject: Re: [PATCH v2] drm/i915: Replace kmap() with kmap_local_page()
Date:   Wed, 21 Jun 2023 15:24:29 +0200
Message-ID: <13316568.uLZWGnKmhe@suse>
In-Reply-To: <79e1f37f-3ffa-0195-860b-08cc890d810e@shipmail.org>
References: <20230620180759.GA419158@sumitra.com>
 <79e1f37f-3ffa-0195-860b-08cc890d810e@shipmail.org>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
Precedence: bulk

On mercoled=EC 21 giugno 2023 11:06:51 CEST Thomas Hellstr=F6m (Intel) wrot=
e:
>=20
> I think one thing worth mentioning in the context of this patch is that
> IIRC kmap_local_page() will block offlining of the mapping CPU until
> kunmap_local(),

Migration is disabled.

> so while I haven't seen any guidelines around the usage
> of this api for long-held mappings,

It would be advisable to not use it for long term mappings, if possible. Th=
ese=20
"local" mappings should better be help for not too long duration.=20

> I figure it's wise to keep the
> mapping duration short, or at least avoid sleeping with a
> kmap_local_page() map active.

Nothing prevents a call of preempt_disable() around the section of code=20
between kmap_local_page() / kunmap_local(). If it is needed, local mappings=
=20
could also be acquired under spinlocks and/or with disabled interrupts.

I don't know the code, however, everything cited above could be the subject=
 of=20
a subsequent patch.

Regards,

=46abio

> I figured, while page compression is probably to be considered "slow"
> it's probably not slow enough to motivate kmap() instead of
> kmap_local_page(), but if anyone feels differently, perhaps it should be
> considered.
>=20
> With that said, my Reviewed-by: still stands.
>=20
> /Thomas
>=20