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

From: Pavan Kondeti <pkondeti@codeaurora.org>
To: Frederic Weisbecker <frederic@kernel.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 23:20:40 +0530	[thread overview]
Message-ID: <20180121175040.GA30677@codeaurora.org> (raw)
In-Reply-To: <20180121161114.GA2879@lerouge>

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.

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.