linux-kernel - Re: [PATCH rfc] workqueue: honour cond

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]

Date:   Mon, 9 Nov 2020 11:10:07 -0500
From:   "tj@...nel.org" <tj@...nel.org>
To:     Trond Myklebust <trondmy@...merspace.com>
Cc:     "jiangshanlai@...il.com" <jiangshanlai@...il.com>,
        "mhocko@...e.com" <mhocko@...e.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "juri.lelli@...hat.com" <juri.lelli@...hat.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "neilb@...e.de" <neilb@...e.de>
Subject: Re: [PATCH rfc] workqueue: honour cond_resched() more effectively.

Hello,

On Mon, Nov 09, 2020 at 02:11:42PM +0000, Trond Myklebust wrote:
> That means changing all filesystem code to use cpu-intensive queues. As
> far as I can tell, they all use workqueues (most of them using the
> standard system queue) for fput(), dput() and/or iput() calls.

I suppose the assumption was that those operations couldn't possiby be
expensive enough to warrant other options, which doesn't seem to be the case
unfortunately. Switching the users to system_unbound_wq, which should be
pretty trivial, seems to be the straight forward solution.

I can definitely see benefits in making workqueue smarter about
concurrency-managed work items taking a long time. Given that nothing on
these types of workqueues can be latency sensitive and the problem being
reported is on the scale of tens of seconds, I think a more palatable
approach could be through watchdog mechanism rather than hooking into
cond_resched(). Something like:

* Run watchdog timer more frequently - e.g. 1/4 of threshold.

* If a work item is occupying the local concurrency for too long, set
  WORKER_CPU_INTENSIVE for the worker and, probably, generate a warning.

I still think this should generate a warning and thus can't replace
switching to unbound wq. The reason is that the concurrency limit isn't the
only problem. A kthread needing to run on one particular CPU for tens of
seconds just isn't great.

Thanks.

-- 
tejun