From: Avi Kivity <avi@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>, kvm <kvm@vger.kernel.org>,
Zachary Amsden <zamsden@redhat.com>
Subject: Re: [PATCH 2/2] KVM: Convert read-only users of vm_list to RCU
Date: Thu, 10 Feb 2011 14:34:28 +0200 [thread overview]
Message-ID: <4D53DB54.90605@redhat.com> (raw)
In-Reply-To: <4D53CCAB.8040204@siemens.com>
On 02/10/2011 01:31 PM, Jan Kiszka wrote:
> >
> >>
> >> @@ -3607,10 +3607,14 @@ static int mmu_shrink(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask)
> >> spin_unlock(&kvm->mmu_lock);
> >> srcu_read_unlock(&kvm->srcu, idx);
> >> }
> >> - if (kvm_freed)
> >> - list_move_tail(&kvm_freed->vm_list,&vm_list);
> >> + if (kvm_freed) {
> >> + raw_spin_lock(&kvm_lock);
> >> + if (!kvm->deleted)
> >> + list_move_tail(&kvm_freed->vm_list,&vm_list);
> >
> > There is no list_move_tail_rcu().
>
> ...specifically not for this one.
Well, we can add one if needed (and if possible).
> >
> > Why check kvm->deleted? it's in the process of being torn down anyway,
> > it doesn't matter if mmu_shrink or kvm_destroy_vm pulls the trigger.
>
> kvm_destroy_vm removes a vm from the list while mmu_shrink is running.
> Then mmu_shrink's list_move_tail will re-add that vm to the list tail
> again (unless already the removal in move_tail produces a crash).
It's too subtle. Communication across threads with a variable needs
memory barriers (even though they're nops on x86) and documentation.
btw, not even sure if it's legal: you have a mutating call within an rcu
read critical section for the same object. If synchronize_rcu() were
called there, would it ever terminate?
(not that synchronize_rcu() is a good thing there, better do it with
call_rcu()).
--
error compiling committee.c: too many arguments to function
next prev parent reply other threads:[~2011-02-10 12:34 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <4D512EF7.8040409@siemens.com>
2011-02-08 11:55 ` [PATCH 2/2] KVM: Convert read-only users of vm_list to RCU Jan Kiszka
2011-02-10 10:16 ` Avi Kivity
2011-02-10 11:31 ` Jan Kiszka
2011-02-10 12:34 ` Avi Kivity [this message]
2011-02-10 12:45 ` Jan Kiszka
2011-02-10 12:56 ` Avi Kivity
2011-02-10 12:57 ` Avi Kivity
2011-02-10 13:14 ` Jan Kiszka
2011-02-10 13:19 ` Avi Kivity
2011-02-10 13:47 ` Jan Kiszka
2011-02-10 14:26 ` Avi Kivity
2011-02-10 14:34 ` Jan Kiszka
2011-02-10 14:47 ` Avi Kivity
2011-02-10 14:55 ` Jan Kiszka
2011-02-15 12:32 ` Marcelo Tosatti
2011-02-15 14:08 ` Jan Kiszka
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=4D53DB54.90605@redhat.com \
--to=avi@redhat.com \
--cc=jan.kiszka@siemens.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=zamsden@redhat.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.