All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Yunfeng Ye <yeyunfeng@huawei.com>,
	fweisbec@gmail.com, mingo@kernel.org,
	linux-kernel@vger.kernel.org, Shiyuan Hu <hushiyuan@huawei.com>,
	Hewenliang <hewenliang4@huawei.com>
Subject: Re: [PATCH] tick/nohz: Reduce the critical region for jiffies_seq
Date: Sun, 15 Nov 2020 20:43:39 +0100	[thread overview]
Message-ID: <87h7pq8kyc.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <ac822c72-673e-73e1-9622-c5f12591b373@huawei.com>

On Wed, Nov 11 2020 at 17:11, Yunfeng Ye wrote:
> When nohz or nohz_full is configured, the concurrency calls of
> tick_do_update_jiffies64 increases,

Why?

> and the conflict between jiffies_lock and jiffies_seq increases,
> especially in multi-core scenarios.

This does not make sense. The sequence counter is updated when holding
the lock, so there is no conflict between the lock and the sequence
count.

> However, it is unnecessary to update the jiffies_seq lock multiple
> times in a tick period, so the critical region of the jiffies_seq
> can be reduced to reduce latency overheads.

This does not make sense either. Before taking the lock we have:

        delta = ktime_sub(now, READ_ONCE(last_jiffies_update));
        if (delta < tick_period)
                return;

as a lockless quick check.

We also have mechanisms to avoid that a gazillion of CPUs call this. Why
are they not working or are some of the callsites missing them?

I'm not against reducing the seqcount write scope per se, but it needs a
proper and correct explanation.

> By the way, last_jiffies_update is protected by jiffies_lock, so
> reducing the jiffies_seq critical area is safe.

This is misleading. The write to last_jiffies_update is serialized by
the jiffies lock, but the write has also to be inside the sequence write
held section because tick_nohz_next_event() does:

	/* Read jiffies and the time when jiffies were updated last */
	do {
		seq = read_seqcount_begin(&jiffies_seq);
		basemono = last_jiffies_update;
		basejiff = jiffies;
	} while (read_seqcount_retry(&jiffies_seq, seq));

So there is no 'By the way'.

Thanks,

        tglx

  reply	other threads:[~2020-11-15 19:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11  9:11 [PATCH] tick/nohz: Reduce the critical region for jiffies_seq Yunfeng Ye
2020-11-15 19:43 ` Thomas Gleixner [this message]
2020-11-16  6:07   ` Yunfeng Ye
2020-11-16 11:29     ` Thomas Gleixner
2020-11-16 13:24       ` Yunfeng Ye
2020-11-16 14:14         ` Thomas Gleixner

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=87h7pq8kyc.fsf@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=fweisbec@gmail.com \
    --cc=hewenliang4@huawei.com \
    --cc=hushiyuan@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=yeyunfeng@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.