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

[Xen-devel] [PATCH 3/4] linux-2.6.18/gntdev: adjust indentation (and fix bugs noticed by doing so)


  • To: "xen-devel" <xen-devel@xxxxxxxxxxxxx>
  • From: "Jan Beulich" <JBeulich@xxxxxxxx>
  • Date: Thu, 10 May 2012 16:18:31 +0100
  • Delivery-date: Thu, 10 May 2012 15:18:19 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>

By inverting two conditions, indentation can be decreased by one level
for large code portions, thus improving legibility.

In gntdev_mmap(), this made obvious that failure of vm_insert_page()
was silently ignored rather than being propagated to the caller.

In gntdev_clear_pte(), the final set_phys_to_machine() must not be
called for the auto-translated case.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/drivers/xen/gntdev/gntdev.c
+++ b/drivers/xen/gntdev/gntdev.c
@@ -607,85 +607,85 @@ static int gntdev_mmap (struct file *fli
                        GNTDEV_INVALID_HANDLE;
 
                /* Now perform the mapping to user space. */
-               if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-
-                       /* NOT USING SHADOW PAGE TABLES. */
-                       /* In this case, we map the grant(s) straight into user
-                        * space.
-                        */
-
-                       /* Get the machine address of the PTE for the user 
-                        *  page.
-                        */
-                       if ((ret = create_lookup_pte_addr(vma->vm_mm, 
-                                                         vma->vm_start 
-                                                         + (i << PAGE_SHIFT), 
-                                                         &ptep)))
-                       {
-                               printk(KERN_ERR "Error obtaining PTE pointer "
-                                      "(%d).\n", ret);
-                               goto undo_map_out;
-                       }
-                       
-                       /* Configure the map operation. */
-               
-                       /* The reference is to be used by host CPUs. */
-                       flags = GNTMAP_host_map;
-                       
-                       /* Specifies a user space mapping. */
-                       flags |= GNTMAP_application_map;
-                       
-                       /* The map request contains the machine address of the
-                        * PTE to update.
-                        */
-                       flags |= GNTMAP_contains_pte;
-                       
-                       if (!(vma->vm_flags & VM_WRITE))
-                               flags |= GNTMAP_readonly;
-
-                       gnttab_set_map_op(&op, ptep, flags, 
-                                         private_data->grants[slot_index+i]
-                                         .u.valid.ref, 
-                                         private_data->grants[slot_index+i]
-                                         .u.valid.domid);
-
-                       /* Carry out the mapping of the grant reference. */
-                       ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
-                                                       &op, 1);
-                       BUG_ON(ret);
-                       if (op.status != GNTST_okay) {
-                               printk(KERN_ERR "Error mapping the grant "
-                                      "reference into user space (%d). domid "
-                                      "= %d; ref = %d\n", op.status,
-                                      private_data->grants[slot_index+i].u
-                                      .valid.domid,
-                                      private_data->grants[slot_index+i].u
-                                      .valid.ref);
-                               /* This should never happen after we've mapped 
into
-                               * the kernel space. */
-                               BUG_ON(op.status == GNTST_eagain);
-                               goto undo_map_out;
-                       }
-                       
-                       /* Record the grant handle, for use in the unmap 
-                        * operation. 
-                        */
-                       private_data->grants[slot_index+i].u.
-                               valid.user_handle = op.handle;
-
-                       /* Update p2m structure with the new mapping. */
-                       set_phys_to_machine(__pa(kernel_vaddr) >> PAGE_SHIFT,
-                                           FOREIGN_FRAME(private_data->
-                                                         grants[slot_index+i]
-                                                         .u.valid.dev_bus_addr
-                                                         >> PAGE_SHIFT));
-               } else {
+               if (xen_feature(XENFEAT_auto_translated_physmap)) {
                        /* USING SHADOW PAGE TABLES. */
                        /* In this case, we simply insert the page into the VM
                         * area. */
                        ret = vm_insert_page(vma, user_vaddr, page);
+                       if (!ret)
+                               continue;
+                       exit_ret = ret;
+                       goto undo_map_out;
+               }
+
+               /* NOT USING SHADOW PAGE TABLES. */
+               /* In this case, we map the grant(s) straight into user
+                * space.
+                */
+
+               /* Get the machine address of the PTE for the user page. */
+               if ((ret = create_lookup_pte_addr(vma->vm_mm,
+                                                 vma->vm_start
+                                                 + (i << PAGE_SHIFT),
+                                                 &ptep)))
+               {
+                       printk(KERN_ERR "Error obtaining PTE pointer (%d)\n",
+                              ret);
+                       goto undo_map_out;
+               }
+
+               /* Configure the map operation. */
+
+               /* The reference is to be used by host CPUs. */
+               flags = GNTMAP_host_map;
+
+               /* Specifies a user space mapping. */
+               flags |= GNTMAP_application_map;
+
+               /* The map request contains the machine address of the
+                * PTE to update.
+                */
+               flags |= GNTMAP_contains_pte;
+
+               if (!(vma->vm_flags & VM_WRITE))
+                       flags |= GNTMAP_readonly;
+
+               gnttab_set_map_op(&op, ptep, flags,
+                                 private_data->grants[slot_index+i]
+                                 .u.valid.ref,
+                                 private_data->grants[slot_index+i]
+                                 .u.valid.domid);
+
+               /* Carry out the mapping of the grant reference. */
+               ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
+                                               &op, 1);
+               BUG_ON(ret);
+               if (op.status != GNTST_okay) {
+                       printk(KERN_ERR "Error mapping the grant reference "
+                              "into user space (%d). domid = %d; ref = %d\n",
+                              op.status,
+                              private_data->grants[slot_index+i].u
+                              .valid.domid,
+                              private_data->grants[slot_index+i].u
+                              .valid.ref);
+                       /* This should never happen after we've mapped into
+                        * the kernel space. */
+                       BUG_ON(op.status == GNTST_eagain);
+                       goto undo_map_out;
                }
 
