From patchwork Mon May 11 11:28:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?SsO8cmdlbiBHcm/Dnw==?= X-Patchwork-Id: 11540461 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 94A9A15AB for ; Mon, 11 May 2020 11:30:14 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7A5F920708 for ; Mon, 11 May 2020 11:30:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7A5F920708 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jY6bx-0001fw-8p; Mon, 11 May 2020 11:28:41 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jY6bv-0001fi-IU for xen-devel@lists.xenproject.org; Mon, 11 May 2020 11:28:39 +0000 X-Inumbo-ID: 8c28f088-937a-11ea-9887-bc764e2007e4 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 8c28f088-937a-11ea-9887-bc764e2007e4; Mon, 11 May 2020 11:28:34 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 63D35AECD; Mon, 11 May 2020 11:28:35 +0000 (UTC) From: Juergen Gross To: xen-devel@lists.xenproject.org Subject: [PATCH v2 1/3] xen/sched: allow rcu work to happen when syncing cpus in core scheduling Date: Mon, 11 May 2020 13:28:27 +0200 Message-Id: <20200511112829.5500-2-jgross@suse.com> X-Mailer: git-send-email 2.26.1 In-Reply-To: <20200511112829.5500-1-jgross@suse.com> References: <20200511112829.5500-1-jgross@suse.com> MIME-Version: 1.0 X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Juergen Gross , Sergey Dyasli , Stefano Stabellini , Julien Grall , Wei Liu , Andrew Cooper , Ian Jackson , George Dunlap , Dario Faggioli , Jan Beulich Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" With RCU barriers moved from tasklets to normal RCU processing cpu offlining in core scheduling might deadlock due to cpu synchronization required by RCU processing and core scheduling concurrently. Fix that by bailing out from core scheduling synchronization in case of pending RCU work. Additionally the RCU softirq is now required to be of higher priority than the scheduling softirqs in order to do RCU processing before entering the scheduler again, as bailing out from the core scheduling synchronization requires to raise another softirq SCHED_SLAVE, which would bypass RCU processing again. Reported-by: Sergey Dyasli Tested-by: Sergey Dyasli Signed-off-by: Juergen Gross Acked-by: Dario Faggioli --- V2: - add BUILD_BUG_ON() and comment (Dario Faggioli) --- xen/common/sched/core.c | 13 ++++++++++--- xen/include/xen/softirq.h | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index d94b95285f..5df66cbf9b 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -2457,13 +2457,20 @@ static struct sched_unit *sched_wait_rendezvous_in(struct sched_unit *prev, v = unit2vcpu_cpu(prev, cpu); } /* - * Coming from idle might need to do tasklet work. + * Check for any work to be done which might need cpu synchronization. + * This is either pending RCU work, or tasklet work when coming from + * idle. It is mandatory that RCU softirqs are of higher priority + * than scheduling ones as otherwise a deadlock might occur. * In order to avoid deadlocks we can't do that here, but have to - * continue the idle loop. + * schedule the previous vcpu again, which will lead to the desired + * processing to be done. * Undo the rendezvous_in_cnt decrement and schedule another call of * sched_slave(). */ - if ( is_idle_unit(prev) && sched_tasklet_check_cpu(cpu) ) + BUILD_BUG_ON(RCU_SOFTIRQ > SCHED_SLAVE_SOFTIRQ || + RCU_SOFTIRQ > SCHEDULE_SOFTIRQ); + if ( rcu_pending(cpu) || + (is_idle_unit(prev) && sched_tasklet_check_cpu(cpu)) ) { struct vcpu *vprev = current; diff --git a/xen/include/xen/softirq.h b/xen/include/xen/softirq.h index b4724f5c8b..1f6c4783da 100644 --- a/xen/include/xen/softirq.h +++ b/xen/include/xen/softirq.h @@ -4,10 +4,10 @@ /* Low-latency softirqs come first in the following list. */ enum { TIMER_SOFTIRQ = 0, + RCU_SOFTIRQ, SCHED_SLAVE_SOFTIRQ, SCHEDULE_SOFTIRQ, NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ, - RCU_SOFTIRQ, TASKLET_SOFTIRQ, NR_COMMON_SOFTIRQS }; From patchwork Mon May 11 11:28:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?SsO8cmdlbiBHcm/Dnw==?= X-Patchwork-Id: 11540459 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AB3AE139A for ; Mon, 11 May 2020 11:29:49 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8E42F20735 for ; Mon, 11 May 2020 11:29:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8E42F20735 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jY6bs-0001fY-0i; Mon, 11 May 2020 11:28:36 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jY6br-0001fN-61 for xen-devel@lists.xenproject.org; Mon, 11 May 2020 11:28:35 +0000 X-Inumbo-ID: 8c1673a4-937a-11ea-a200-12813bfff9fa Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 8c1673a4-937a-11ea-a200-12813bfff9fa; Mon, 11 May 2020 11:28:33 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 7D308AEC1; Mon, 11 May 2020 11:28:35 +0000 (UTC) From: Juergen Gross To: xen-devel@lists.xenproject.org Subject: [PATCH v2 2/3] xen/sched: don't call sync_vcpu_execstate() in sched_unit_migrate_finish() Date: Mon, 11 May 2020 13:28:28 +0200 Message-Id: <20200511112829.5500-3-jgross@suse.com> X-Mailer: git-send-email 2.26.1 In-Reply-To: <20200511112829.5500-1-jgross@suse.com> References: <20200511112829.5500-1-jgross@suse.com> MIME-Version: 1.0 X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Juergen Gross , George Dunlap , Dario Faggioli Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" With support of core scheduling sched_unit_migrate_finish() gained a call of sync_vcpu_execstate() as it was believed to be called as a result of vcpu migration in any case. In case of migrating a vcpu away from a physical cpu for a short period of time only this might not be true, so drop the call and let the lazy state syncing do its job. Signed-off-by: Juergen Gross Reviewed-by: Jan Beulich --- V2: - new patch --- xen/common/sched/core.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index 5df66cbf9b..cb49a8bc02 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -1078,12 +1078,7 @@ static void sched_unit_migrate_finish(struct sched_unit *unit) sched_spin_unlock_double(old_lock, new_lock, flags); if ( old_cpu != new_cpu ) - { - /* Vcpus are moved to other pcpus, commit their states to memory. */ - for_each_sched_unit_vcpu ( unit, v ) - sync_vcpu_execstate(v); sched_move_irqs(unit); - } /* Wake on new CPU. */ for_each_sched_unit_vcpu ( unit, v ) From patchwork Mon May 11 11:28:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?SsO8cmdlbiBHcm/Dnw==?= X-Patchwork-Id: 11540455 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id F2FF415AB for ; Mon, 11 May 2020 11:29:08 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D82A720722 for ; Mon, 11 May 2020 11:29:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D82A720722 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jY6c2-0001gW-Hp; Mon, 11 May 2020 11:28:46 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jY6c0-0001gC-JS for xen-devel@lists.xenproject.org; Mon, 11 May 2020 11:28:44 +0000 X-Inumbo-ID: 8c77ccd0-937a-11ea-ae69-bc764e2007e4 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 8c77ccd0-937a-11ea-ae69-bc764e2007e4; Mon, 11 May 2020 11:28:34 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id BD775AE91; Mon, 11 May 2020 11:28:35 +0000 (UTC) From: Juergen Gross To: xen-devel@lists.xenproject.org Subject: [PATCH v2 3/3] xen/sched: fix latent races accessing vcpu->dirty_cpu Date: Mon, 11 May 2020 13:28:29 +0200 Message-Id: <20200511112829.5500-4-jgross@suse.com> X-Mailer: git-send-email 2.26.1 In-Reply-To: <20200511112829.5500-1-jgross@suse.com> References: <20200511112829.5500-1-jgross@suse.com> MIME-Version: 1.0 X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Juergen Gross , Stefano Stabellini , Julien Grall , Wei Liu , Andrew Cooper , Ian Jackson , George Dunlap , Jan Beulich , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" The dirty_cpu field of struct vcpu denotes which cpu still holds data of a vcpu. All accesses to this field should be atomic in case the vcpu could just be running, as it is accessed without any lock held in most cases. Especially sync_local_execstate() and context_switch() for the same vcpu running concurrently have a risk for failing. There are some instances where accesses are not atomically done, and even worse where multiple accesses are done when a single one would be mandated. Correct that in order to avoid potential problems. Add some assertions to verify dirty_cpu is handled properly. Signed-off-by: Juergen Gross --- V2: - convert all accesses to v->dirty_cpu to atomic ones (Jan Beulich) - drop cast (Julien Grall) --- xen/arch/x86/domain.c | 16 +++++++++++----- xen/common/domain.c | 2 +- xen/common/keyhandler.c | 2 +- xen/include/xen/sched.h | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index a4428190d5..2e5717b983 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -183,7 +183,7 @@ void startup_cpu_idle_loop(void) ASSERT(is_idle_vcpu(v)); cpumask_set_cpu(v->processor, v->domain->dirty_cpumask); - v->dirty_cpu = v->processor; + write_atomic(&v->dirty_cpu, v->processor); reset_stack_and_jump(idle_loop); } @@ -1769,6 +1769,7 @@ static void __context_switch(void) if ( !is_idle_domain(pd) ) { + ASSERT(read_atomic(&p->dirty_cpu) == cpu); memcpy(&p->arch.user_regs, stack_regs, CTXT_SWITCH_STACK_BYTES); vcpu_save_fpu(p); pd->arch.ctxt_switch->from(p); @@ -1832,7 +1833,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next) { unsigned int cpu = smp_processor_id(); const struct domain *prevd = prev->domain, *nextd = next->domain; - unsigned int dirty_cpu = next->dirty_cpu; + unsigned int dirty_cpu = read_atomic(&next->dirty_cpu); ASSERT(prev != next); ASSERT(local_irq_is_enabled()); @@ -1844,6 +1845,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next) { /* Remote CPU calls __sync_local_execstate() from flush IPI handler. */ flush_mask(cpumask_of(dirty_cpu), FLUSH_VCPU_STATE); + ASSERT(!vcpu_cpu_dirty(next)); } _update_runstate_area(prev); @@ -1956,13 +1958,17 @@ void sync_local_execstate(void) void sync_vcpu_execstate(struct vcpu *v) { - if ( v->dirty_cpu == smp_processor_id() ) + unsigned int dirty_cpu = read_atomic(&v->dirty_cpu); + + if ( dirty_cpu == smp_processor_id() ) sync_local_execstate(); - else if ( vcpu_cpu_dirty(v) ) + else if ( is_vcpu_dirty_cpu(dirty_cpu) ) { /* Remote CPU calls __sync_local_execstate() from flush IPI handler. */ - flush_mask(cpumask_of(v->dirty_cpu), FLUSH_VCPU_STATE); + flush_mask(cpumask_of(dirty_cpu), FLUSH_VCPU_STATE); } + ASSERT(!is_vcpu_dirty_cpu(dirty_cpu) || + read_atomic(&v->dirty_cpu) != dirty_cpu); } static int relinquish_memory( diff --git a/xen/common/domain.c b/xen/common/domain.c index 7cc9526139..70ff05eefc 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -158,7 +158,7 @@ struct vcpu *vcpu_create(struct domain *d, unsigned int vcpu_id) v->domain = d; v->vcpu_id = vcpu_id; - v->dirty_cpu = VCPU_CPU_CLEAN; + write_atomic(&v->dirty_cpu, VCPU_CPU_CLEAN); spin_lock_init(&v->virq_lock); diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index 87bd145374..68364e987d 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -316,7 +316,7 @@ static void dump_domains(unsigned char key) vcpu_info(v, evtchn_upcall_pending), !vcpu_event_delivery_is_enabled(v)); if ( vcpu_cpu_dirty(v) ) - printk("dirty_cpu=%u", v->dirty_cpu); + printk("dirty_cpu=%u", read_atomic(&v->dirty_cpu)); printk("\n"); printk(" pause_count=%d pause_flags=%lx\n", atomic_read(&v->pause_count), v->pause_flags); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 6101761d25..ac53519d7f 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -844,7 +844,7 @@ static inline bool is_vcpu_dirty_cpu(unsigned int cpu) static inline bool vcpu_cpu_dirty(const struct vcpu *v) { - return is_vcpu_dirty_cpu(v->dirty_cpu); + return is_vcpu_dirty_cpu(read_atomic(&v->dirty_cpu)); } void vcpu_block(void);