|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 4/4] linux-2.6.18/gntdev: misc cleanup
- eliminate struct gntdev_grant_info's dev_bus_addr member (which was
used to communicate a value inside the main loop of gntdev_mmap())
- correct types
- use a local variable 'grants' in gntdev_mmap() to improve readability
- avoid re-calculating already calculated values
- properly check for out of range values
- combine both GNTTABOP_unmap_grant_ref calls in gntdev_clear_pte()
into a single hypercall
- adjust error diagnostic in unmap ioctl to be more useful and better
legible
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
--- a/drivers/xen/gntdev/gntdev.c
+++ b/drivers/xen/gntdev/gntdev.c
@@ -80,7 +80,6 @@ typedef struct gntdev_grant_info {
grant_ref_t ref;
grant_handle_t kernel_handle;
grant_handle_t user_handle;
- uint64_t dev_bus_addr;
} valid;
} u;
} gntdev_grant_info_t;
@@ -473,14 +472,14 @@ static int gntdev_mmap (struct file *fli
{
struct gnttab_map_grant_ref op;
unsigned long slot_index = vma->vm_pgoff;
- unsigned long kernel_vaddr, user_vaddr;
- uint32_t size = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ unsigned long kernel_vaddr, user_vaddr, mfn;
+ unsigned long size = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
uint64_t ptep;
int ret, exit_ret;
- int flags;
- int i;
+ unsigned int i, flags;
struct page *page;
gntdev_file_private_data_t *private_data = flip->private_data;
+ gntdev_grant_info_t *grants;
struct vm_foreign_map *foreign_map;
if (unlikely(!private_data)) {
@@ -490,17 +489,19 @@ static int gntdev_mmap (struct file *fli
/* Test to make sure that the grants array has been initialised. */
down_read(&private_data->grants_sem);
- if (unlikely(!private_data->grants)) {
- up_read(&private_data->grants_sem);
+ grants = private_data->grants;
+ up_read(&private_data->grants_sem);
+
+ if (unlikely(!grants)) {
printk(KERN_ERR "Attempted to mmap before ioctl.\n");
return -EINVAL;
}
- up_read(&private_data->grants_sem);
+ grants += slot_index;
- if (unlikely((size <= 0) ||
- (size + slot_index) > private_data->grants_size)) {
+ if (unlikely(size + slot_index <= slot_index ||
+ size + slot_index > private_data->grants_size)) {
printk(KERN_ERR "Invalid number of pages or offset"
- "(num_pages = %d, first_slot = %ld).\n",
+ "(num_pages = %lu, first_slot = %lu)\n",
size, slot_index);
return -ENXIO;
}
@@ -521,11 +522,10 @@ static int gntdev_mmap (struct file *fli
/* Slots must be in the NOT_YET_MAPPED state. */
down_write(&private_data->grants_sem);
for (i = 0; i < size; ++i) {
- if (private_data->grants[slot_index + i].state !=
- GNTDEV_SLOT_NOT_YET_MAPPED) {
- printk(KERN_ERR "Slot (index = %ld) is in the wrong "
+ if (grants[i].state != GNTDEV_SLOT_NOT_YET_MAPPED) {
+ printk(KERN_ERR "Slot (index = %lu) is in the wrong "
"state (%d).\n", slot_index + i,
- private_data->grants[slot_index + i].state);
+ grants[i].state);
up_write(&private_data->grants_sem);
kfree(foreign_map);
return -EINVAL;
@@ -565,14 +565,11 @@ static int gntdev_mmap (struct file *fli
flags |= GNTMAP_readonly;
kernel_vaddr = get_kernel_vaddr(private_data, slot_index + i);
- user_vaddr = get_user_vaddr(vma, i);
page = private_data->foreign_pages[slot_index + i];
gnttab_set_map_op(&op, kernel_vaddr, flags,
- private_data->grants[slot_index+i]
- .u.valid.ref,
- private_data->grants[slot_index+i]
- .u.valid.domid);
+ grants[i].u.valid.ref,
+ grants[i].u.valid.domid);
/* Carry out the mapping of the grant reference. */
ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
@@ -583,10 +580,8 @@ static int gntdev_mmap (struct file *fli
printk(KERN_ERR "Error mapping the grant
reference "
"into the kernel (%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);
+ grants[i].u.valid.domid,
+ grants[i].u.valid.ref);
else
/* Propagate eagain instead of trying to fix it
up */
exit_ret = -EAGAIN;
@@ -597,16 +592,14 @@ static int gntdev_mmap (struct file *fli
SetPageReserved(page);
/* Record the grant handle, for use in the unmap operation. */
- private_data->grants[slot_index+i].u.valid.kernel_handle =
- op.handle;
- private_data->grants[slot_index+i].u.valid.dev_bus_addr =
- op.dev_bus_addr;
+ grants[i].u.valid.kernel_handle = op.handle;
- private_data->grants[slot_index+i].state = GNTDEV_SLOT_MAPPED;
- private_data->grants[slot_index+i].u.valid.user_handle =
- GNTDEV_INVALID_HANDLE;
+ grants[i].state = GNTDEV_SLOT_MAPPED;
+ grants[i].u.valid.user_handle = GNTDEV_INVALID_HANDLE;
/* Now perform the mapping to user space. */
+ user_vaddr = get_user_vaddr(vma, i);
+
if (xen_feature(XENFEAT_auto_translated_physmap)) {
/* USING SHADOW PAGE TABLES. */
/* In this case, we simply insert the page into the VM
@@ -622,11 +615,11 @@ static int gntdev_mmap (struct file *fli
/* In this case, we map the grant(s) straight into user
* space.
*/
+ mfn = op.dev_bus_addr >> PAGE_SHIFT;
/* 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),
+ user_vaddr,
&ptep)))
{
printk(KERN_ERR "Error obtaining PTE pointer (%d)\n",
@@ -636,9 +629,6 @@ static int gntdev_mmap (struct file *fli
/* 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;
@@ -647,14 +637,9 @@ static int gntdev_mmap (struct file *fli
*/
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);
+ grants[i].u.valid.ref,
+ grants[i].u.valid.domid);
/* Carry out the mapping of the grant reference. */
ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
@@ -664,10 +649,8 @@ static int gntdev_mmap (struct file *fli
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);
+ grants[i].u.valid.domid,
+ grants[i].u.valid.ref);
/* This should never happen after we've mapped into
* the kernel space. */
BUG_ON(op.status == GNTST_eagain);
@@ -677,15 +660,11 @@ static int gntdev_mmap (struct file *fli
/* Record the grant handle, for use in the unmap
* operation.
*/
- private_data->grants[slot_index+i].u.valid.user_handle
- = op.handle;
+ grants[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));
+ FOREIGN_FRAME(mfn));
}
exit_ret = 0;
@@ -715,9 +694,11 @@ undo_map_out:
static pte_t gntdev_clear_pte(struct vm_area_struct *vma, unsigned long addr,
pte_t *ptep, int is_fullmm)
{
- int slot_index, ret;
+ int ret;
+ unsigned int nr;
+ unsigned long slot_index;
pte_t copy;
- struct gnttab_unmap_grant_ref op;
+ struct gnttab_unmap_grant_ref op[2];
gntdev_file_private_data_t *private_data;
/* THIS IS VERY UNPLEASANT: do_mmap_pgoff() will set the vma->vm_file
@@ -750,36 +731,35 @@ static pte_t gntdev_clear_pte(struct vm_
return copy;
}
- /* First, we clear the user space mapping, if it has been made.
- */
+ /* 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),
+ GNTDEV_INVALID_HANDLE) {
+ /* NOT USING SHADOW PAGE TABLES (and user handle valid). */
+ gnttab_set_unmap_op(&op[0], 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);
+ nr = 1;
} else {
- /* USING SHADOW PAGE TABLES. */
+ /* USING SHADOW PAGE TABLES (or user handle invalid). */
pte_clear_full(vma->vm_mm, addr, ptep, is_fullmm);
+ nr = 0;
}
- /* Finally, we unmap the grant from kernel space. */
- gnttab_set_unmap_op(&op,
+ /* We always unmap the grant from kernel space. */
+ gnttab_set_unmap_op(&op[nr],
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);
+
+ ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, op, nr + 1);
BUG_ON(ret);
- if (op.status != GNTST_okay)
- printk("Kernel unmap grant status = %d\n", op.status);
+
+ if (nr && op[0].status != GNTST_okay)
+ printk("User unmap grant status = %d\n", op[0].status);
+ if (op[nr].status != GNTST_okay)
+ printk("Kernel unmap grant status = %d\n", op[nr].status);
/* Return slot to the not-yet-mapped state, so that it may be
* mapped again, or removed by a subsequent ioctl.
@@ -915,7 +895,8 @@ private_data_initialised:
return -EFAULT;
start_index = op.index >> PAGE_SHIFT;
- if (start_index + op.count > private_data->grants_size)
+ if (start_index + op.count < start_index ||
+ start_index + op.count > private_data->grants_size)
return -EINVAL;
down_write(&private_data->grants_sem);
@@ -924,28 +905,29 @@ private_data_initialised:
* state.
*/
for (i = 0; i < op.count; ++i) {
- if (unlikely
- (private_data->grants[start_index + i].state
- != GNTDEV_SLOT_NOT_YET_MAPPED)) {
- if (private_data->grants[start_index + i].state
- == GNTDEV_SLOT_INVALID) {
- printk(KERN_ERR
- "Tried to remove an invalid "
- "grant at offset 0x%x.",
- (start_index + i)
- << PAGE_SHIFT);
- rc = -EINVAL;
- } else {
- printk(KERN_ERR
- "Tried to remove a grant which "
- "is currently mmap()-ed at "
- "offset 0x%x.",
- (start_index + i)
- << PAGE_SHIFT);
- rc = -EBUSY;
- }
- goto unmap_out;
+ const char *what;
+
+ switch (private_data->grants[start_index + i].state) {
+ case GNTDEV_SLOT_NOT_YET_MAPPED:
+ continue;
+ case GNTDEV_SLOT_INVALID:
+ what = "invalid";
+ rc = -EINVAL;
+ break;
+ case GNTDEV_SLOT_MAPPED:
+ what = "currently mmap()-ed";
+ rc = -EBUSY;
+ break;
+ default:
+ what = "in an invalid state";
+ rc = -ENXIO;
+ break;
}
+ printk(KERN_ERR "%s[%d] tried to remove a grant"
+ " which is %s at %#x+%#x\n",
+ current->comm, current->pid,
+ what, start_index, i);
+ goto unmap_out;
}
down_write(&private_data->free_list_sem);
Attachment:
xen-gntdev-cleanup.patch _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |