From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 15/15] KVM: Fix racy in kvm_free_assigned_irq Date: Sat, 27 Dec 2008 18:06:26 -0200 Message-ID: <20081227200626.GA4095@amt.cnet> References: <20081225115609.GA10087@syang10-desktop> <1230258607-15208-1-git-send-email-sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm@vger.kernel.org To: Sheng Yang Return-path: Received: from mx2.redhat.com ([66.187.237.31]:41127 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753286AbYL0UGn (ORCPT ); Sat, 27 Dec 2008 15:06:43 -0500 Content-Disposition: inline In-Reply-To: <1230258607-15208-1-git-send-email-sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Dec 26, 2008 at 10:30:07AM +0800, Sheng Yang wrote: > Thanks to Marcelo's observation, The following code have potential issue: > > if (cancel_work_sync(&assigned_dev->interrupt_work)) > kvm_put_kvm(kvm); > > In fact, cancel_work_sync() would return true either work struct is only > scheduled or the callback of work struct is executed. This code only > consider the former situation. Why not simply drop the reference inc / dec from irq handler/work function? Just make sure that there is no queued/executing work left behind on vm shutdown. Don't think an additional reference is necessary. Or am I missing something? > Also, we have a window between cancel_work_sync() and free_irq. This one looks OK.