From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQdMV-000364-6v for qemu-devel@nongnu.org; Mon, 30 Sep 2013 09:18:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VQdMQ-0003R6-5U for qemu-devel@nongnu.org; Mon, 30 Sep 2013 09:18:07 -0400 Received: from mail-vc0-f174.google.com ([209.85.220.174]:46839) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQdMQ-0003Qy-0T for qemu-devel@nongnu.org; Mon, 30 Sep 2013 09:18:02 -0400 Received: by mail-vc0-f174.google.com with SMTP id gd11so3756956vcb.33 for ; Mon, 30 Sep 2013 06:18:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4C38C504-350E-4492-89A9-AFF84BAE9836@alex.org.uk> References: <1378976540-10812-1-git-send-email-stefanha@redhat.com> <1378976540-10812-3-git-send-email-stefanha@redhat.com> <877gdy4gwj.fsf@pixel.localdomain> <4C38C504-350E-4492-89A9-AFF84BAE9836@alex.org.uk> Date: Mon, 30 Sep 2013 09:18:01 -0400 Message-ID: From: Mike Day Content-Type: multipart/alternative; boundary=047d7b6dc224619c9804e799aa62 Subject: Re: [Qemu-devel] [PATCH v4 2/3] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh Cc: Anthony Liguori , Paolo Bonzini , Ping Fan Liu , qemu-devel@nongnu.org, Stefan Hajnoczi --047d7b6dc224619c9804e799aa62 Content-Type: text/plain; charset=ISO-8859-1 > > > On Mon, Sep 30, 2013 at 8:55 AM, Alex Bligh wrote: > > > > > > On 30 Sep 2013, at 13:45, Mike Day wrote: > > > > > I've applied this set to Paolo's rcu tree - I see a couple of routines > > > that appear to need the active_timers_lock: > > > > > > (line 137 of qemu-timer.c in my tree) > > > void qemu_clock_notify(QEMUClockType type) > > > { > > > QEMUTimerList *timer_list; > > > QEMUClock *clock = qemu_clock_ptr(type); > > > QLIST_FOREACH(timer_list, &clock->timerlists, list) { > > > timerlist_notify(timer_list); > > > } > > > } > > > > > > (line 228 of qemu-timer.c in my tree) > > > int64_t qemu_clock_deadline_ns_all(QEMUClockType type) > > > { > > > int64_t deadline = -1; > > > QEMUTimerList *timer_list; > > > QEMUClock *clock = qemu_clock_ptr(type); > > > QLIST_FOREACH(timer_list, &clock->timerlists, list) { > > > deadline = qemu_soonest_timeout(deadline, > > > > timerlist_deadline_ns(timer_list)); > > > } > > > return deadline; > > > } > > > > > > I think these functions are always called now with the BQL held, so I > > > wonder if they are good candidates for RCU? > > > > These routines iterate through the list of timerlists held by > > a clock. > > > > They do not iterate through the list of active timers in a timer > > list. I believe the latter is what active_timers_lock protects. > > > > The list of timers attached to a clock is only modified when timers > > are created and deleted which is (currently) under the BQL. > > > > Sorry, and thanks for the correction re: active_timers_lock. I should have said that clock->timerlists may need its own mutex if and when we remove the BQL from the timer code. Mike --047d7b6dc224619c9804e799aa62 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Mon, Sep 30, 2013 = at 8:55 AM, Alex Bligh <alex@alex.or= g.uk> wrote:
>
>
> On 30 Sep 2013, at 13:45, Mike Day wrote:
>
&= gt; > I've applied this set to Paolo's rcu tree - I see a couple= of routines
> > that appear to need the active_timers_lock:
> >
> > (line 137 of qemu-timer.c in my tree)
> > v= oid qemu_clock_notify(QEMUClockType type)
> > {
> > =A0 = =A0QEMUTimerList *timer_list;
> > =A0 =A0QEMUClock *clock =3D qemu= _clock_ptr(type);
> > =A0 =A0QLIST_FOREACH(timer_list, &clock->timerlists, list)= {
> > =A0 =A0 =A0 =A0timerlist_notify(timer_list);
> > = =A0 =A0}
> > }
> >
> > (line 228 of qemu-timer.c= in my tree)
> > int64_t qemu_clock_deadline_ns_all(QEMUClockType type)
> &g= t; {
> > =A0 =A0int64_t deadline =3D -1;
> > =A0 =A0QEMUT= imerList *timer_list;
> > =A0 =A0QEMUClock *clock =3D qemu_clock_p= tr(type);
> > =A0 =A0QLIST_FOREACH(timer_list, &clock->timerlists, list)= {
> > =A0 =A0 =A0 =A0deadline =3D qemu_soonest_timeout(deadline,<= br>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0timerlist_deadline_ns(timer_list));
> > =A0 =A0}
> > =A0 =A0return deadline;
> > }
&= gt; >
> > I think these functions are always called now with th= e BQL held, so I
> > wonder if they are good candidates for RCU?>
> These routines iterate through the list of timerlists held by
> = a clock.
>
> They do not iterate through the list of active tim= ers in a timer
> list. I believe the latter is what active_timers_loc= k protects.
>
> The list of timers attached to a clock is only modified when t= imers
> are created and deleted which is (currently) under the BQL.>


Sorry,= and thanks for the correction re: active_timers_lock. I should have said t= hat clock->timerlists may need its own mutex if and when we remove the B= QL from the timer code.

Mike

--047d7b6dc224619c9804e799aa62--