Skip to content

Commit 24bc5ea

Browse files
committed
seqlock, procfs: Match scoped_seqlock_read() critical section vs. RCU ordering in do_task_stat() to do_io_accounting()
There's two patterns of taking the RCU read-lock and the sig->stats_lock read-seqlock in do_task_stat() and do_io_accounting(), with a different ordering: # do_io_accounting(): guard(rcu)(); scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) { # do_task_stat(): scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) { ... rcu_read_lock(); The ordering is RCU-read+seqlock_read in the first case, seqlock_read+RCU-read in the second case. While technically these read locks can be taken in any order, nevertheless it's good practice to use the more intrusive lock on the inside (which is the IRQs-off section in this case), and reduces head-scratching during review when done consistently, so let's use the do_io_accounting() pattern in do_task_stat(). This will also reduce irqs-off latencies in do_task_stat() a tiny bit. Signed-off-by: Ingo Molnar <mingo@kernel.org> Acked-by: Oleg Nesterov <oleg@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Christian Brauner <brauner@kernel.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Link: https://patch.msgid.link/aS6rwnaPbHFCdHp1@gmail.com
1 parent 4a26e70 commit 24bc5ea

1 file changed

Lines changed: 20 additions & 20 deletions

File tree

fs/proc/array.c

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -537,27 +537,27 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
537537
if (permitted && (!whole || num_threads < 2))
538538
wchan = !task_is_running(task);
539539

540-
scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) {
541-
cmin_flt = sig->cmin_flt;
542-
cmaj_flt = sig->cmaj_flt;
543-
cutime = sig->cutime;
544-
cstime = sig->cstime;
545-
cgtime = sig->cgtime;
546-
547-
if (whole) {
548-
struct task_struct *t;
549-
550-
min_flt = sig->min_flt;
551-
maj_flt = sig->maj_flt;
552-
gtime = sig->gtime;
553-
554-
rcu_read_lock();
555-
__for_each_thread(sig, t) {
556-
min_flt += t->min_flt;
557-
maj_flt += t->maj_flt;
558-
gtime += task_gtime(t);
540+
scoped_guard(rcu) {
541+
scoped_seqlock_read (&sig->stats_lock, ss_lock_irqsave) {
542+
cmin_flt = sig->cmin_flt;
543+
cmaj_flt = sig->cmaj_flt;
544+
cutime = sig->cutime;
545+
cstime = sig->cstime;
546+
cgtime = sig->cgtime;
547+
548+
if (whole) {
549+
struct task_struct *t;
550+
551+
min_flt = sig->min_flt;
552+
maj_flt = sig->maj_flt;
553+
gtime = sig->gtime;
554+
555+
__for_each_thread(sig, t) {
556+
min_flt += t->min_flt;
557+
maj_flt += t->maj_flt;
558+
gtime += task_gtime(t);
559+
}
559560
}
560-
rcu_read_unlock();
561561
}
562562
}
563563

0 commit comments

Comments
 (0)