From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv4 10/14] xen/gntdev: convert priv->lock to a mutex Date: Tue, 27 Jan 2015 10:00:36 +0000 Message-ID: <54C761C4.1070700@citrix.com> References: <1422291687-7398-1-git-send-email-david.vrabel@citrix.com> <1422291687-7398-11-git-send-email-david.vrabel@citrix.com> <54C692DD.8070704@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YG2wr-0008IM-KO for xen-devel@lists.xenproject.org; Tue, 27 Jan 2015 10:00:41 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , David Vrabel Cc: xen-devel@lists.xenproject.org, Boris Ostrovsky List-Id: xen-devel@lists.xenproject.org On 26/01/15 21:07, Stefano Stabellini wrote: > On Mon, 26 Jan 2015, David Vrabel wrote: >> On 26/01/15 18:57, Stefano Stabellini wrote: >>> >>>> @@ -443,14 +443,14 @@ static void mn_invl_range_start(struct mmu_notifier *mn, >>>> struct gntdev_priv *priv = container_of(mn, struct gntdev_priv, mn); >>>> struct grant_map *map; >>>> >>>> - spin_lock(&priv->lock); >>>> + mutex_lock(&priv->lock); >>>> list_for_each_entry(map, &priv->maps, next) { >>>> unmap_if_in_range(map, start, end); >>>> } >>>> list_for_each_entry(map, &priv->freeable_maps, next) { >>>> unmap_if_in_range(map, start, end); >>>> } >>>> - spin_unlock(&priv->lock); >>>> + mutex_unlock(&priv->lock); >>>> } >>> >>> I don't think that mmu_notifier callbacks are allowed to sleep: >>> >>> https://lkml.org/lkml/2010/1/25/187 >> >> I don't think that limitation exists any more. SRCU is used and you can >> sleep between tlb_gather_mmu()/tlb_finish_mmu(). >> >> Perhaps you could point to something that isn't 5 years old? > > Point taken. > > However the problem is that I couldn't find anything that points in the > other direction either. If you look at include/linux/mmu_notifier.h, it > doesn't state that the callbacks can sleep, except for: > > * The invalidate_range() function is called under the ptl > * spin-lock and not allowed to sleep. > > Therefore maybe we can assume that the others are allowed to sleep, > because there are no comments about it? 1. DEBUG_ATOMIC_SLEEP didn't trigger. 2. The documentation doesn't exclude sleeping (unlike for other ops). 3. Looking at the code I see nothing that would prevent sleeping and plenty of changes to actually allow this. 4. Other drivers (e.g., the i915 driver) sleep. David