Re: [PATCH 00/41] Simplify, reorganize and clean up the x86 INT3 based batch-patching code (alternative.c)

[Ingo Molnar]

* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Thu, 27 Mar 2025 at 13:54, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > The second part of the series simplifies and standardizes the SMP batch-patching
> > data & types namespace, around the new tp_array* namespace:
> >
> >         int3_patching_desc      => [removed]
> >         temp_mm_state_t         => [removed]
> >         try_get_desc()          => [removed]
> >         put_desc()              => [removed]
> >
> >         tp_vec,tp_vec_nr        => tp_array
> >         int3_refs               => tp_array_refs
> 
> Honestly, I think "int3" is better than "tp" as a part of the name.

Yeah, was thinking hard about those two as well, but skipped it for 
this series, because of the brevity issue: text_poke_int3_ is quite a 
mouthful for a commonly used global state variable.

> "tp" doesn't say _anything_ to me, even though I understand it is 
> short for "text poke". But if you want to say 'text_poke", please 
> just write it out.
> 
> At least "int3" has some meaning in x86 context, unlike "tp".
> 
> So please either write out "text_poke" and accept that the names are 
> a bit longer (but a lot more descriptive), or use "int3" if you want 
> to save some typing.

Yeah.

So the thing is, the whole _int3 naming itself is a bit artificial 
IMHO, what we *really* want to signal here is whether something is 
boot/UP functionality or SMP functionality under the text_mutex.

That the SMP functionality relies on INT3 traps is an implementational 
detail that isn't necessary to show up in the API namespace. It also 
relies on CR3 flushing, so we could as well have added _cr3. ;-)

So I was thinking about something like this for the boot/UP variants:

  text_poke_*()

and for the SMP variants:

  smp_text_poke_*()

and text_poke_* for the data/type space.

Plus I think with the adding of 'smp_' we can also add 'batch_' to a 
few APIs to make the family of APIs clearer, plus a few other things:

A quick summary of changes (mockup):

	# boot/UP APIs & single-thread helpers:

						text_poke()
						text_poke_kgdb()
	[ unchanged APIs: ]			text_poke_copy()
						text_poke_copy_locked()
						text_poke_set()

						text_poke_addr()

	# SMP API & helpers namespace:

	text_poke_bp()			=>	smp_text_poke_single()
	text_poke_loc_init()		=>	__smp_text_poke_batch_add()
	text_poke_queue()		=>	smp_text_poke_batch_add()
	text_poke_finish()		=>	smp_text_poke_batch_flush()
	text_poke_flush()		=>	smp_text_poke_batch_finish()

	text_poke_bp_batch()		=>	smp_text_poke_batch_process()
	poke_int3_handler()		=>	smp_text_poke_int3_trap_handler()
        text_poke_sync()		=>	smp_text_poke_sync_each_cpu()


	# data/type namespace:

	int3_patching_desc		=>	[removed]
	temp_mm_state_t			=>	[removed]
	try_get_desc()			=>	[removed]
	put_desc()			=>	[removed]

	tp_vec,tp_vec_nr		=>	text_poke_array
	int3_refs			=>	text_poke_array_refs

Some of the API names are now a bit long, but I think this is one of 
the cases where clarity is more important than brevity, plus these are 
usually used in a pretty compact, straightforward fashion to trigger 
text-patching processing, not part of complex compound expressions.

I'll propagate this nomenclature into the series and repost.

> PS. The casual meaning "tp" has in English everyday language is short 
> for "toilet paper".

LOL, this seals the deal, the tp_ prefix is *so* dead.

Thanks,

	Ingo