From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: [PATCH 1/6] xen-gntdev: Fix circular locking dependency Date: Wed, 15 Dec 2010 16:28:31 -0800 Message-ID: <4D095D2F.8070002@goop.org> References: <1292338553-20575-1-git-send-email-dgdegra@tycho.nsa.gov> <1292338553-20575-2-git-send-email-dgdegra@tycho.nsa.gov> <4D07DD76.2010900@goop.org> <4D07E444.3020309@tycho.nsa.gov> <1292406442.32368.2613.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1292406442.32368.2613.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Campbell Cc: Daniel De Graaf , xen-devel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 12/15/2010 01:47 AM, Ian Campbell wrote: > On Tue, 2010-12-14 at 21:40 +0000, Daniel De Graaf wrote: >> On 12/14/2010 04:11 PM, Jeremy Fitzhardinge wrote: >>> On 12/14/2010 06:55 AM, Daniel De Graaf wrote: >>>> apply_to_page_range will acquire PTE lock while priv->lock is held, and >>>> mn_invl_range_start tries to acquire priv->lock with PTE already held. >>>> Fix by not holding priv->lock during the entire map operation. >>> Is priv->lock needed to protect the contents of map? >>> >>> J >> No, since the map can only be mapped once (checked by map->vma assignment >> while the lock is held). The unmap ioctl will return -EBUSY until >> an munmap() is called on the area, so it also can't race, and the other >> users are only active once the mmap operation completes. > Sounds reasonable enough to me. There are a few unlocked accesses to > vma->map: > gntdev_del_map (when called from gntdev_ioctl_map_grant_ref) > gntdev_vma_close > are these safe? If so then it would be worth a comment about why. > > Anyway this patch appears to resolve the lockdep warning I was seeing > with 2.6.37 with qemu-xen backed block devices. Good. Stefano should stick this on his patch queue. J