Re: [RFC PATCH 2/4] softirq: Per vector deferment to workqueue

From: Frederic Weisbecker <frederic@kernel.org>
To: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Levin Alexander <alexander.levin@verizon.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mauro Carvalho Chehab <mchehab@s-opensource.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Wanpeng Li <wanpeng.li@hotmail.com>,
	Dmitry Safonov <dima@arista.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Paolo Abeni <pabeni@redhat.com>, Radu Rendec <rrendec@arista.com>,
	Ingo Molnar <mingo@kernel.org>,
	Stanislaw Gruszka <sgruszka@redhat.com>,
	Rik van Riel <riel@redhat.com>,
	Eric Dumazet <edumazet@google.com>,
	David Miller <davem@davemloft.net>
Subject: Re: [RFC PATCH 2/4] softirq: Per vector deferment to workqueue
Date: Sun, 21 Jan 2018 21:48:14 +0100	[thread overview]
Message-ID: <20180121204812.GC2879@lerouge> (raw)
In-Reply-To: <20180121175040.GA30677@codeaurora.org>

On Sun, Jan 21, 2018 at 11:20:40PM +0530, Pavan Kondeti wrote:
> On Sun, Jan 21, 2018 at 05:11:17PM +0100, Frederic Weisbecker wrote:
> > On Sat, Jan 20, 2018 at 02:11:39PM +0530, Pavan Kondeti wrote:
> > 
> > Hi Pavan,
> > 
> > 
> > > I have couple questions/comments.
> > > 
> > > (1) Since the work is queued on a bounded per-cpu worker, we may run
> > > into a deadlock if a TASKLET is killed from another work running on
> > > the same bounded per-cpu worker.
> > > 
> > > For example,
> > > 
> > > (1) Schedule a TASKLET on CPU#0 from IRQ.
> > > (2) Another IRQ comes on the same CPU and we queue a work to kill
> > > the TASKLET. 
> > > (3) The TASKLET vector is deferred to workqueue.
> > > (4) We run the TASKLET kill work and wait for the TASKLET to finish,
> > > which won't happen.
> > > 
> > > We can fix this by queueing the TASKLET kill work on an unbounded
> > > workqueue so that this runs in parallel with TASKLET vector work.
> > > 
> > > Just wanted to know if we have to be aware of this *condition*.
> > 
> > But IIRC the workqueues have several workers per CPU so the tasklet to
> > be killed can run while the tasklet killer yields.
> > 
> AFAIK, the work items queued via schedule_work_on() goes to the system_wq
> which is bounded to a CPU with concurrency restrictions. If any work
> item (in this case tasklet kill) is getting executed on this bounded
> worker, the next items have to wait. The forward progress happens only
> when the current work is finished or enters sleep.

Workqueues have multiple workers to handle the pending works, unless they
are __WQ_ORDERED.

The worker manager in a pool is supposed to create workers on-demand
when necessary to make sure that no work is blocking the others.

Thanks.

> 
> This also makes me wonder what happens if a CPU hogging work gets executed
> on the system_wq while softirq work is pending? The softirq work gets
> starved which won't happen now with ksoftirqd design. Ideally the CPU
> hogging work should not be queued on the system_wq and instead should
> be queued on CPU intenstive workqueue (WQ_CPU_INTENSIVE) to exempt
> from concurrency management. May be we need some special workqueue
> which is bounded but not subjected to concurrency management.
> 
> -- 
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
> 