LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alexei Lozovsky <me@ilammy.net>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	Christoph Lameter <cl@linux.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/7] proc/stat: Maintain monotonicity of "intr" and "softirq"
Date: Wed, 15 Sep 2021 13:24:11 +0900	[thread overview]
Message-ID: <44D2E875-33FF-4756-9FAB-7F2E1ED56139@ilammy.net> (raw)
In-Reply-To: <87y27zb62e.ffs@tglx>

Thanks for vetting my ideas!

On Tue, Sep 14, 2021, at 23:11, Thomas Gleixner wrote:
> On Sun, Sep 12 2021 at 21:37, Alexei Lozovsky wrote:
>> On Sun, Sep 12, 2021, at 18:30, Alexey Dobriyan wrote:
>>> How about making everything "unsigned long" or even "u64" like NIC
>>> drivers do?
>> 
>> I see some possible hurdles ahead:
>> 
>> - Not all architectures have atomic operations for 64-bit values
> 
> This is not about atomics.

Yeah, I got mixed up in terminology. As you said, atomic
read-modify-write for increment is not important here, but what
*is* important is absence of tearing when doing loads and stores.

If there is no tearing we don't need any barriers to observe counters
that make sense. They might be slightly outdated but we don't care
as long as they are observed to be monotonically increasing and
we don't see the low bits wrap before the high bits are updated
because 64-bit store got split into two 32-bit ones.

That said, I believe this rules out updating counter types to u64
because on 32-bit platforms those will tear. However, we can use
unsigned long so that platforms with 64-bit native words get 64-bit
counters and platforms with 32-bit words stay with 32-bit counters
that wrap like they should.

I've checked this on Godbolt for a number of archs and it seems that
all of them will emit single loads and stores for unsigned long.
Well, except for 16-bit platforms, but those would certainly not use
PPC or x86 and procfs in the first place, so I think we can ignore
them for this matter.

> On 32bit systems a 32bit load (as long as the compiler does not emit
> load tearing) is always consistent even when there is a concurrent
> increment going on. It either gets the old or the new value.

Regarding tearing, I thought about wrapping counter reads in READ_ONCE()
to signal that they should be performed in one load. __this_cpu_inc()
should probably do WRITE_ONCE() for the sake of pairing, but that
should not be too important.

Is it a good idea to use READ_ONCE here?
Or just assume that compiler will not emit any weird loads?

(READ_ONCE does not strictly check that reads will not tear. Right now
it allows unsigned long long because reasons. But I guess it will enable
some extra debugging checks.)

  reply	other threads:[~2021-09-15  4:24 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10  8:53 /proc/stat interrupt counter wrap-around Alexei Lozovsky
2021-09-11  3:48 ` [PATCH 0/7] proc/stat: Maintain monotonicity of "intr" and "softirq" Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 1/7] genirq: Use unsigned int for irqs_sum Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 2/7] powerpc/irq: arch_irq_stat_cpu() returns unsigned int Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 3/7] x86/irq: " Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 4/7] x86/irq: arch_irq_stat() " Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 5/7] proc/stat: Use unsigned int for "intr" sum Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 6/7] proc/stat: Use unsigned int for "softirq" sum Alexei Lozovsky
2021-09-11  3:48   ` [PATCH 7/7] docs: proc.rst: stat: Note the interrupt counter wrap-around Alexei Lozovsky
2021-09-11  3:59     ` Randy Dunlap
2021-09-12  9:30   ` [PATCH 0/7] proc/stat: Maintain monotonicity of "intr" and "softirq" Alexey Dobriyan
2021-09-12 12:37     ` Alexei Lozovsky
2021-09-14 14:11       ` Thomas Gleixner
2021-09-15  4:24         ` Alexei Lozovsky [this message]
2021-09-15 17:58   ` [PATCH v2 00/12] " Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 01/12] genirq: Use READ_ONCE for IRQ counter reads Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 02/12] genirq: Use unsigned long for IRQ counters Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 03/12] powerpc/irq: Use READ_ONCE for IRQ counter reads Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 04/12] powerpc/irq: Use unsigned long for IRQ counters Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 05/12] powerpc/irq: Use unsigned long for IRQ counter sum Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 06/12] x86/irq: Use READ_ONCE for IRQ counter reads Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 07/12] x86/irq: Use unsigned long for IRQ counters Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 08/12] x86/irq: Use unsigned long for IRQ counters more Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 09/12] x86/irq: Use unsigned long for IRQ counter sum Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 10/12] proc/stat: Use unsigned long for "intr" sum Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 11/12] proc/stat: Use unsigned long for "softirq" sum Alexei Lozovsky
2021-09-15 17:58     ` [PATCH v2 12/12] docs: proc.rst: stat: Note the interrupt counter wrap-around Alexei Lozovsky

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=44D2E875-33FF-4756-9FAB-7F2E1ED56139@ilammy.net \
    --to=me@ilammy.net \
    --cc=adobriyan@gmail.com \
    --cc=cl@linux.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /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).