mbox series

[0/2] uprobes: kill xol_area->slot_count

Message ID 20241001142416.GA13599@redhat.com (mailing list archive)
Headers show
Series uprobes: kill xol_area->slot_count | expand

Message

Oleg Nesterov Oct. 1, 2024, 2:24 p.m. UTC
Hello,

On top of

	[PATCH 0/7] uprobes: deuglify xol_get_insn_slot/xol_free_insn_slot paths
	https://lore.kernel.org/all/20240929144201.GA9429@redhat.com/

To me this change looks like a simplification, but perhaps it can
also be considered as optimization.

Yes, in the contended case xol_get_slot_nr() will be called twice,
the second time after prepare_to_wait_event(), but I don't think it
can hurt in the slow path when the caller is likely going to sleep.

Oleg.
---

 kernel/events/uprobes.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

Comments

Andrii Nakryiko Oct. 1, 2024, 4:59 p.m. UTC | #1
On Tue, Oct 1, 2024 at 7:24 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> Hello,
>
> On top of
>
>         [PATCH 0/7] uprobes: deuglify xol_get_insn_slot/xol_free_insn_slot paths
>         https://lore.kernel.org/all/20240929144201.GA9429@redhat.com/
>
> To me this change looks like a simplification, but perhaps it can
> also be considered as optimization.
>
> Yes, in the contended case xol_get_slot_nr() will be called twice,
> the second time after prepare_to_wait_event(), but I don't think it
> can hurt in the slow path when the caller is likely going to sleep.
>
> Oleg.
> ---
>
>  kernel/events/uprobes.c | 38 +++++++++++++++++---------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)
>

LGTM, for the series:

Acked-by: Andrii Nakryiko <andrii@kernel.org>


But I've also been wondering if the kernel already has some sort of
slot allocator that we can utilize here, instead of reimplementing all
this? I'll cc Vlastimil in case he has some good pointers (or can
point to someone else who might know).