linux-kernel - Re: [PATCH 1/2 V3] workqueue: substitute POOL_FREEZING with __WQ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140421222035.GA22730@htj.dyndns.org>
Date:	Mon, 21 Apr 2014 18:20:35 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2 V3] workqueue: substitute POOL_FREEZING with
 __WQ_FREEZING

On Mon, Apr 21, 2014 at 07:59:20PM +0800, Lai Jiangshan wrote:
> Only workqueues have freezable or freezing attribution/state, not worker pools.
> But POOL_FREEZING adds a suspicious state and makes reviewers confused.
> 
> And it causes freeze_workqueues_begin() and thaw_workqueues() much complicated,
> they need to travel all the pools besides wqs.
> 
> Since freezable is workqueue instance's attribution, and freezing
> is workqueue instance's state, so we introduce __WQ_FREEZING
> to wq->flags instead and remove POOL_FREEZING.
> 
> It is different from POOL_FREEZING, POOL_FREEZING is simply set
> all over the world(all pools), while __WQ_FREEZING is only set for freezable wq.
> freeze_workqueues_begin()/thaw_workqueues() skip to handle non-freezable wqs
> and don't touch the non-freezable wqs' flags.

I was about to apply the patch and have updated the patch description.

  While freezing takes place globally, its execution is per-workqueue;
  however, the current implementation makes use of the per-worker_pool
  POOL_FREEZING flag.  While it's not broken, the flag makes the code
  more confusing and complicates freeze_workqueues_begin() and
  thaw_workqueues() by requiring them to walk through all pools.

  Since freezable is a workqueue's attribute, and freezing is a
  workqueue's state, let's introduce __WQ_FREEZING to wq->flags instead
  and remove POOL_FREEZING.

  It is different from POOL_FREEZING in that __WQ_FREEZING is only set
  for freezable workqueues while POOL_FREEZING is set globally over all
  pools.  freeze_workqueues_begin() and thaw_workqueues() now skip
  non-freezable workqueues.

But looking at the patch, why do we need __WQ_FREEZING at all?  We
should be able to test workqueue_freezing in pwq_adjust_max_active(),
right?  The only requirement there would be that
pwq_adjust_max_active(0 is invoked at least once after
workqueue_freezing is changed, which is already guaranteed.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/