[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] [PVOPS] fix gntdev on PAE
On Fri, 28 May 2010, Jeremy Fitzhardinge wrote: > > I managed to catch a lockdep problem in gntdev, which may be the same as > before: > > BUG: sleeping function called from invalid context at kernel/rwsem.c:21 > in_atomic(): 1, irqs_disabled(): 0, pid: 4091, name: qemu-dm > 2 locks held by qemu-dm/4091: > #0: (&mm->mmap_sem){++++++}, at: [<ffffffff810bb50f>] sys_munmap+0x33/0x58 > #1: (rcu_read_lock){.+.+..}, at: [<ffffffff810cd63a>] > __mmu_notifier_invalidate_range_start+0x0/0xc7 > Pid: 4091, comm: qemu-dm Not tainted 2.6.32.13 #23 > Call Trace: > [<ffffffff8106705b>] ? __debug_show_held_locks+0x22/0x24 > [<ffffffff81039522>] __might_sleep+0x123/0x127 > [<ffffffff810a8536>] ? release_pages+0xd2/0x1e7 > [<ffffffff81498849>] down_read+0x1f/0x57 > [<ffffffff81010142>] ? check_events+0x12/0x20 > [<ffffffff810a8536>] ? release_pages+0xd2/0x1e7 > [<ffffffff810cd63a>] ? __mmu_notifier_invalidate_range_start+0x0/0xc7 > [<ffffffff8123e069>] mn_invl_range_start+0x32/0x118 > [<ffffffff810cd69c>] __mmu_notifier_invalidate_range_start+0x62/0xc7 > [<ffffffff810cd63a>] ? __mmu_notifier_invalidate_range_start+0x0/0xc7 > [<ffffffff810b54bc>] unmap_vmas+0x8c/0x91a > [<ffffffff810ba363>] unmap_region+0xda/0x178 > [<ffffffff810bb472>] do_munmap+0x2ae/0x318 > [<ffffffff810bb51d>] sys_munmap+0x41/0x58 > [<ffffffff81013b82>] system_call_fastpath+0x16/0x1b > > > The problem is that mn_invl_range_start does a down_read(), but it is > called from __mmu_notifier_invalidate_range_start(), which does an > rcu_read_lock, which has the side-effect of disabling preemption. > > The mmu notifier code seems to have always used rcu_read_lock this way, > so I guess this bug has always been there. It's not immediately obvious > how to fix it. > > Thoughts? What about turning the semaphore into a rwlock? Performances shouldn't matter in this case. Something like this: --- diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index ddc59cc..91846b9 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -28,7 +28,7 @@ #include <linux/types.h> #include <linux/uaccess.h> #include <linux/sched.h> -#include <linux/rwsem.h> +#include <linux/spinlock_types.h> #include <xen/xen.h> #include <xen/grant_table.h> @@ -51,7 +51,7 @@ struct gntdev_priv { struct list_head maps; uint32_t used; uint32_t limit; - struct rw_semaphore sem; + rwlock_t rwlock; struct mm_struct *mm; struct mmu_notifier mn; }; @@ -277,7 +277,7 @@ static void mn_invl_range_start(struct mmu_notifier *mn, unsigned long mstart, mend; int err; - down_read(&priv->sem); + read_lock(&priv->rwlock); list_for_each_entry(map, &priv->maps, next) { if (!map->vma) continue; @@ -299,7 +299,7 @@ static void mn_invl_range_start(struct mmu_notifier *mn, (mend - mstart) >> PAGE_SHIFT); WARN_ON(err); } - up_read(&priv->sem); + read_unlock(&priv->rwlock); } static void mn_invl_page(struct mmu_notifier *mn, @@ -316,7 +316,7 @@ static void mn_release(struct mmu_notifier *mn, struct grant_map *map; int err; - down_read(&priv->sem); + read_lock(&priv->rwlock); list_for_each_entry(map, &priv->maps, next) { if (!map->vma) continue; @@ -327,7 +327,7 @@ static void mn_release(struct mmu_notifier *mn, err = unmap_grant_pages(map, 0, map->count); WARN_ON(err); } - up_read(&priv->sem); + read_unlock(&priv->rwlock); } struct mmu_notifier_ops gntdev_mmu_ops = { @@ -347,7 +347,7 @@ static int gntdev_open(struct inode *inode, struct file *flip) return -ENOMEM; INIT_LIST_HEAD(&priv->maps); - init_rwsem(&priv->sem); + priv->rwlock = RW_LOCK_UNLOCKED; priv->limit = limit; priv->mm = get_task_mm(current); @@ -375,13 +375,13 @@ static int gntdev_release(struct inode *inode, struct file *flip) if (debug) printk("%s: priv %p\n", __FUNCTION__, priv); - down_write(&priv->sem); + write_lock(&priv->rwlock); while (!list_empty(&priv->maps)) { map = list_entry(priv->maps.next, struct grant_map, next); err = gntdev_del_map(map); WARN_ON(err); } - up_write(&priv->sem); + write_unlock(&priv->rwlock); mmu_notifier_unregister(&priv->mn, priv->mm); kfree(priv); return 0; @@ -404,7 +404,7 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv *priv, if (unlikely(op.count > priv->limit)) return -EINVAL; - down_write(&priv->sem); + write_lock(&priv->rwlock); err = -ENOMEM; map = gntdev_add_map(priv, op.count); if (!map) @@ -417,13 +417,13 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv *priv, op.index = map->index << PAGE_SHIFT; if (copy_to_user(u, &op, sizeof(op)) != 0) goto err_free; - up_write(&priv->sem); + write_unlock(&priv->rwlock); return 0; err_free: gntdev_del_map(map); err_unlock: - up_write(&priv->sem); + write_unlock(&priv->rwlock); return err; } @@ -440,11 +440,11 @@ static long gntdev_ioctl_unmap_grant_ref(struct gntdev_priv *priv, printk("%s: priv %p, del %d+%d\n", __FUNCTION__, priv, (int)op.index, (int)op.count); - down_write(&priv->sem); + write_lock(&priv->rwlock); map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count); if (map) err = gntdev_del_map(map); - up_write(&priv->sem); + write_unlock(&priv->rwlock); return err; } @@ -460,16 +460,16 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct gntdev_priv *priv, printk("%s: priv %p, offset for vaddr %lx\n", __FUNCTION__, priv, (unsigned long)op.vaddr); - down_read(&priv->sem); + read_lock(&priv->rwlock); map = gntdev_find_map_vaddr(priv, op.vaddr); if (map == NULL || map->vma->vm_start != op.vaddr) { - up_read(&priv->sem); + read_unlock(&priv->rwlock); return -EINVAL; } op.offset = map->index << PAGE_SHIFT; op.count = map->count; - up_read(&priv->sem); + read_unlock(&priv->rwlock); if (copy_to_user(u, &op, sizeof(op)) != 0) return -EFAULT; @@ -488,9 +488,9 @@ static long gntdev_ioctl_set_max_grants(struct gntdev_priv *priv, if (op.count > limit) return -EINVAL; - down_write(&priv->sem); + write_lock(&priv->rwlock); priv->limit = op.count; - up_write(&priv->sem); + write_unlock(&priv->rwlock); return 0; } @@ -538,7 +538,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) printk("%s: map %d+%d at %lx (pgoff %lx)\n", __FUNCTION__, index, count, vma->vm_start, vma->vm_pgoff); - down_read(&priv->sem); + read_lock(&priv->rwlock); map = gntdev_find_map_index(priv, index, count); if (!map) goto unlock_out; @@ -580,7 +580,7 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) map->is_mapped = 1; unlock_out: - up_read(&priv->sem); + read_unlock(&priv->rwlock); return err; } _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |