Geoffrey Lefebvre
2007-May-21 23:23 UTC
[Xen-devel] [PATCH 0 of 2] RFC blktap: Small changes to blktap + added a hook to clear ptes on implicit release of VMAs.
Hi, These patches contain changes to the kernel blktap code. The main change is the addition of a vma hook to clear ptes when linux implicitly releases a vma. This allows the block tap to release grants when a user level process such as tapdisk crashes. This fixes the problem of Xen killing a domain because of implicit grant unmapping. The patches should apply cleanly to changeset 15080. Comments are most welcome. Thanks, geoffrey _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Geoffrey Lefebvre
2007-May-21 23:23 UTC
[Xen-devel] [PATCH 1 of 2] Small changes to blktap: free memory on release, invalidate p2m entry when unmapping grants, check for alloc failure of idx_map, added DONTCOPY flag
# HG changeset patch # User Geoffrey Lefebvre <geoffrey@cs.ubc.ca> # Date 1179788512 25200 # Node ID f5218394705702c1aaa02520be2ba5a9f2648f37 # Parent 9cf9027ec568929946a1ee9094821fdec1eb68ff Small changes to blktap: free memory on release, invalidate p2m entry when unmapping grants, check for alloc failure of idx_map, added DONTCOPY flag. diff -r 9cf9027ec568 -r f52183947057 linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c --- a/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Sat May 19 16:41:48 2007 -0700 +++ b/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Mon May 21 16:01:52 2007 -0700 @@ -477,6 +477,11 @@ static int blktap_open(struct inode *ino info->idx_map = kmalloc(sizeof(unsigned long) * MAX_PENDING_REQS, GFP_KERNEL); + if (info->idx_map == NULL) { + goto fail_nomem; + } + + if (idx > 0) { init_waitqueue_head(&info->wait); for (i = 0; i < MAX_PENDING_REQS; i++) @@ -510,16 +515,24 @@ static int blktap_release(struct inode * zap_page_range( info->vma, info->vma->vm_start, info->vma->vm_end - info->vma->vm_start, NULL); + + kfree(info->vma->vm_private_data); + info->vma = NULL; } - + + if (info->idx_map) { + kfree(info->idx_map); + info->idx_map = NULL; + } + if ( (info->status != CLEANSHUTDOWN) && (info->blkif != NULL) ) { if (info->blkif->xenblkd != NULL) { kthread_stop(info->blkif->xenblkd); info->blkif->xenblkd = NULL; } info->status = CLEANSHUTDOWN; - } + } return 0; } @@ -590,6 +603,7 @@ static int blktap_mmap(struct file *filp vma->vm_private_data = map; vma->vm_flags |= VM_FOREIGN; + vma->vm_flags |= VM_DONTCOPY; info->vma = vma; info->ring_ok = 1; @@ -885,6 +899,10 @@ static void fast_flush_area(pending_req_ idx_to_kaddr(mmap_idx, k_idx, i), GNTMAP_host_map, khandle->kernel); invcount++; + + set_phys_to_machine( + __pa(idx_to_kaddr(mmap_idx, k_idx, i)) + >> PAGE_SHIFT, INVALID_P2M_ENTRY); } if (khandle->user != INVALID_GRANT_HANDLE) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Geoffrey Lefebvre
2007-May-21 23:23 UTC
[Xen-devel] [PATCH 2 of 2] Added a hook for clearing ptes when a vma is freed by the kernel. Adapted from gntdev
# HG changeset patch # User Geoffrey Lefebvre <geoffrey@cs.ubc.ca> # Date 1179788664 25200 # Node ID 236647990e986bca16f150c2fca87c0ed1ebd729 # Parent f5218394705702c1aaa02520be2ba5a9f2648f37 Added a hook for clearing ptes when a vma is freed by the kernel. Adapted from gntdev. diff -r f52183947057 -r 236647990e98 linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c --- a/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Mon May 21 16:01:52 2007 -0700 +++ b/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Mon May 21 16:04:24 2007 -0700 @@ -269,6 +269,16 @@ static inline int GET_NEXT_REQ(unsigned return INVALID_REQ; } +static inline int OFFSET_TO_USR_IDX(int offset) +{ + return offset / BLKIF_MAX_SEGMENTS_PER_REQUEST; +} + +static inline int OFFSET_TO_SEG(int offset) +{ + return offset % BLKIF_MAX_SEGMENTS_PER_REQUEST; +} + #define BLKTAP_INVALID_HANDLE(_g) \ (((_g->kernel) == INVALID_GRANT_HANDLE) && \ @@ -295,8 +305,94 @@ static struct page *blktap_nopage(struct return NOPAGE_SIGBUS; } +static pte_t blktap_clear_pte(struct vm_area_struct *vma, + unsigned long uvaddr, + pte_t *ptep, int is_fullmm) +{ + pte_t copy = *ptep; + tap_blkif_t *info; + int offset, seg, usr_idx, pending_idx, mmap_idx; + unsigned long uvstart = vma->vm_start + (RING_PAGES << PAGE_SHIFT); + unsigned long kvaddr; + struct page **map; + struct page *pg; + struct grant_handle_pair *khandle; + struct gnttab_unmap_grant_ref unmap[2]; + int count = 0; + static int print_warning = 1; + + /*print a warning message once, if the hook gets called*/ + if (print_warning) { + WPRINTK("Clear pte hook called!\n"); + print_warning = 0; + } + + /* If the address is before the start of the grant mapped region + * or if vm_file is NULL (meaning mmap failed and we have nothing to do) + */ + if (uvaddr < uvstart || vma->vm_file == NULL) + return copy; + + info = vma->vm_file->private_data; + map = vma->vm_private_data; + + /* TODO Should these be changed to if statements? */ + BUG_ON(!info); + BUG_ON(!info->idx_map); + BUG_ON(!map); + + offset = (int) ((uvaddr - uvstart) >> PAGE_SHIFT); + usr_idx = OFFSET_TO_USR_IDX(offset); + seg = OFFSET_TO_SEG(offset); + + pending_idx = MASK_PEND_IDX(ID_TO_IDX(info->idx_map[usr_idx])); + mmap_idx = ID_TO_MIDX(info->idx_map[usr_idx]); + + kvaddr = idx_to_kaddr(mmap_idx, pending_idx, seg); + pg = pfn_to_page(__pa(kvaddr) >> PAGE_SHIFT); + ClearPageReserved(pg); + map[offset] = NULL; + + khandle = &pending_handle(mmap_idx, pending_idx, seg); + + if (khandle->kernel != INVALID_GRANT_HANDLE) { + + gnttab_set_unmap_op(&unmap[count], kvaddr, + GNTMAP_host_map, khandle->kernel); + count++; + + set_phys_to_machine(__pa(kvaddr) >> PAGE_SHIFT, + INVALID_P2M_ENTRY); + } + + if (khandle->user != INVALID_GRANT_HANDLE) { + + BUG_ON(xen_feature(XENFEAT_auto_translated_physmap)); + + gnttab_set_unmap_op(&unmap[count], virt_to_machine(ptep), + GNTMAP_host_map + | GNTMAP_application_map + | GNTMAP_contains_pte, + khandle->user); + count++; + } + + if (count) { + int ret; + + BLKTAP_INVALIDATE_HANDLE(khandle); + + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, + unmap, count); + BUG_ON(ret); + } + + return copy; +} + struct vm_operations_struct blktap_vm_ops = { nopage: blktap_nopage, + zap_pte: blktap_clear_pte, }; /****************************************************************** @@ -604,6 +700,10 @@ static int blktap_mmap(struct file *filp vma->vm_private_data = map; vma->vm_flags |= VM_FOREIGN; vma->vm_flags |= VM_DONTCOPY; + +#ifdef CONFIG_X86 + vma->vm_mm->context.has_foreign_mappings = 1; +#endif info->vma = vma; info->ring_ok = 1; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel