From patchwork Thu Feb 20 13:28:26 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "lan,Tianyu" X-Patchwork-Id: 3687011 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 7E8FCBF13A for ; Thu, 20 Feb 2014 13:28:31 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A608620173 for ; Thu, 20 Feb 2014 13:28:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A2D5820158 for ; Thu, 20 Feb 2014 13:28:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754193AbaBTN22 (ORCPT ); Thu, 20 Feb 2014 08:28:28 -0500 Received: from mga01.intel.com ([192.55.52.88]:64173 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753437AbaBTN21 (ORCPT ); Thu, 20 Feb 2014 08:28:27 -0500 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 20 Feb 2014 05:28:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,512,1389772800"; d="scan'208";a="484835904" Received: from unknown (HELO tlan1-mobl2.ccr.corp.intel.com) ([10.255.21.198]) by fmsmga002.fm.intel.com with ESMTP; 20 Feb 2014 05:28:23 -0800 Message-ID: <530602FA.1020103@intel.com> Date: Thu, 20 Feb 2014 21:28:26 +0800 From: Lan Tianyu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130612 Thunderbird/17.0.6 MIME-Version: 1.0 To: Tejun Heo , Oleg Nesterov , Jiri Olsa CC: Zhang Rui , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: Re: WARNING at kernel/workqueue.c:829 wq_worker_waking_up+0x53/0x70() References: <20140213124059.GA2908@krava.brq.redhat.com> <20140217171900.GB29173@redhat.com> <20140218224907.GG31892@mtj.dyndns.org> In-Reply-To: <20140218224907.GG31892@mtj.dyndns.org> Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 02/19/2014 06:49 AM, Tejun Heo wrote: > Hello, > > On Mon, Feb 17, 2014 at 06:19:00PM +0100, Oleg Nesterov wrote: >> acpi_processor_set_throttling() plays with set_cpus_allowed_ptr(current), >> this is obviously wrong, and the worker is bound. > > Umm... yeah, anything running on workqueues shouldn't be diddling with > cpu affinity. The function even has /* FIXME: use work_on_cpu() */ in > it. I suppose it's about time to actually implement that? > > Thanks. > Hi Jiri: Could you try this patch which reworks ACPI processor throttling with work_on_cpu()? { @@ -1068,7 +1085,8 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, unsigned int i; struct acpi_processor *match_pr; struct acpi_processor_throttling *p_throttling; - struct throttling_tstate t_state; + struct acpi_processor_throttling_arg arg; + struct throttling_tstate t_state; cpumask_var_t online_throttling_cpus; if (!pr) @@ -1083,10 +1101,8 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, if (!alloc_cpumask_var(&saved_mask, GFP_KERNEL)) return -ENOMEM; - if (!alloc_cpumask_var(&online_throttling_cpus, GFP_KERNEL)) { - free_cpumask_var(saved_mask); + if (!alloc_cpumask_var(&online_throttling_cpus, GFP_KERNEL)) return -ENOMEM; - } if (cpu_is_offline(pr->id)) { /* @@ -1096,7 +1112,6 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, return -ENODEV; } - cpumask_copy(saved_mask, ¤t->cpus_allowed); t_state.target_state = state; p_throttling = &(pr->throttling); cpumask_and(online_throttling_cpus, cpu_online_mask, @@ -1118,14 +1133,10 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, * it can be called only for the cpu pointed by pr. */ if (p_throttling->shared_type == DOMAIN_COORD_TYPE_SW_ANY) { - /* FIXME: use work_on_cpu() */ - if (set_cpus_allowed_ptr(current, cpumask_of(pr->id))) { - /* Can't migrate to the pr->id CPU. Exit */ - ret = -ENODEV; - goto exit; - } - ret = p_throttling->acpi_processor_set_throttling(pr, - t_state.target_state, force); + arg.pr = pr; + arg.target_state = state; + arg.force = force; + ret = work_on_cpu(pr->id, acpi_processor_throttling_fn, &arg); } else { /* * When the T-state coordination is SW_ALL or HW_ALL, @@ -1153,13 +1164,11 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, "on CPU %d\n", i)); continue; } - t_state.cpu = i; - /* FIXME: use work_on_cpu() */ - if (set_cpus_allowed_ptr(current, cpumask_of(i))) - continue; - ret = match_pr->throttling. - acpi_processor_set_throttling( - match_pr, t_state.target_state, force); + + arg.pr = match_pr; + arg.target_state = state; + arg.force = force; + ret = work_on_cpu(pr->id, acpi_processor_throttling_fn, &arg); } } /* @@ -1173,12 +1182,8 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, acpi_processor_throttling_notifier(THROTTLING_POSTCHANGE, &t_state); } - /* restore the previous state */ - /* FIXME: use work_on_cpu() */ - set_cpus_allowed_ptr(current, saved_mask); -exit: + free_cpumask_var(online_throttling_cpus); - free_cpumask_var(saved_mask); return ret; } diff --git a/drivers/acpi/processor_throttling.c b/drivers/acpi/processor_throttling.c index 28baa05..9122d64 100644 --- a/drivers/acpi/processor_throttling.c +++ b/drivers/acpi/processor_throttling.c @@ -1060,6 +1060,23 @@ static int acpi_processor_set_throttling_ptc(struct acpi_processor *pr, return 0; } +struct acpi_processor_throttling_arg { + struct acpi_processor *pr; + int target_state; + bool force; +}; + +static long acpi_processor_throttling_fn(void *data) +{ + struct acpi_processor_throttling_arg *arg = data; + struct acpi_processor *pr = arg->pr; + struct acpi_processor_throttling *p_throttling = &pr->throttling; + + return p_throttling->acpi_processor_set_throttling(pr, + arg->target_state, arg->force); +} + + int acpi_processor_set_throttling(struct acpi_processor *pr, int state, bool force) { @@ -1068,7 +1085,8 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, unsigned int i; struct acpi_processor *match_pr; struct acpi_processor_throttling *p_throttling; - struct throttling_tstate t_state; + struct acpi_processor_throttling_arg arg; + struct throttling_tstate t_state; cpumask_var_t online_throttling_cpus; if (!pr) @@ -1083,10 +1101,8 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, if (!alloc_cpumask_var(&saved_mask, GFP_KERNEL)) return -ENOMEM; - if (!alloc_cpumask_var(&online_throttling_cpus, GFP_KERNEL)) { - free_cpumask_var(saved_mask); + if (!alloc_cpumask_var(&online_throttling_cpus, GFP_KERNEL)) return -ENOMEM; - } if (cpu_is_offline(pr->id)) { /* @@ -1096,7 +1112,6 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, return -ENODEV; } - cpumask_copy(saved_mask, ¤t->cpus_allowed); t_state.target_state = state; p_throttling = &(pr->throttling); cpumask_and(online_throttling_cpus, cpu_online_mask, @@ -1118,14 +1133,10 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, * it can be called only for the cpu pointed by pr. */ if (p_throttling->shared_type == DOMAIN_COORD_TYPE_SW_ANY) { - /* FIXME: use work_on_cpu() */ - if (set_cpus_allowed_ptr(current, cpumask_of(pr->id))) { - /* Can't migrate to the pr->id CPU. Exit */ - ret = -ENODEV; - goto exit; - } - ret = p_throttling->acpi_processor_set_throttling(pr, - t_state.target_state, force); + arg.pr = pr; + arg.target_state = state; + arg.force = force; + ret = work_on_cpu(pr->id, acpi_processor_throttling_fn, &arg); } else { /* * When the T-state coordination is SW_ALL or HW_ALL, @@ -1153,13 +1164,11 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, "on CPU %d\n", i)); continue; } - t_state.cpu = i; - /* FIXME: use work_on_cpu() */ - if (set_cpus_allowed_ptr(current, cpumask_of(i))) - continue; - ret = match_pr->throttling. - acpi_processor_set_throttling( - match_pr, t_state.target_state, force); + + arg.pr = match_pr; + arg.target_state = state; + arg.force = force; + ret = work_on_cpu(pr->id, acpi_processor_throttling_fn, &arg); } } /* @@ -1173,12 +1182,8 @@ int acpi_processor_set_throttling(struct acpi_processor *pr, acpi_processor_throttling_notifier(THROTTLING_POSTCHANGE, &t_state); } - /* restore the previous state */ - /* FIXME: use work_on_cpu() */ - set_cpus_allowed_ptr(current, saved_mask); -exit: + free_cpumask_var(online_throttling_cpus); - free_cpumask_var(saved_mask); return ret; }