From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= Subject: Re: [PATCH v2 2/2] xen/m2p: use GNTTABOP_unmap_and_replace to reinstate the original mapping Date: Tue, 23 Jul 2013 12:35:51 +0200 Message-ID: <51EE5C87.90406@citrix.com> References: <1374510529-10395-2-git-send-email-stefano.stabellini@eu.citrix.com> <51ED65B5.1090705@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51ED65B5.1090705@citrix.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: David Vrabel Cc: dcrisan@flexiant.com, xen-devel@lists.xensource.com, Ian.Campbell@citrix.com, alex@alex.org.uk, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 22/07/13 19:02, David Vrabel wrote: > On 22/07/13 17:28, Stefano Stabellini wrote: >> GNTTABOP_unmap_grant_ref unmaps a grant and replaces it with a 0 >> mapping instead of reinstating the original mapping. >> Doing so separately would be racy. >> >> To unmap a grant and reinstate the original mapping atomically we use >> GNTTABOP_unmap_and_replace. >> GNTTABOP_unmap_and_replace doesn't work with GNTMAP_contains_pte, so >> don't use it for kmaps. GNTTABOP_unmap_and_replace zeroes the mapping >> passed in new_addr so we have to reinstate it, however that is a >> per-cpu mapping only used for balloon trade pages, so we can be sure that >> it's not going to be accessed while the mapping is not valid. > > This solves the problem of userspace accessing a disk image on an NFS > mount but what would blkback talking to an iSCSI LUN? Will that need > similar fixes to blkback? This series does not need to fix this now though. I have not played much with iSCSI and blkback, but I guess the same issue exists there, the only difference is that we don't perform the grant mappings with GNTMAP_contains_pte, so less modifications are required. Maybe the unmap and reinstate of the trade page address could be placed somewhere more generic, so that I don't have to also code the multicall in blkback? Adding a proper unmap and replace op that reinstates the trade page address to grant-table.c does seem like the more logical option IMHO. (I can do that after this patch is committed).