2022-11-15 21:17:18

by Guilherme G. Piccoli

[permalink] [raw]
Subject: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

The function match_records() may take a while due to a large
number of string comparisons, so when in PREEMPT_VOLUNTARY
kernels we could face RCU stalls due to that.

Add a cond_resched() to prevent that.

Suggested-by: Steven Rostedt <[email protected]>
Acked-by: Paul E. McKenney <[email protected]> # from RCU CPU stall warning perspective
Cc: Masami Hiramatsu <[email protected]>
Cc: Mark Rutland <[email protected]>
Signed-off-by: Guilherme G. Piccoli <[email protected]>
---

Hi Steve / Paul, thanks for the discussions on the first thread [0],
much appreciated! Here is the "official" version.

Steve: lemme know if it's good for you, and in case you prefer to
send it yourself (since you proposed it on IRC), fine by me!

Paul: kept your ACK (thanks for that BTW) even though I changed the
place of cond_resched() to align with Steve's preference. Lemme know
in case you want to drop this ACK.

Cheers,

Guilherme


[0] https://lore.kernel.org/lkml/[email protected]/


kernel/trace/ftrace.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7dc023641bf1..80639bdb85f6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4192,6 +4192,7 @@ match_records(struct ftrace_hash *hash, char *func, int len, char *mod)
}
found = 1;
}
+ cond_resched();
} while_for_each_ftrace_rec();
out_unlock:
mutex_unlock(&ftrace_lock);
--
2.38.0



2022-11-16 07:14:32

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On Tue, 15 Nov 2022 17:48:47 -0300
"Guilherme G. Piccoli" <[email protected]> wrote:

> The function match_records() may take a while due to a large
> number of string comparisons, so when in PREEMPT_VOLUNTARY
> kernels we could face RCU stalls due to that.
>
> Add a cond_resched() to prevent that.
>
> Suggested-by: Steven Rostedt <[email protected]>
> Acked-by: Paul E. McKenney <[email protected]> # from RCU CPU stall warning perspective
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Guilherme G. Piccoli <[email protected]>

This looks good to me.

Acked-by: Masami Hiramatsu (Google) <[email protected]>

Thanks!

> ---
>
> Hi Steve / Paul, thanks for the discussions on the first thread [0],
> much appreciated! Here is the "official" version.
>
> Steve: lemme know if it's good for you, and in case you prefer to
> send it yourself (since you proposed it on IRC), fine by me!
>
> Paul: kept your ACK (thanks for that BTW) even though I changed the
> place of cond_resched() to align with Steve's preference. Lemme know
> in case you want to drop this ACK.
>
> Cheers,
>
> Guilherme
>
>
> [0] https://lore.kernel.org/lkml/[email protected]/
>
>
> kernel/trace/ftrace.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 7dc023641bf1..80639bdb85f6 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4192,6 +4192,7 @@ match_records(struct ftrace_hash *hash, char *func, int len, char *mod)
> }
> found = 1;
> }
> + cond_resched();
> } while_for_each_ftrace_rec();
> out_unlock:
> mutex_unlock(&ftrace_lock);
> --
> 2.38.0
>


--
Masami Hiramatsu (Google) <[email protected]>

2022-11-16 13:21:06

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On 16/11/2022 03:36, Masami Hiramatsu (Google) wrote:
> [...]
> This looks good to me.
>
> Acked-by: Masami Hiramatsu (Google) <[email protected]>
>
> Thanks!

Thanks for your review/Ack Masami, much appreciated.
Cheers,


Guilherme

2022-11-22 13:41:26

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On 16/11/2022 03:36, Masami Hiramatsu (Google) wrote:
> On Tue, 15 Nov 2022 17:48:47 -0300
> "Guilherme G. Piccoli" <[email protected]> wrote:
>
>> The function match_records() may take a while due to a large
>> number of string comparisons, so when in PREEMPT_VOLUNTARY
>> kernels we could face RCU stalls due to that.
>>
>> Add a cond_resched() to prevent that.
>>
>> Suggested-by: Steven Rostedt <[email protected]>
>> Acked-by: Paul E. McKenney <[email protected]> # from RCU CPU stall warning perspective
>> Cc: Masami Hiramatsu <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Signed-off-by: Guilherme G. Piccoli <[email protected]>
>
> This looks good to me.
>
> Acked-by: Masami Hiramatsu (Google) <[email protected]>
>
> Thanks!
>

