From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 2/7] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS Date: Thu, 8 Oct 2015 14:18:10 +0100 Message-ID: <56166D12.4040809@citrix.com> References: <20151008124027.12522.42552.stgit@Solace.station> <20151008125243.12522.8999.stgit@Solace.station> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZkB5O-0004bV-Da for xen-devel@lists.xenproject.org; Thu, 08 Oct 2015 13:18:18 +0000 In-Reply-To: <20151008125243.12522.8999.stgit@Solace.station> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli , xen-devel@lists.xenproject.org Cc: George Dunlap , Meng Xu , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 08/10/15 13:52, Dario Faggioli wrote: > The insert_vcpu scheduler hook is called with an inconsistent > locking strategy. In fact, it is sometimes invoked while > holding the runqueue lock and sometimes when that is not the > case. > > For instance, in case of schedule_cpu_switch() the lock is > acquired in generic code. On the other hand, in case of > sched_move_domain(), locking is left as a responsibility > of the schedulers implementing the hook. > > This results in Credit1 and RTDS schedulers ending up (in > case of sched_move_domain()) doing runqueue manipulation > without holding any runqueue lock, which is a bug. (Credit2 > was doing the locking by itself already.) > > The right thing is to defer locking to the specific schedulers, > as it's them that know what, how and when it is best to lock > (as in: runqueue locks, vs. private scheduler locks, vs. both, > etc.). > > This patch, therefore: > - removes any locking around insert_vcpu() from generic > code; > - add proper locking in the hook implementations, for > both Credit1 and RTDS. > > Signed-off-by: Dario Faggioli Reviewed-by: Andrew Cooper