All of lore.kernel.org
 help / color / mirror / Atom feed
From: Quentin Perret <qperret@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: will@kernel.org, james.morse@arm.com, alexandru.elisei@arm.com,
	catalin.marinas@arm.com, suzuki.poulose@arm.com,
	linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kernel-team@android.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/7] KVM: arm64: Move hyp_pool locking out of refcount helpers
Date: Tue, 1 Jun 2021 13:31:06 +0000	[thread overview]
Message-ID: <YLY2mjxGCSyunnhV@google.com> (raw)
In-Reply-To: <87sg2123pj.wl-maz@kernel.org>

On Tuesday 01 Jun 2021 at 13:02:00 (+0100), Marc Zyngier wrote:
> On Thu, 27 May 2021 13:51:28 +0100,
> Quentin Perret <qperret@google.com> wrote:
> > 
> > The hyp_page refcount helpers currently rely on the hyp_pool lock for
> > serialization. However, this means the refcounts can't be changed from
> > the buddy allocator core as it already holds the lock, which means pages
> > have to go through odd transient states.
> > 
> > For example, when a page is freed, its refcount is set to 0, and the
> > lock is transiently released before the page can be attached to a free
> > list in the buddy tree. This is currently harmless as the allocator
> > checks the list node of each page to see if it is available for
> > allocation or not, but it means the page refcount can't be trusted to
> > represent the state of the page even if the pool lock is held.
> > 
> > In order to fix this, remove the pool locking from the refcount helpers,
> > and move all the logic to the buddy allocator. This will simplify the
> > removal of the list node from struct hyp_page in a later patch.
> 
> Is there any chance some documentation could be added so that we have
> a record of what the locking boundaries are? Something along the line
> of what we have in arch/arm64/kvm/vgic/vgic.c, for example.

Sounds like a good idea, I'll go write something.

Cheers,
Quentin

WARNING: multiple messages have this Message-ID (diff)
From: Quentin Perret <qperret@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: kernel-team@android.com, catalin.marinas@arm.com,
	linux-kernel@vger.kernel.org, will@kernel.org,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/7] KVM: arm64: Move hyp_pool locking out of refcount helpers
Date: Tue, 1 Jun 2021 13:31:06 +0000	[thread overview]
Message-ID: <YLY2mjxGCSyunnhV@google.com> (raw)
In-Reply-To: <87sg2123pj.wl-maz@kernel.org>

On Tuesday 01 Jun 2021 at 13:02:00 (+0100), Marc Zyngier wrote:
> On Thu, 27 May 2021 13:51:28 +0100,
> Quentin Perret <qperret@google.com> wrote:
> > 
> > The hyp_page refcount helpers currently rely on the hyp_pool lock for
> > serialization. However, this means the refcounts can't be changed from
> > the buddy allocator core as it already holds the lock, which means pages
> > have to go through odd transient states.
> > 
> > For example, when a page is freed, its refcount is set to 0, and the
> > lock is transiently released before the page can be attached to a free
> > list in the buddy tree. This is currently harmless as the allocator
> > checks the list node of each page to see if it is available for
> > allocation or not, but it means the page refcount can't be trusted to
> > represent the state of the page even if the pool lock is held.
> > 
> > In order to fix this, remove the pool locking from the refcount helpers,
> > and move all the logic to the buddy allocator. This will simplify the
> > removal of the list node from struct hyp_page in a later patch.
> 
> Is there any chance some documentation could be added so that we have
> a record of what the locking boundaries are? Something along the line
> of what we have in arch/arm64/kvm/vgic/vgic.c, for example.

Sounds like a good idea, I'll go write something.

Cheers,
Quentin
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Quentin Perret <qperret@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: will@kernel.org, james.morse@arm.com, alexandru.elisei@arm.com,
	catalin.marinas@arm.com, suzuki.poulose@arm.com,
	linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kernel-team@android.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/7] KVM: arm64: Move hyp_pool locking out of refcount helpers