Hi Steve, apologies for the ping, just want to ask you if you plan to
take this one or should I change something? Thanks in advance,


Guilherme

2022-11-22 16:24:51

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On Tue, 22 Nov 2022 10:16:41 -0300
"Guilherme G. Piccoli" <[email protected]> wrote:

> Hi Steve, apologies for the ping, just want to ask you if you plan to
> take this one or should I change something? Thanks in advance,

I was traveling for work from Oct 31-Nov 14 and when I got back, I had over
300 patches to review in my queue. I'm now down under 100, so hopefully I
can get to it soon.

Just may take a bit.

Thanks,

-- Steve

2022-11-22 18:13:07

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On 22/11/2022 12:49, Steven Rostedt wrote:
> [...]
> I was traveling for work from Oct 31-Nov 14 and when I got back, I had over
> 300 patches to review in my queue. I'm now down under 100, so hopefully I
> can get to it soon.
>
> Just may take a bit.
>
> Thanks,
>
> -- Steve

No hurries, thanks for your heads-up!
Cheers,


Guilherme

2022-12-13 22:15:51

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels



On 16/11/2022 03:36, Masami Hiramatsu (Google) wrote:
> On Tue, 15 Nov 2022 17:48:47 -0300
> "Guilherme G. Piccoli" <[email protected]> wrote:
>
>> The function match_records() may take a while due to a large
>> number of string comparisons, so when in PREEMPT_VOLUNTARY
>> kernels we could face RCU stalls due to that.
>>
>> Add a cond_resched() to prevent that.
>>
>> Suggested-by: Steven Rostedt <[email protected]>
>> Acked-by: Paul E. McKenney <[email protected]> # from RCU CPU stall warning perspective
>> Cc: Masami Hiramatsu <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Signed-off-by: Guilherme G. Piccoli <[email protected]>
>
> This looks good to me.
>
> Acked-by: Masami Hiramatsu (Google) <[email protected]>
>
> Thanks!
>

Hi Steve, sorry to annoy, just a regular ping to see if we can get this
into 6.2 (if too late, not a huge deal).

Thanks,


Guilherme

2022-12-13 22:44:48

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On Tue, 13 Dec 2022 19:04:41 -0300
"Guilherme G. Piccoli" <[email protected]> wrote:

> Hi Steve, sorry to annoy, just a regular ping to see if we can get this
> into 6.2 (if too late, not a huge deal).

Thanks for the ping. For some reason this patch didn't make it into my
internal Patchwork, so I missed it (even though I did reply to you, I just
assumed it was in my patchwork).

It's not too late. I have some last minute patches I'm still testing. I'll
queue this one up.

BTW, we now have a kernel ftrace mailing list that you can Cc (but still Cc
LKML).

[email protected]

-- Steve

2022-12-13 23:46:09

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [PATCH] ftrace: Prevent RCU stall on PREEMPT_VOLUNTARY kernels

On 13/12/2022 19:34, Steven Rostedt wrote:
> On Tue, 13 Dec 2022 19:04:41 -0300
> "Guilherme G. Piccoli" <[email protected]> wrote:
>
>> Hi Steve, sorry to annoy, just a regular ping to see if we can get this
>> into 6.2 (if too late, not a huge deal).
>
> Thanks for the ping. For some reason this patch didn't make it into my
> internal Patchwork, so I missed it (even though I did reply to you, I just
> assumed it was in my patchwork).
>
> It's not too late. I have some last minute patches I'm still testing. I'll
> queue this one up.
>
> BTW, we now have a kernel ftrace mailing list that you can Cc (but still Cc
> LKML).
>
> [email protected]
>
> -- Steve

Tnx a lot Steve! I'll start to CC this list as well.

Cheers,


Guilherme