From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread Date: Tue, 27 Jul 2010 22:19:11 +0300 Message-ID: <20100727191911.GA16350@redhat.com> References: <20100724191447.GA4972@redhat.com> <4C4BEAA2.6040301@kernel.org> <20100726152510.GA26223@redhat.com> <4C4DAB14.5050809@kernel.org> <20100726155014.GA26412@redhat.com> <4C4DB247.9060709@kernel.org> <4C4DB466.6000409@kernel.org> <20100726165114.GA27353@redhat.com> <4C4DDE7E.8030406@kernel.org> <4C4DE2AE.40302@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Oleg Nesterov , Sridhar Samudrala , netdev , lkml , "kvm@vger.kernel.org" , Andrew Morton , Dmitri Vorobiev , Jiri Kosina , Thomas Gleixner , Ingo Molnar , Andi Kleen To: Tejun Heo Return-path: Content-Disposition: inline In-Reply-To: <4C4DE2AE.40302@kernel.org> Sender: kvm-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Mon, Jul 26, 2010 at 09:31:58PM +0200, Tejun Heo wrote: > Hello, > > On 07/26/2010 09:14 PM, Tejun Heo wrote: > > On 07/26/2010 06:51 PM, Michael S. Tsirkin wrote: > >> I noticed that with vhost, flush_work was getting the worker > >> pointer as well. Can we live with this API change? > > > > Yeah, the flushing mechanism wouldn't work reliably if the work is > > queued to a different worker without flushing, so yeah passing in > > @worker might actually be better. > > Thinking a bit more about it, it kind of sucks that queueing to > another worker from worker->func() breaks flush. Maybe the right > thing to do there is using atomic_t for done_seq? I don't believe it will help: we might have: worker1 runs work work requeues itself queued index = 1 worker1 reads queued index = 1 worker2 runs work work requeues itself queued index = 2 worker2 runs work worker2 reads queued index = 2 worker2 writes done index = 2 worker1 writes done index = 1 As you see, done index got moved back. > It pays a bit more > overhead but maybe that's justifiable to keep the API saner? It would > be great if it can be fixed somehow even if it means that the work has > to be separately flushed for each worker it has been on before being > destroyed. > > Or, if flushing has to be associated with a specific worker anyway, > maybe it would be better to move the sequence counter to > kthread_worker and do it similarly with the original workqueue so that > work can be destroyed once execution starts? Then, it can at least > remain semantically identical to the original workqueue. > > Thanks. > > -- > tejun