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: > > > text_poke() _does_ create a separate RW mapping. > > > > Sorry, I meant a separate RW allocation. > > Ah yes, that makes sense > > > > > > > > > > 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? > > > 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.
