From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [RFC PATCH] dpci: Put the dpci back on the list if running on another CPU. Date: Fri, 23 Jan 2015 09:54:20 -0500 Message-ID: <20150123145420.GI7365@l.oracle.com> References: <1421081130-22959-1-git-send-email-konrad.wilk@oracle.com> <54B4FF600200007800054376@mail.emea.novell.com> <20150123014459.GA6106@l.oracle.com> <54C2248102000078000589D6@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YEfd0-0004I3-AN for xen-devel@lists.xenproject.org; Fri, 23 Jan 2015 14:54:30 +0000 Content-Disposition: inline In-Reply-To: <54C2248102000078000589D6@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: andrew.cooper3@citrix.com, malcolm.crossley@citrix.com, xen-devel@lists.xenproject.org, linux@eikelenboom.it List-Id: xen-devel@lists.xenproject.org On Fri, Jan 23, 2015 at 09:37:53AM +0000, Jan Beulich wrote: > >>> On 23.01.15 at 02:44, wrote: > > Here is a patch that does this. I don't yet have an setup to test > > the failing scenario (working on that). I removed the part in > > the softirq because with this patch I cannot see a way it would > > ever get there (in the softirq hitting the BUG). > > Hmm, but how do you now schedule the second instance that needed ... > > > --- a/xen/drivers/passthrough/io.c > > +++ b/xen/drivers/passthrough/io.c > > @@ -64,16 +64,24 @@ static void raise_softirq_for(struct hvm_pirq_dpci > > *pirq_dpci) > > { > > unsigned long flags; > > > > - if ( test_and_set_bit(STATE_SCHED, &pirq_dpci->state) ) > > + switch ( cmpxchg(&pirq_dpci->state, 0, 1 << STATE_SCHED) ) > > + { > > + case (1 << STATE_SCHED): > > + case (1 << STATE_RUN): > > ... in this case? We do need that. I was under the false understanding that the old code would just drop it if it was running. However the old code (tasklet only) followed the logic that: a) If it is scheduled (on the list), then don't put it there. b) If it is running (t->running), then don't put it there. However once the function is done (in the do_tasklet_work) we detect that we tried to schedule it (t->scheduled_on is set to non-zero value) - and we put it back on the list. I need to think about this some more. > > >> Additionally I think it should be considered whether the bitmap > >> approach of interpreting ->state is the right one, and we don't > >> instead want a clean 3-state (idle, sched, run) model. > > > > Could you elaborate a bit more please? As in three different unsigned int > > (or bool_t) that set in what state we are in? > > An enum { STATE_IDLE, STATE_SCHED, STATE_RUN }. Especially > if my comment above turns out to be wrong, you'd have no real > need for the SCHED and RUN flags to be set at the same time. Seems we do need these four states: scheduled; running; schedulding while running; and idle. > > Jan >