From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932930AbdDQGyg (ORCPT ); Mon, 17 Apr 2017 02:54:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37756 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932813AbdDQGyd (ORCPT ); Mon, 17 Apr 2017 02:54:33 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 06796C0567A1 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=peterx@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 06796C0567A1 Date: Mon, 17 Apr 2017 14:54:21 +0800 From: Peter Xu To: Alex Williamson Cc: kvm@vger.kernel.org, eric.auger@redhat.com, kwankhede@nvidia.com, linux-kernel@vger.kernel.org, slp@redhat.com Subject: Re: [PATCH v4 2/2] vfio/type1: Prune vfio_pin_page_external() Message-ID: <20170417065420.GD16703@pxdev.xzpeter.org> References: <20170417014142.25866.16852.stgit@gimli.home> <20170417014239.25866.42333.stgit@gimli.home> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170417014239.25866.42333.stgit@gimli.home> User-Agent: Mutt/1.5.24 (2015-08-30) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 17 Apr 2017 06:54:33 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Apr 16, 2017 at 07:42:39PM -0600, Alex Williamson wrote: > With vfio_lock_acct() testing the locked memory limit under mmap_sem, > it's redundant to do it here for a single page. We can also reorder > our tests such that we can avoid testing for reserved pages if we're > not doing accounting, and test the process CAP_IPC_LOCK only if we > are doing accounting. Finally, this function oddly returns 1 on > success. Update to return zero on success, -errno on error. Since > the function only pins a single page, there's no need to return the > number of pages pinned. > > N.B. vfio_pin_pages_remote() can pin a large contiguous range of pages > before calling vfio_lock_acct(). If we were to similarly remove the > extra test there, a user could temporarily pin far more pages than > they're allowed. > > Suggested-by: Kirti Wankhede > Suggested-by: Peter Xu Maybe this suggested-by honor should be for Kirti only? :) For the patch, I think it's good to me as long as we have the accounting check in vfio_lock_acct() which is just introduced in previous patch, so: Reviewed-by: Peter Xu Thanks! -- Peter Xu