All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zhang, Qiang1" <qiang1.zhang@intel.com>
To: Ammar Faizi <ammarfaizi2@gnuweeb.org>,
	"paulmck@kernel.org" <paulmck@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] rcu: Add per-CPU rcuc task info to RCU CPU stall warnings
Date: Tue, 25 Jan 2022 02:24:47 +0000	[thread overview]
Message-ID: <PH0PR11MB588099DBA57F16345037C348DA5F9@PH0PR11MB5880.namprd11.prod.outlook.com> (raw)
In-Reply-To: <3f08b8cf-a1dc-4d83-0de2-94203dff9a4c@gnuweeb.org>


On 1/24/22 11:42 PM, Paul E. McKenney wrote:
> On Mon, Jan 24, 2022 at 05:38:21PM +0700, Ammar Faizi wrote:
>> [snip...]
>> FWIW, this one makes more sense:
>> ```
>> static void rcuc_kthread_dump(struct rcu_data *rdp) {
>> 	 int cpu;
>> 	 unsigned long j;
>> 	 struct task_struct *rcuc;
>>
>> 	 if (!rcu_is_rcuc_kthread_starving(rdp, &j))
>> 		 return;
>>
>> 	 rcuc = rdp->rcu_cpu_kthread_task;
>> 	 if (!rcuc)
>> 		 return;
>>
>> 	 pr_err("%s kthread starved for %ld jiffies, stack dump:\n", 
>> rcuc->comm, j);
> 
> Thank you for looking this over and for the great feedback, Ammar!
> 
> I am also wondering why the above message should be printed when the 
> corresponding CPU is offline or idle.  Why not move the above pr_err() 
> line down to replace the pr_err() line below?
> 
> 							Thanx, Paul

>>Hi Paul, Thank you for the review. Agree with that.
>>Hopefully this one looks better (untested):
>>```
>>static void rcuc_kthread_dump(struct rcu_data *rdp) {
>>	int cpu;
>>	unsigned long j;
>>	struct task_struct *rcuc;
>>
>>	rcuc = rdp->rcu_cpu_kthread_task;
>>	if (!rcuc)
>>		return;
>>
>>	cpu = task_cpu(rcuc);
>>	if (cpu_is_offline(cpu) || idle_cpu(cpu))
>>		return;
>>
>>	if (!rcu_is_rcuc_kthread_starving(rdp, &j))
>>		return;
>>
>>	pr_err("%s kthread starved for %ld jiffies\n", rcuc->comm, j);
>>	sched_show_task(rcuc);
>>	if (!trigger_single_cpu_backtrace(cpu))
>>		dump_cpu_task(cpu);
>>}
>>```

>>Recall that dump_cpu_task looks like this:
>>```
>>void dump_cpu_task(int cpu)
>>{
>>	pr_info("Task dump for CPU %d:\n", cpu);
>>	sched_show_task(cpu_curr(cpu));
>>}
>>```
>>which already tells us it's a dump, so "stack dump" in the pr_err() can be omitted. Any comment, Zqiang?

Thanks Ammar, this look like more compact, I wiil resend.

Thanks
Zqiang

>>>>
>>--
>>Ammar Faizi

      reply	other threads:[~2022-01-25  3:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-24 10:36 [PATCH v2] rcu: Add per-CPU rcuc task info to RCU CPU stall warnings Zqiang
2022-01-24 10:03 ` Ammar Faizi
2022-01-24 10:38 ` Ammar Faizi
2022-01-24 16:42   ` Paul E. McKenney
2022-01-24 22:08     ` Ammar Faizi
2022-01-25  2:24       ` Zhang, Qiang1 [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR11MB588099DBA57F16345037C348DA5F9@PH0PR11MB5880.namprd11.prod.outlook.com \
    --to=qiang1.zhang@intel.com \
    --cc=ammarfaizi2@gnuweeb.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.