Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932636AbbERUeg (ORCPT ); Mon, 18 May 2015 16:34:36 -0400 Received: from mail-qg0-f53.google.com ([209.85.192.53]:35580 "EHLO mail-qg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932404AbbERUef (ORCPT ); Mon, 18 May 2015 16:34:35 -0400 Date: Mon, 18 May 2015 16:34:31 -0400 From: Tejun Heo To: Lai Jiangshan Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/7 V2] workqueue: reuse the current per-node pwq when its attrs unchanged Message-ID: <20150518203431.GE24861@htj.duckdns.org> References: <1431433955-3173-1-git-send-email-laijs@cn.fujitsu.com> <1431433955-3173-5-git-send-email-laijs@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431433955-3173-5-git-send-email-laijs@cn.fujitsu.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2270 Lines: 63 On Tue, May 12, 2015 at 08:32:32PM +0800, Lai Jiangshan wrote: > If the cpuamsk is changed, it is possible that only a part of the per-node cpumask > pwq is affected. This can happen when the user changes the cpumask of > a workqueue or the low level cpumask. > > So we try to reuse the current per-node pwq when its attrs unchanged. are unchanged. > @@ -3592,9 +3593,14 @@ apply_wqattrs_prepare(struct workqueue_struct *wq, > > for_each_node(node) { > if (wq_calc_node_cpumask(new_attrs, node, -1, tmp_attrs->cpumask)) { > - ctx->pwq_tbl[node] = alloc_unbound_pwq(wq, tmp_attrs); > - if (!ctx->pwq_tbl[node]) > + pwq = unbound_pwq_by_node(wq, node); > + if (pwq && wqattrs_equal(tmp_attrs, pwq->pool->attrs)) > + pwq = get_pwq_unlocked(pwq); Ah, okay, the function gets used here again. BTW, why does this function return anything? Can this function ever return something which isn't the pwq it was called with? > + else > + pwq = alloc_unbound_pwq(wq, tmp_attrs); > + if (!pwq) > goto out_free; If get_pwq_unlocked() can't fail, why are we testing for NULL pwq here? This code is kinda misleading. > + ctx->pwq_tbl[node] = pwq; > } else { > ctx->dfl_pwq->refcnt++; > ctx->pwq_tbl[node] = ctx->dfl_pwq; > @@ -3739,7 +3745,6 @@ static void wq_update_unbound_numa(struct workqueue_struct *wq, int cpu, > cpumask = target_attrs->cpumask; > > copy_workqueue_attrs(target_attrs, wq->unbound_attrs); > - pwq = unbound_pwq_by_node(wq, node); > > /* > * Let's determine what needs to be done. If the target cpumask is > @@ -3748,6 +3753,7 @@ static void wq_update_unbound_numa(struct workqueue_struct *wq, int cpu, > * equals the default pwq's, the default pwq should be used. > */ > if (wq_calc_node_cpumask(wq->dfl_pwq->pool->attrs, node, cpu_off, cpumask)) { > + pwq = unbound_pwq_by_node(wq, node); It'd be nice to note this change in the patch description. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/