+               /* Record the grant handle, for use in the unmap
+                * operation.
+                */
+               private_data->grants[slot_index+i].u.valid.user_handle
+                       = op.handle;
+
+               /* Update p2m structure with the new mapping. */
+               set_phys_to_machine(__pa(kernel_vaddr) >> PAGE_SHIFT,
+                                   FOREIGN_FRAME(private_data->
+                                                 grants[slot_index+i]
+                                                 .u.valid.dev_bus_addr
+                                                 >> PAGE_SHIFT));
        }
        exit_ret = 0;
 
@@ -745,55 +745,52 @@ static pte_t gntdev_clear_pte(struct vm_
        /* Only unmap grants if the slot has been mapped. This could be being
         * called from a failing mmap().
         */
-       if (private_data->grants[slot_index].state == GNTDEV_SLOT_MAPPED) {
-
-               /* First, we clear the user space mapping, if it has been made.
-                */
-               if (private_data->grants[slot_index].u.valid.user_handle !=
-                   GNTDEV_INVALID_HANDLE && 
-                   !xen_feature(XENFEAT_auto_translated_physmap)) {
-                       /* NOT USING SHADOW PAGE TABLES. */
-                       gnttab_set_unmap_op(&op, ptep_to_machine(ptep), 
-                                           GNTMAP_contains_pte,
-                                           private_data->grants[slot_index]
-                                           .u.valid.user_handle);
-                       ret = HYPERVISOR_grant_table_op(
-                               GNTTABOP_unmap_grant_ref, &op, 1);
-                       BUG_ON(ret);
-                       if (op.status != GNTST_okay)
-                               printk("User unmap grant status = %d\n", 
-                                      op.status);
-               } else {
-                       /* USING SHADOW PAGE TABLES. */
-                       pte_clear_full(vma->vm_mm, addr, ptep, is_fullmm);
-               }
+       if (private_data->grants[slot_index].state != GNTDEV_SLOT_MAPPED) {
+               pte_clear_full(vma->vm_mm, addr, ptep, is_fullmm);
+               return copy;
+       }
 
-               /* Finally, we unmap the grant from kernel space. */
-               gnttab_set_unmap_op(&op, 
-                                   get_kernel_vaddr(private_data, slot_index),
-                                   GNTMAP_host_map, 
+       /* First, we clear the user space mapping, if it has been made.
+        */
+       if (private_data->grants[slot_index].u.valid.user_handle !=
+           GNTDEV_INVALID_HANDLE &&
+           !xen_feature(XENFEAT_auto_translated_physmap)) {
+               /* NOT USING SHADOW PAGE TABLES. */
+               gnttab_set_unmap_op(&op, ptep_to_machine(ptep),
+                                   GNTMAP_contains_pte,
                                    private_data->grants[slot_index].u.valid
-                                   .kernel_handle);
-               ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, 
+                                   .user_handle);
+               ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
                                                &op, 1);
                BUG_ON(ret);
                if (op.status != GNTST_okay)
-                       printk("Kernel unmap grant status = %d\n", op.status);
+                       printk("User unmap grant status = %d\n", op.status);
+       } else {
+               /* USING SHADOW PAGE TABLES. */
+               pte_clear_full(vma->vm_mm, addr, ptep, is_fullmm);
+       }
 
+       /* Finally, we unmap the grant from kernel space. */
+       gnttab_set_unmap_op(&op,
+                           get_kernel_vaddr(private_data, slot_index),
+                           GNTMAP_host_map,
+                           private_data->grants[slot_index].u.valid
+                           .kernel_handle);
+       ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1);
+       BUG_ON(ret);
+       if (op.status != GNTST_okay)
+               printk("Kernel unmap grant status = %d\n", op.status);
 
-               /* Return slot to the not-yet-mapped state, so that it may be
-                * mapped again, or removed by a subsequent ioctl.
-                */
-               private_data->grants[slot_index].state = 
-                       GNTDEV_SLOT_NOT_YET_MAPPED;
+       /* Return slot to the not-yet-mapped state, so that it may be
+        * mapped again, or removed by a subsequent ioctl.
+        */
+       private_data->grants[slot_index].state = GNTDEV_SLOT_NOT_YET_MAPPED;
 
+       if (!xen_feature(XENFEAT_auto_translated_physmap)) {
                /* Invalidate the physical to machine mapping for this page. */
                set_phys_to_machine(
                        page_to_pfn(private_data->foreign_pages[slot_index]),
                        INVALID_P2M_ENTRY);
-
-       } else {
-               pte_clear_full(vma->vm_mm, addr, ptep, is_fullmm);
        }
 
        return copy;


Attachment: xen-gntdev-indent.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

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