gntdev: adjust indentation (and fix bugs noticed by doing so) 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 --- 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;