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? :) 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! -- Peter Xu
