All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sergey Fedorov <serge.fdrv@gmail.com>,
	mttcg@greensocs.com, qemu-devel@nongnu.org,
	fred konrad <fred.konrad@greensocs.com>,
	a rigo <a.rigo@virtualopensystems.com>,
	cota@braap.org, bobby prani <bobby.prani@gmail.com>,
	mark burton <mark.burton@greensocs.com>,
	jan kiszka <jan.kiszka@siemens.com>,
	rth@twiddle.net, peter maydell <peter.maydell@linaro.org>,
	claudio fontana <claudio.fontana@huawei.com>
Subject: Re: [Qemu-devel] [RFC 0/7] Safe watch and breakpoint manipulation
Date: Mon, 20 Jun 2016 17:27:24 +0100	[thread overview]
Message-ID: <8737o7su4z.fsf@linaro.org> (raw)
In-Reply-To: <702806117.314024.1466438917254.JavaMail.zimbra@redhat.com>


Paolo Bonzini <pbonzini@redhat.com> writes:

>> > The patch series changes things in stages.
>> >
>> > First we move the break/watchpoints into an array which is more
>> > amenable to RCU control that the QLIST. We then control the life time
>> > of references to break/watchpoint data by removing long held
>> > references in the target code and getting information when needed from
>> > the core. Then we stop dynamically allocation the watch/breakpoint
>> > data and store it directly in the array which makes iteration across
>> > the list a bit more cache friendly than referenced pointers. Finally
>> > addition and removal of elements of the array is put under RCU
>> > control. This ensures there is always a safe array of data to check
>> > in the run-loop.
>>
>> I a little bit unsure if we really want to complicate things with RCU.
>> Why don't we simply protect the lists with a mutex given that there's no
>> contention expected? BTW, as it comes to debugging, I suppose we don't
>> expect great performance anyway.
>
> Mutexes do introduce some overhead.  The breakpoints list are mostly touched
> during translation, but watchpoints aren't so we could use tb_lock for
> breakpoints and a separate per-CPU mutex for watchpoints.  That could
> indeed work.

The watchpoint contention is the biggest one. FWIW I like the RCU
approach because it is low impact when running (and I'm hoping faster as
well by not being a linked list).

It's not a major problem in system mode because generally the system is
halted when changes are made to the list. However I'd like to solve it
properly for both system and user-mode so I can then forgot about
another special case.

--
Alex Bennée

  reply	other threads:[~2016-06-20 16:27 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-17 16:33 [Qemu-devel] [RFC 0/7] Safe watch and breakpoint manipulation Alex Bennée
2016-06-17 16:33 ` [RFC 1/7] cpu: move break/watchpoints into arrays Alex Bennée
2016-06-17 16:33   ` [Qemu-devel] " Alex Bennée
2016-06-17 17:03   ` Paolo Bonzini
2016-06-17 17:03     ` [Qemu-devel] " Paolo Bonzini
2016-06-17 16:33 ` [RFC 2/7] exec: keep CPUWatchpoint references internal Alex Bennée
2016-06-17 16:33   ` [Qemu-devel] " Alex Bennée
2016-06-17 17:17   ` Paolo Bonzini
2016-06-17 17:17     ` [Qemu-devel] " Paolo Bonzini
2016-06-17 16:33 ` [RFC 3/7] exec: keep CPUBreakpoint " Alex Bennée
2016-06-17 16:33   ` [Qemu-devel] " Alex Bennée
2016-06-17 16:33 ` [RFC 4/7] break/watchpoints: store inside array Alex Bennée
2016-06-17 16:33   ` [Qemu-devel] " Alex Bennée
2016-06-17 17:15   ` Paolo Bonzini
2016-06-17 17:15     ` [Qemu-devel] " Paolo Bonzini
2016-06-17 16:33 ` [Qemu-devel] [RFC 5/7] breakpoints: put breakpoints under RCU control Alex Bennée
2016-06-17 16:59   ` Paolo Bonzini
2016-06-17 16:33 ` [Qemu-devel] [RFC 6/7] linux-user: don't clone watchpoints Alex Bennée
2016-06-17 17:18   ` Paolo Bonzini
2016-06-17 16:33 ` [Qemu-devel] [RFC 7/7] watchpoints: put watchpoints under RCU control Alex Bennée
2016-06-17 17:10   ` Paolo Bonzini
2016-06-17 17:01 ` [Qemu-devel] [RFC 0/7] Safe watch and breakpoint manipulation Paolo Bonzini
2016-06-20 13:55   ` Sergey Fedorov
2016-06-20 14:25     ` Paolo Bonzini
2016-06-20 15:23       ` Sergey Fedorov
2016-06-20 15:49 ` Sergey Fedorov
2016-06-20 16:08   ` Paolo Bonzini
2016-06-20 16:27     ` Alex Bennée [this message]
2016-06-20 18:16       ` Sergey Fedorov
2016-06-20 18:19     ` Sergey Fedorov

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=8737o7su4z.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=a.rigo@virtualopensystems.com \
    --cc=bobby.prani@gmail.com \
    --cc=claudio.fontana@huawei.com \
    --cc=cota@braap.org \
    --cc=fred.konrad@greensocs.com \
    --cc=jan.kiszka@siemens.com \
    --cc=mark.burton@greensocs.com \
    --cc=mttcg@greensocs.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=serge.fdrv@gmail.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.