All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: James Morse <james.morse@arm.com>
Cc: jonathan.zhang@cavium.com, Rafael Wysocki <rjw@rjwysocki.net>,
	Tony Luck <tony.luck@intel.com>,
	linux-mm@kvack.org, Marc Zyngier <marc.zyngier@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Tyler Baicar <tbaicar@codeaurora.org>,
	Will Deacon <will.deacon@arm.com>,
	Dongjiu Geng <gengdongjiu@huawei.com>,
	linux-acpi@vger.kernel.org, Punit Agrawal <punit.agrawal@arm.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org, Len Brown <lenb@kernel.org>
Subject: Re: [PATCH v6 00/18] APEI in_nmi() rework
Date: Thu, 4 Oct 2018 17:15:55 +0200	[thread overview]
Message-ID: <20181004151555.GN1864@zn.tnic> (raw)
In-Reply-To: <c04d1b78-122b-d7f2-5a75-3d9c56386b11@arm.com>

On Wed, Oct 03, 2018 at 06:50:38PM +0100, James Morse wrote:

...

> The non-ghes HEST entries have a "number of records to pre-allocate" too, we
> could make this memory pool something hest.c looks after, but I can't see if the
> other error sources use those values.

Thanks for the detailed analysis!

> Hmmm, The size is capped to 64K, we could ignore the firmware description of the
> memory requirements, and allocate SZ_64K each time. Doing it per-GHES is still
> the only way to avoid allocating nmi-safe memory for irqs.

Right, so I'm thinking a lot simpler: allocate a pool which should
be large enough to handle all situations and drop all that logic
which recomputes and reallocates pool size. Just a static thing which
JustWorks(tm).

For a couple of reasons:

 - you state it above: all those synchronization issues are gone with a
 prellocated pool

 - 64K per-GHES pool is nothing if you consider the machines this thing
 runs on - fat servers with lotsa memory. And RAS there *is* important.
 And TBH 64K is nothing even on a small client sporting gigabytes of
 memory.

 - code is a lot simpler and cleaner - you don't need all that pool
 expanding and shrinking. I mean, I'm all for smarter solutions if they
 have any clear advantages warranting the complication but this is a
 lot of machinery just so that we can save a couple of KBs. Which, as a
 whole, sounds just too much to me.

But this is just me.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de>
To: James Morse <james.morse@arm.com>
Cc: linux-acpi@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
	Marc Zyngier <marc.zyngier@arm.com>,
	Christoffer Dall <christoffer.dall@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Rafael Wysocki <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	Tyler Baicar <tbaicar@codeaurora.org>,
	Dongjiu Geng <gengdongjiu@huawei.com>,
	Xie XiuQi <xiexiuqi@huawei.com>,
	Punit Agrawal <punit.agrawal@arm.com>,
	jonathan.zhang@cavium.com
Subject: Re: [PATCH v6 00/18] APEI in_nmi() rework
Date: Thu, 4 Oct 2018 17:15:55 +0200	[thread overview]
Message-ID: <20181004151555.GN1864@zn.tnic> (raw)
In-Reply-To: <c04d1b78-122b-d7f2-5a75-3d9c56386b11@arm.com>

On Wed, Oct 03, 2018 at 06:50:38PM +0100, James Morse wrote:

...

> The non-ghes HEST entries have a "number of records to pre-allocate" too, we
> could make this memory pool something hest.c looks after, but I can't see if the
> other error sources use those values.

Thanks for the detailed analysis!

> Hmmm, The size is capped to 64K, we could ignore the firmware description of the
> memory requirements, and allocate SZ_64K each time. Doing it per-GHES is still
> the only way to avoid allocating nmi-safe memory for irqs.

Right, so I'm thinking a lot simpler: allocate a pool which should
be large enough to handle all situations and drop all that logic
which recomputes and reallocates pool size. Just a static thing which
JustWorks(tm).

For a couple of reasons:

 - you state it above: all those synchronization issues are gone with a
 prellocated pool

 - 64K per-GHES pool is nothing if you consider the machines this thing
 runs on - fat servers with lotsa memory. And RAS there *is* important.
 And TBH 64K is nothing even on a small client sporting gigabytes of
 memory.

 - code is a lot simpler and cleaner - you don't need all that pool
 expanding and shrinking. I mean, I'm all for smarter solutions if they
 have any clear advantages warranting the complication but this is a
 lot of machinery just so that we can save a couple of KBs. Which, as a
 whole, sounds just too much to me.

