From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neeraj Upadhyay Subject: Re: [PATCH] workqueue: Handle race between wake up and rebind Date: Wed, 17 Jan 2018 01:38:06 +0530 Message-ID: References: <1516005492-4994-1-git-send-email-neeraju@codeaurora.org> <20180116173534.GR3460072@devbig577.frc2.facebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:36682 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751070AbeAPUIL (ORCPT ); Tue, 16 Jan 2018 15:08:11 -0500 In-Reply-To: <20180116173534.GR3460072@devbig577.frc2.facebook.com> Content-Language: en-US Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Tejun Heo Cc: jiangshanlai@gmail.com, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, prsood@codeaurora.org, sramana@codeaurora.org On 01/16/2018 11:05 PM, Tejun Heo wrote: > Hello, Neeraj. > > On Mon, Jan 15, 2018 at 02:08:12PM +0530, Neeraj Upadhyay wrote: >> - kworker/0:0 gets chance to run on cpu1; while processing >> a work, it goes to sleep. However, it does not decrement >> pool->nr_running. This is because WORKER_REBOUND (NOT_ >> RUNNING) flag was cleared, when worker entered worker_ > Do you mean that because REBOUND was set? Actually, I meant REBOUND was not set. Below is the sequence - cpu0 bounded pool is unbound. - kworker/0:0 is woken up on cpu1. - cpu0 pool is rebound REBOUND is set for kworker/0:0 - kworker/0:0 starts running on cpu1 worker_thread() // It clears REBOUND and sets nr_running =1 after below call worker_clr_flags(worker, WORKER_PREP | WORKER_REBOUND); - kworker/0:0 goes to sleep wq_worker_sleeping() // Below condition is not true, as all NOT_RUNNING // flags were cleared in worker_thread() if (worker->flags & WORKER_NOT_RUNNING) // Below is true, as worker is running on cpu1 if (WARN_ON_ONCE(pool->cpu != raw_smp_processor_id())) return NULL; // Below is not reached and nr_running stays 1 if (atomic_dec_and_test(&pool->nr_running) && - kworker/0:0 wakes up again, this time on cpu0, as worker->task cpus_allowed was set to cpu0, in rebind_workers. wq_worker_waking_up() if (!(worker->flags & WORKER_NOT_RUNNING)) { // Increments pool->nr_running to 2 atomic_inc(&worker->pool->nr_running); > >> thread(). >> >> Worker 0 runs on cpu1 >> worker_thread() >> process_one_work() >> wq_worker_sleeping() >> if (worker->flags & WORKER_NOT_RUNNING) >> return NULL; >> if (WARN_ON_ONCE(pool->cpu != raw_smp_processor_id())) >> >> >> - After this, when kworker/0:0 wakes up, this time on its >> bounded cpu cpu0, it increments pool->nr_running again. >> So, pool->nr_running becomes 2. > Why is it suddenly 2? Who made it one on the account of the kworker? As shown in above comment, it became 1 in worker_clr_flags(worker, WORKER_PREP | WORKER_REBOUND); > > Do you see this happening? Or better, is there a (semi) reliable > repro for this issue? Yes, this was reported in our long run testing with random hotplug. Sorry, don't have a quick reproducer for it. Issue is reported in few days of testing. > > Thanks. > -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation