linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Hillf Danton <hdanton@sina.com>
Cc: "tj@kernel.org" <tj@kernel.org>,
	Trond Myklebust <trondmy@hammerspace.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH rfc] workqueue: honour cond_resched() more effectively.
Date: Fri, 20 Nov 2020 10:07:56 +1100	[thread overview]
Message-ID: <87ima1ndwz.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <20201118055108.358-1-hdanton@sina.com>

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

On Wed, Nov 18 2020, Hillf Danton wrote:

> On Wed, 18 Nov 2020 16:11:44 +1100 NeilBrown wrote:
>> On Wed, Nov 18 2020, Hillf Danton wrote:
>> ...
>> I don't think this is a good idea.
>
> Let me add a few more words.
>
>> cond_resched() is expected to be called often.  Adding all this extra
>
> They are those only invoked in concurrency-managed worker contexts and
> are thus supposed to be less often than thought; what is more the callers
> know what they are doing if a schedule() follows up, needless to say it
> is an ant-antenna-size add-in to check WORKER_CPU_INTENSIVE given
> 	WARN_ON_ONCE(workqueue_mustnt_use_cpu())
> added in cond_resched().

"supposed to be less often" is the central point here.
Because the facts are that they sometime happen with high frequency
despite what is "supposed" to happen.
Either the assumption that CM-workers don't call cond_resched() is
wrong, or the code that schedules such workers on CM-queues is wrong.

I much prefer the perspective that the assumption is wrong.  If that is
agreed then we need to handle that circumstance without making
cond_resched() more expensive.
Note that adding WARN_ON_ONCE() does not make it more expensive as it is
only enabled with KERNEL_DEBUG (and WQ_WATCHDOG, though the particular
config option could be changed). It isn't needed in production.

If the workqueue maintainers are unmovable in the position that a
CM-workitem must not use excessive CPU ever, and so must not call
cond_resched(), then I can take that back to the NFS maintainers and
negotiate different workqueue settings.  But as I've said, I think this
is requiring the decision to be made in a place that is not well
positioned to make it.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]

  parent reply	other threads:[~2020-11-19 23:08 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-09  2:54 [PATCH rfc] workqueue: honour cond_resched() more effectively NeilBrown
2020-11-09  7:50 ` Michal Hocko
2020-11-09  8:00 ` Peter Zijlstra
2020-11-09 13:50   ` Trond Myklebust
2020-11-09 14:01     ` tj
2020-11-09 14:11       ` Trond Myklebust
2020-11-09 16:10         ` tj
2020-11-17 22:16           ` NeilBrown
     [not found]           ` <20201118025820.307-1-hdanton@sina.com>
2020-11-18  5:11             ` NeilBrown
     [not found]             ` <20201118055108.358-1-hdanton@sina.com>
2020-11-19 23:07               ` NeilBrown [this message]
2020-12-02 20:20                 ` tj
     [not found]               ` <20201120025953.607-1-hdanton@sina.com>
2020-11-20  4:33                 ` NeilBrown
     [not found]                 ` <20201126100646.1790-1-hdanton@sina.com>
2020-11-26 23:44                   ` NeilBrown
2020-11-19 23:23           ` NeilBrown
2020-11-25 12:36             ` tj
2020-11-26 23:30               ` NeilBrown
2020-11-09 14:20     ` Peter Zijlstra
2020-11-10  2:26       ` NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ima1ndwz.fsf@notabene.neil.brown.name \
    --to=neilb@suse.de \
    --cc=hdanton@sina.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tj@kernel.org \
    --cc=trondmy@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).