But this is just me.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

WARNING: multiple messages have this Message-ID (diff)
From: bp@alien8.de (Borislav Petkov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 00/18] APEI in_nmi() rework
Date: Thu, 4 Oct 2018 17:15:55 +0200	[thread overview]
Message-ID: <20181004151555.GN1864@zn.tnic> (raw)
In-Reply-To: <c04d1b78-122b-d7f2-5a75-3d9c56386b11@arm.com>

On Wed, Oct 03, 2018 at 06:50:38PM +0100, James Morse wrote:

...

> The non-ghes HEST entries have a "number of records to pre-allocate" too, we
> could make this memory pool something hest.c looks after, but I can't see if the
> other error sources use those values.

Thanks for the detailed analysis!

> Hmmm, The size is capped to 64K, we could ignore the firmware description of the
> memory requirements, and allocate SZ_64K each time. Doing it per-GHES is still
> the only way to avoid allocating nmi-safe memory for irqs.

Right, so I'm thinking a lot simpler: allocate a pool which should
be large enough to handle all situations and drop all that logic
which recomputes and reallocates pool size. Just a static thing which
JustWorks(tm).

For a couple of reasons:

 - you state it above: all those synchronization issues are gone with a
 prellocated pool

 - 64K per-GHES pool is nothing if you consider the machines this thing
 runs on - fat servers with lotsa memory. And RAS there *is* important.
 And TBH 64K is nothing even on a small client sporting gigabytes of
 memory.

 - code is a lot simpler and cleaner - you don't need all that pool
 expanding and shrinking. I mean, I'm all for smarter solutions if they
 have any clear advantages warranting the complication but this is a
 lot of machinery just so that we can save a couple of KBs. Which, as a
 whole, sounds just too much to me.

But this is just me.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

  reply	other threads:[~2018-10-04 15:15 UTC|newest]

