From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv11 3/4] gnttab: make the grant table lock a read-write lock Date: Fri, 5 Jun 2015 15:39:12 +0100 Message-ID: <5571B490.9080703@citrix.com> References: <1433262381-8965-1-git-send-email-david.vrabel@citrix.com> <1433262381-8965-4-git-send-email-david.vrabel@citrix.com> <5571CFA20200007800081714@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z0smG-00011l-75 for xen-devel@lists.xenproject.org; Fri, 05 Jun 2015 14:39:20 +0000 In-Reply-To: <5571CFA20200007800081714@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Malcolm Crossley , Keir Fraser , Ian Campbell , Tim Deegan List-Id: xen-devel@lists.xenproject.org On 05/06/15 15:34, Jan Beulich wrote: >>>> On 02.06.15 at 18:26, wrote: >> @@ -970,9 +988,10 @@ __gnttab_unmap_common( >> TRACE_1D(TRC_MEM_PAGE_GRANT_UNMAP, dom); >> >> rgt = rd->grant_table; >> - double_gt_lock(lgt, rgt); >> >> - op->flags = op->map->flags; >> + read_lock(&rgt->lock); >> + >> + op->flags = read_atomic(&op->map->flags); >> if ( unlikely(!op->flags) || unlikely(op->map->domid != dom) ) >> { >> gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle); >> @@ -1019,31 +1038,34 @@ __gnttab_unmap_common( >> act->pin -= GNTPIN_hstw_inc; >> } >> >> - if ( gnttab_need_iommu_mapping(ld) ) >> + act_release_out: >> + active_entry_release(act); >> + unmap_out: >> + read_unlock(&rgt->lock); > > Trying to answer the question on what protects the maptrack > entries: With the flags check done first and, after initial setup, the > ref field never changing, all modifications of flags as well as the > decision whether to drop the maptrack handle appear to be guarded > by ref's active entry lock. I think this is implicit enough to require > being properly spelled out somewhere. > > Together with struct grant_mapping not being used elsewhere (I > just now created a patch to make this more explicit by moving its > declaration to the C file) I think this addresses that particular > concern. If you agree, feel free to add Yes, this was exactly my reasoning. I shall improve the docs/comments to make this clearer in v12. > Reviewed-by: Jan Beulich Thanks! David