Date: Tue, 1 Jun 2021 13:31:06 +0000	[thread overview]
Message-ID: <YLY2mjxGCSyunnhV@google.com> (raw)
In-Reply-To: <87sg2123pj.wl-maz@kernel.org>

On Tuesday 01 Jun 2021 at 13:02:00 (+0100), Marc Zyngier wrote:
> On Thu, 27 May 2021 13:51:28 +0100,
> Quentin Perret <qperret@google.com> wrote:
> > 
> > The hyp_page refcount helpers currently rely on the hyp_pool lock for
> > serialization. However, this means the refcounts can't be changed from
> > the buddy allocator core as it already holds the lock, which means pages
> > have to go through odd transient states.
> > 
> > For example, when a page is freed, its refcount is set to 0, and the
> > lock is transiently released before the page can be attached to a free
> > list in the buddy tree. This is currently harmless as the allocator
> > checks the list node of each page to see if it is available for
> > allocation or not, but it means the page refcount can't be trusted to
> > represent the state of the page even if the pool lock is held.
> > 
> > In order to fix this, remove the pool locking from the refcount helpers,
> > and move all the logic to the buddy allocator. This will simplify the
> > removal of the list node from struct hyp_page in a later patch.
> 
> Is there any chance some documentation could be added so that we have
> a record of what the locking boundaries are? Something along the line
> of what we have in arch/arm64/kvm/vgic/vgic.c, for example.

Sounds like a good idea, I'll go write something.

Cheers,
Quentin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-06-01 13:31 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-27 12:51 [PATCH 0/7] KVM: arm64: Reduce hyp_vmemmap overhead Quentin Perret
2021-05-27 12:51 ` Quentin Perret
2021-05-27 12:51 ` Quentin Perret
2021-05-27 12:51 ` [PATCH 1/7] KVM: arm64: Move hyp_pool locking out of refcount helpers Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-06-01 12:02   ` Marc Zyngier
2021-06-01 12:02     ` Marc Zyngier
2021-06-01 12:02     ` Marc Zyngier
2021-06-01 13:31     ` Quentin Perret [this message]
2021-06-01 13:31       ` Quentin Perret
2021-06-01 13:31       ` Quentin Perret
2021-05-27 12:51 ` [PATCH 2/7] KVM: arm64: Use refcount at hyp to check page availability Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51 ` [PATCH 3/7] KVM: arm64: Remove list_head from hyp_page Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-06-01 14:38   ` Marc Zyngier
2021-06-01 14:38     ` Marc Zyngier
2021-06-01 14:38     ` Marc Zyngier
2021-06-01 15:48     ` Quentin Perret
2021-06-01 15:48       ` Quentin Perret
2021-06-01 15:48       ` Quentin Perret
2021-06-01 17:41       ` Marc Zyngier
2021-06-01 17:41         ` Marc Zyngier
2021-06-01 17:41         ` Marc Zyngier
2021-06-02  9:23         ` Quentin Perret
2021-06-02  9:23           ` Quentin Perret
2021-06-02  9:23           ` Quentin Perret
2021-05-27 12:51 ` [PATCH 4/7] KVM: arm64: Unify MMIO and mem host stage-2 pools Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51 ` [PATCH 5/7] KVM: arm64: Remove hyp_pool pointer from struct hyp_page Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-06-01 15:00   ` Marc Zyngier
2021-06-01 15:00     ` Marc Zyngier
2021-06-01 15:00     ` Marc Zyngier
2021-05-27 12:51 ` [PATCH 6/7] KVM: arm64: Use less bits for hyp_page order Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51 ` [PATCH 7/7] KVM: arm64: Use less bits for hyp_page refcount Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-05-27 12:51   ` Quentin Perret
2021-06-01 15:21   ` Marc Zyngier
2021-06-01 15:21     ` Marc Zyngier
2021-06-01 15:21     ` Marc Zyngier

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=YLY2mjxGCSyunnhV@google.com \
    --to=qperret@google.com \
    --cc=alexandru.elisei@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=kernel-team@android.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    /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.