On Thu, 2023-06-01 at 14:38 -0400, Kent Overstreet wrote:

On Thu, Jun 01, 2023 at 06:13:44PM +0000, Edgecombe, Rick P wrote:

quoted

quoted

text_poke() _does_ create a separate RW mapping.

Sorry, I meant a separate RW allocation.

Ah yes, that makes sense

quoted

quoted

The thing that sucks about text_poke() is that it always does a
full
TLB
flush, and AFAICT that's not remotely needed. What it really
wants to
be
doing is conceptually just

kmap_local()
mempcy()
kunmap_loca()
flush_icache();

...except that kmap_local() won't actually create a new mapping
on
non-highmem architectures, so text_poke() open codes it.

Text poke creates only a local CPU RW mapping. It's more secure
because
other threads can't write to it.

*nod*, same as kmap_local

It's only used and flushed locally, but it is accessible to all CPU's,
right?

quoted

It also only needs to flush the local core when it's done since
it's
not using a shared MM.

 
Ahh! Thanks for that; perhaps the comment in text_poke() about IPIs
could be a bit clearer.

What is it (if anything) you don't like about text_poke() then? It
looks
like it's doing broadly similar things to kmap_local(), so should be
in the same ballpark from a performance POV?

The way text_poke() is used here, it is creating a new writable alias
and flushing it for *each* write to the module (like for each write of
an individual relocation, etc). I was just thinking it might warrant
some batching or something.