On Mon, 17 Apr 2017 14:54:21 +0800 Peter Xu <peterx@xxxxxxxxxx> wrote: > 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 <kwankhede@xxxxxxxxxx> > > Suggested-by: Peter Xu <peterx@xxxxxxxxxx> > > Maybe this suggested-by honor should be for Kirti only? :) Sorry, I mis-attributed this, Eric suggested that vfio_pin_page_external() should have a standard return value. I'll change the Suggested-by. > 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 <peterx@xxxxxxxxxx> Thanks!
