[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/3] xen/privcmd: Corrected error handling path

On 07.07.20 13:40, Souptick Joarder wrote:
On Tue, Jul 7, 2020 at 3:05 PM Jürgen Groß <jgross@xxxxxxxx> wrote:

On 06.07.20 20:16, Souptick Joarder wrote:
Previously, if lock_pages() end up partially mapping pages, it used
to return -ERRNO due to which unlock_pages() have to go through
each pages[i] till *nr_pages* to validate them. This can be avoided
by passing correct number of partially mapped pages & -ERRNO separately,
while returning from lock_pages() due to error.

With this fix unlock_pages() doesn't need to validate pages[i] till
*nr_pages* for error scenario and few condition checks can be ignored.

Signed-off-by: Souptick Joarder <jrdr.linux@xxxxxxxxx>
Cc: John Hubbard <jhubbard@xxxxxxxxxx>
Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Cc: Paul Durrant <xadimgnik@xxxxxxxxx>
   drivers/xen/privcmd.c | 31 +++++++++++++++----------------
   1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index a250d11..33677ea 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -580,13 +580,13 @@ static long privcmd_ioctl_mmap_batch(

   static int lock_pages(
       struct privcmd_dm_op_buf kbufs[], unsigned int num,
-     struct page *pages[], unsigned int nr_pages)
+     struct page *pages[], unsigned int nr_pages, unsigned int *pinned)
       unsigned int i;
+     int page_count = 0;

Initial value shouldn't be needed, and ...

       for (i = 0; i < num; i++) {
               unsigned int requested;
-             int pinned;

... you could move the declaration here.

With that done you can add my

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>

Ok. But does it going make any difference other than limiting scope ?

Dropping the initializer surely does, and in the end page_count just
replaces the former pinned variable, so why would we want to widen the
scope with this patch?




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.