From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH 1/2] Fix broken IOCTL_PRIVCMD_MMAPBATCH (old version). Date: Fri, 16 Nov 2012 14:46:55 +0000 Message-ID: <50A651DF.8010103@citrix.com> References: <1353062828-5476-1-git-send-email-mats.petersson@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1353062828-5476-1-git-send-email-mats.petersson@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: Mats Petersson Cc: konrad.wilk@oracle.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 16/11/12 10:47, Mats Petersson wrote: > Most code-paths prefer the MMAPBATCH_V2, so this wasn't very obvious > that it broke. The return value is set early on to -EINVAL, and if all > goes well, the "set top bits of the MFN's" never gets called, so the > return value is still EINVAL when the function gets to the end, causing > the caller to think it went wrong (which it didn't!) Better subject line: "xen/privcmd: correctly return success from IOCTL_PRIVCMD_MMAPBATCH." This is a regression introduced by ceb90fa0 (xen/privcmd: add PRIVCMD_MMAPBATCH_V2 ioctl). It broke xentrace as it used xc_map_foreign() instead of xc_map_foreign_bulk(). It would be nice if the commit message mentioned this. > Signed off by: Mats Petersson If the subject/commit message is improved: Acked-by: David Vrabel > --- a/drivers/xen/privcmd.c > +++ b/drivers/xen/privcmd.c > @@ -347,6 +347,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version) > > if (ret) > goto out; > + Stray change, please remove. > if (list_empty(&pagelist)) { > ret = -EINVAL; > goto out; David