2021-08-18 07:30:13

by Neeraj Upadhyay

[permalink] [raw]
Subject: [PATCH 0/5] rcu-tasks miscellaneous fixes

Minor typos in comments and fixes for rcu-tasks.

Neeraj Upadhyay (5):
rcu-tasks: Fix s/rcu_add_holdout/trc_add_holdout/ typo in comment
rcu-tasks: Correct firstreport usage in check_all_holdout_tasks_trace
rcu-tasks: Correct check for no_hz_full cpu in show_stalled_task_trace
rcu-tasks: Fix read-side primitives comment for call_rcu_tasks_trace
rcu-tasks: Clarify read side section info for rcu_tasks_rude GP
primitives

kernel/rcu/tasks.h | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


2021-08-18 07:30:46

by Neeraj Upadhyay

[permalink] [raw]
Subject: [PATCH 2/5] rcu-tasks: Correct firstreport usage in check_all_holdout_tasks_trace

In check_all_holdout_tasks_trace(), firstreport is a pointer argument;
so, check the dereferenced value, instead of checking the pointer.

Signed-off-by: Neeraj Upadhyay <[email protected]>
---
kernel/rcu/tasks.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index e62da45..9db7293 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1134,7 +1134,7 @@ static void check_all_holdout_tasks_trace(struct list_head *hop,
cpus_read_unlock();

if (needreport) {
- if (firstreport)
+ if (*firstreport)
pr_err("INFO: rcu_tasks_trace detected stalls? (Late IPI?)\n");
show_stalled_ipi_trace();
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2021-08-18 07:31:46

by Neeraj Upadhyay

[permalink] [raw]
Subject: [PATCH 4/5] rcu-tasks: Fix read-side primitives comment for call_rcu_tasks_trace

call_rcu_tasks_trace() does have read-side primitives - rcu_read_lock_trace()
and rcu_read_unlock_trace(). Fix this information in the comments.

Signed-off-by: Neeraj Upadhyay <[email protected]>
---
kernel/rcu/tasks.h | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 244e06a..5f9c14c 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1214,13 +1214,9 @@ static void exit_tasks_rcu_finish_trace(struct task_struct *t)
*
* The callback function will be invoked some time after a full grace
* period elapses, in other words after all currently executing RCU
- * read-side critical sections have completed. call_rcu_tasks_trace()
- * assumes that the read-side critical sections end at context switch,
- * cond_resched_rcu_qs(), or transition to usermode execution. As such,
- * there are no read-side primitives analogous to rcu_read_lock() and
- * rcu_read_unlock() because this primitive is intended to determine
- * that all tasks have passed through a safe state, not so much for
- * data-structure synchronization.
+ * read-side critical sections have completed. These read-side
+ * critical sections are delimited by calls to rcu_read_lock_trace()
+ * and rcu_read_unlock_trace().
*
* See the description of call_rcu() for more detailed information on
* memory ordering guarantees.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2021-08-18 07:31:49

by Neeraj Upadhyay

[permalink] [raw]
Subject: [PATCH 5/5] rcu-tasks: Clarify read side section info for rcu_tasks_rude GP primitives

RCU tasks rude variant does not check whether the current
running context on a CPU is usermode. Read side critical section ends
on transition to usermode execution, by the virtue of usermode
execution being schedulable. Clarify this in comments for
call_rcu_tasks_rude() and synchronize_rcu_tasks_rude().

Signed-off-by: Neeraj Upadhyay <[email protected]>
---
kernel/rcu/tasks.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 5f9c14c..702a4fa 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -677,11 +677,11 @@ DEFINE_RCU_TASKS(rcu_tasks_rude, rcu_tasks_rude_wait_gp, call_rcu_tasks_rude,
* period elapses, in other words after all currently executing RCU
* read-side critical sections have completed. call_rcu_tasks_rude()
* assumes that the read-side critical sections end at context switch,
- * cond_resched_rcu_qs(), or transition to usermode execution. As such,
- * there are no read-side primitives analogous to rcu_read_lock() and
- * rcu_read_unlock() because this primitive is intended to determine
- * that all tasks have passed through a safe state, not so much for
- * data-structure synchronization.
+ * cond_resched_rcu_qs(), or transition to usermode execution (as
+ * usermode execution is schedulable). As such, there are no read-side
+ * primitives analogous to rcu_read_lock() and rcu_read_unlock() because
+ * this primitive is intended to determine that all tasks have passed
+ * through a safe state, not so much for data-structure synchronization.
*
* See the description of call_rcu() for more detailed information on
* memory ordering guarantees.
@@ -699,8 +699,8 @@ EXPORT_SYMBOL_GPL(call_rcu_tasks_rude);
* grace period has elapsed, in other words after all currently
* executing rcu-tasks read-side critical sections have elapsed. These
* read-side critical sections are delimited by calls to schedule(),
- * cond_resched_tasks_rcu_qs(), userspace execution, and (in theory,
- * anyway) cond_resched().
+ * cond_resched_tasks_rcu_qs(), userspace execution (which is a schedulable
+ * context), and (in theory, anyway) cond_resched().
*
* This is a very specialized primitive, intended only for a few uses in
* tracing and other situations requiring manipulation of function preambles
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2021-08-18 07:32:32

by Neeraj Upadhyay

[permalink] [raw]
Subject: [PATCH 1/5] rcu-tasks: Fix s/rcu_add_holdout/trc_add_holdout/ typo in comment

Signed-off-by: Neeraj Upadhyay <[email protected]>
---
kernel/rcu/tasks.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 39cef3c..e62da45 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -996,7 +996,7 @@ static void trc_wait_for_one_reader(struct task_struct *t,

// If this task is not yet on the holdout list, then we are in
// an RCU read-side critical section. Otherwise, the invocation of
- // rcu_add_holdout() that added it to the list did the necessary
+ // trc_add_holdout() that added it to the list did the necessary
// get_task_struct(). Either way, the task cannot be freed out
// from under this code.

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2021-08-18 07:33:10

by Neeraj Upadhyay

[permalink] [raw]
Subject: [PATCH 3/5] rcu-tasks: Correct check for no_hz_full cpu in show_stalled_task_trace

Correct the check for no_hz_full cpu in show_stalled_task_trace(),
to include cpu 0.

Signed-off-by: Neeraj Upadhyay <[email protected]>
---
kernel/rcu/tasks.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 9db7293..244e06a 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1091,7 +1091,7 @@ static void show_stalled_task_trace(struct task_struct *t, bool *firstreport)
t->pid,
".I"[READ_ONCE(t->trc_ipi_to_cpu) > 0],
".i"[is_idle_task(t)],
- ".N"[cpu > 0 && tick_nohz_full_cpu(cpu)],
+ ".N"[cpu >= 0 && tick_nohz_full_cpu(cpu)],
READ_ONCE(t->trc_reader_nesting),
" N"[!!READ_ONCE(t->trc_reader_special.b.need_qs)],
cpu);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2021-08-18 15:48:32

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 4/5] rcu-tasks: Fix read-side primitives comment for call_rcu_tasks_trace

On Wed, Aug 18, 2021 at 12:58:42PM +0530, Neeraj Upadhyay wrote:
> call_rcu_tasks_trace() does have read-side primitives - rcu_read_lock_trace()
> and rcu_read_unlock_trace(). Fix this information in the comments.
>
> Signed-off-by: Neeraj Upadhyay <[email protected]>
> ---
> kernel/rcu/tasks.h | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 244e06a..5f9c14c 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1214,13 +1214,9 @@ static void exit_tasks_rcu_finish_trace(struct task_struct *t)
> *
> * The callback function will be invoked some time after a full grace
> * period elapses, in other words after all currently executing RCU
> - * read-side critical sections have completed. call_rcu_tasks_trace()
> - * assumes that the read-side critical sections end at context switch,
> - * cond_resched_rcu_qs(), or transition to usermode execution. As such,
> - * there are no read-side primitives analogous to rcu_read_lock() and
> - * rcu_read_unlock() because this primitive is intended to determine
> - * that all tasks have passed through a safe state, not so much for
> - * data-structure synchronization.
> + * read-side critical sections have completed. These read-side

The above line should start "Tasks Trace read-side critical..." for
the benefit of people who stop reading before the next sentence. :-/

Thanx, Paul

> + * critical sections are delimited by calls to rcu_read_lock_trace()
> + * and rcu_read_unlock_trace().
> *
> * See the description of call_rcu() for more detailed information on
> * memory ordering guarantees.
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>

2021-08-18 15:54:46

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 0/5] rcu-tasks miscellaneous fixes

On Wed, Aug 18, 2021 at 12:58:38PM +0530, Neeraj Upadhyay wrote:
> Minor typos in comments and fixes for rcu-tasks.

I queued all but 4/5 for review and testing, thank you!!!

4/5 needs a slight fix that I cannot do in the patch itself.

Thanx, Paul

> Neeraj Upadhyay (5):
> rcu-tasks: Fix s/rcu_add_holdout/trc_add_holdout/ typo in comment
> rcu-tasks: Correct firstreport usage in check_all_holdout_tasks_trace
> rcu-tasks: Correct check for no_hz_full cpu in show_stalled_task_trace
> rcu-tasks: Fix read-side primitives comment for call_rcu_tasks_trace
> rcu-tasks: Clarify read side section info for rcu_tasks_rude GP
> primitives
>
> kernel/rcu/tasks.h | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>