Thread overview: 123+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-21 22:16 [PATCH v6 00/18] APEI in_nmi() rework James Morse
2018-09-21 22:16 ` James Morse
2018-09-21 22:16 ` James Morse
2018-09-21 22:16 ` [PATCH v6 01/18] ACPI / APEI: Move the estatus queue code up, and under its own ifdef James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16 ` [PATCH v6 02/18] ACPI / APEI: Generalise the estatus queue's add/remove and notify code James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16 ` [PATCH v6 03/18] ACPI / APEI: don't wait to serialise with oops messages when panic()ing James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16 ` [PATCH v6 04/18] ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-09-28 17:04   ` Borislav Petkov
2018-09-28 17:04     ` Borislav Petkov
2018-09-28 17:04     ` Borislav Petkov
2018-09-21 22:16 ` [PATCH v6 05/18] ACPI / APEI: Make estatus queue a Kconfig symbol James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-01 17:59   ` Borislav Petkov
2018-10-01 17:59     ` Borislav Petkov
2018-10-01 17:59     ` Borislav Petkov
2018-10-03 17:50     ` James Morse
2018-10-03 17:50       ` James Morse
2018-10-03 17:50       ` James Morse
2018-10-04 17:34       ` Borislav Petkov
2018-10-04 17:34         ` Borislav Petkov
2018-10-04 17:34         ` Borislav Petkov
2018-10-12 17:17         ` James Morse
2018-10-12 17:17           ` James Morse
2018-10-12 17:17           ` James Morse
2018-10-12 18:10           ` Borislav Petkov
2018-10-12 18:10             ` Borislav Petkov
2018-10-12 18:10             ` Borislav Petkov
2018-09-21 22:16 ` [PATCH v6 06/18] KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-12  9:57   ` Borislav Petkov
2018-10-12  9:57     ` Borislav Petkov
2018-10-12  9:57     ` Borislav Petkov
2018-10-12 17:18     ` James Morse
2018-10-12 17:18       ` James Morse
2018-10-12 17:18       ` James Morse
2018-09-21 22:16 ` [PATCH v6 07/18] arm64: KVM/mm: Move SEA handling behind a single 'claim' interface James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-12 10:02   ` Borislav Petkov
2018-10-12 10:02     ` Borislav Petkov
2018-10-12 10:02     ` Borislav Petkov
2018-10-12 17:18     ` James Morse
2018-10-12 17:18       ` James Morse
2018-10-12 17:18       ` James Morse
2018-09-21 22:16 ` [PATCH v6 08/18] ACPI / APEI: Move locking to the notification helper James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-12 11:08   ` Borislav Petkov
2018-10-12 11:08     ` Borislav Petkov
2018-10-12 11:08     ` Borislav Petkov
2018-09-21 22:16 ` [PATCH v6 09/18] ACPI / APEI: Let the notification helper specify the fixmap slot James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-12 11:14   ` Borislav Petkov
2018-10-12 11:14     ` Borislav Petkov
2018-10-12 11:14     ` Borislav Petkov
2018-09-21 22:16 ` [PATCH v6 10/18] ACPI / APEI: preparatory split of ghes->estatus James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-12 16:37   ` Borislav Petkov
2018-10-12 16:37     ` Borislav Petkov
2018-10-12 16:37     ` Borislav Petkov
2018-09-21 22:16 ` [PATCH v6 11/18] ACPI / APEI: Remove silent flag from ghes_read_estatus() James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-10-12 16:55   ` Borislav Petkov
2018-10-12 16:55     ` Borislav Petkov
2018-10-12 16:55     ` Borislav Petkov
2018-09-21 22:16 ` [PATCH v6 12/18] ACPI / APEI: Don't store CPER records physical address in struct ghes James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:16   ` James Morse
2018-09-21 22:17 ` [PATCH v6 13/18] ACPI / APEI: Don't update struct ghes' flags in read/clear estatus James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17   ` James Morse
2018-10-12 17:14   ` Borislav Petkov
2018-10-12 17:14     ` Borislav Petkov
2018-10-12 17:14     ` Borislav Petkov
2018-09-21 22:17 ` [PATCH v6 14/18] ACPI / APEI: Split ghes_read_estatus() to read CPER length James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17   ` James Morse
2018-10-12 17:25   ` Borislav Petkov
2018-10-12 17:25     ` Borislav Petkov
2018-10-12 17:25     ` Borislav Petkov
2018-09-21 22:17 ` [PATCH v6 15/18] ACPI / APEI: Only use queued estatus entry during _in_nmi_notify_one() James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17   ` James Morse
2018-10-12 17:34   ` Borislav Petkov
2018-10-12 17:34     ` Borislav Petkov
2018-10-12 17:34     ` Borislav Petkov
2018-09-21 22:17 ` [PATCH v6 16/18] ACPI / APEI: Split fixmap pages for arm64 NMI-like notifications James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17 ` [PATCH v6 17/18] mm/memory-failure: increase queued recovery work's priority James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17   ` James Morse
2018-10-15 16:49   ` Borislav Petkov
2018-10-15 16:49     ` Borislav Petkov
2018-10-15 16:49     ` Borislav Petkov
2018-10-16  7:43     ` Peter Zijlstra
2018-10-16  7:43       ` Peter Zijlstra
2018-10-16  7:43       ` Peter Zijlstra
2018-09-21 22:17 ` [PATCH v6 18/18] arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work James Morse
2018-09-21 22:17   ` James Morse
2018-09-21 22:17   ` James Morse
2018-09-25 12:45 ` [PATCH v6 00/18] APEI in_nmi() rework Borislav Petkov
2018-09-25 12:45   ` Borislav Petkov
2018-09-25 12:45   ` Borislav Petkov
2018-10-03 17:50   ` James Morse
2018-10-03 17:50     ` James Morse
2018-10-03 17:50     ` James Morse
2018-10-04 15:15     ` Borislav Petkov [this message]
2018-10-04 15:15       ` Borislav Petkov
2018-10-04 15:15       ` Borislav Petkov

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=20181004151555.GN1864@zn.tnic \
    --to=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=gengdongjiu@huawei.com \
    --cc=james.morse@arm.com \
    --cc=jonathan.zhang@cavium.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mm@kvack.org \
    --cc=marc.zyngier@arm.com \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=punit.agrawal@arm.com \
    --cc=rjw@rjwysocki.net \
    --cc=tbaicar@codeaurora.org \
    --cc=tony.luck@intel.com \
    --cc=will.deacon@arm.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.