Re: [PATCH] x86/sgx: Replace kmap/kunmap_atomic calls

From: Kristen Carlson Accardi
Date: Thu Oct 13 2022 - 12:04:31 EST


On Wed, 2022-10-12 at 18:50 +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 12, 2022 at 05:50:19PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Oct 12, 2022 at 07:13:26AM -0700, Dave Hansen wrote:
> > > On 10/12/22 00:15, Jarkko Sakkinen wrote:
> > > > There's no data to show that this change would be useful to do.
> > >
> > > Jarkko, I think the overall transition to kmap_local_page() is a
> > > good
> > > one.  It is a superior API and having it around will pave the way
> > > for
> > > new features.  I don't think we should demand 'data' for each and
> > > every
> > > one of these.
> > >
> > > Please take a look around the tree and see how other maintainers
> > > are
> > > handling these patches.  They're not limited to SGX.
> >
> > Sure, I'll take a look for comparison.
>
> Yeah, I think it is pretty solid idea.
>
> Looking at the decription:
>
> "It is not necessary to disable page faults or preemption when
> using kmap calls, so replace kmap_atomic() and kunmap_atomic()
> calls with more the more appropriate kmap_local_page() and
> kunmap_local() calls."
>
> We did not pick kmap_atomic() because it disables preeemption,
> i.e. it was not a "design choice". I'd rather phrase this as
> along the lines:
>
> "Migrate to the newer kmap_local_page() interface from kmap_atomic()
> in order to move away from per-CPU maps to pre-task_struct maps.
> This in effect removes the need to disable preemption in the
> local CPU while kmap is active, and thus vastly reduces overall
> system latency."
>
> Can be improved or written completely otherwise. I just wrote it
> in the way that I had understood the whole deal in the first place.
>
> BR, Jarkko

Thanks for looking into this Jarkko - I will update the commit log for
the next version.

Kristen