> By your patch you completely removed locked_vm accounting on deregister
> path. I don't think this was your intention.
Of course. Closing the file should be handled separately from regular
deregistration. A new patch (below) fixes that.
---
Dont touch locked_vm when file is being closed, since a well-behaved
user is either exiting or has freed all mrs already, and a malicious
user is only hurting himself by making locked_vm seem higher than the
actual usage.
Note that current code doesnt seem to handle the later case anyway,
since there's a window after close() returns where locked_vm isnt updated yet.
Signed-off-by: Michael S. Tsirkin <[EMAIL PROTECTED]>
Index: core/uverbs_mem.c
===================================================================
--- core/uverbs_mem.c (revision 2726)
+++ core/uverbs_mem.c (working copy)
@@ -37,13 +37,6 @@
#include "uverbs.h"
-struct ib_umem_account_work {
- struct work_struct work;
- struct mm_struct *mm;
- unsigned long diff;
-};
-
-
static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int
dirty)
{
struct ib_umem_chunk *chunk, *tmp;
@@ -167,15 +160,9 @@ out:
return ret;
}
-static void ib_umem_account(void *work_ptr)
+void ib_umem_put(struct ib_device *dev, struct ib_umem *umem)
{
- struct ib_umem_account_work *work = work_ptr;
-
- down_write(&work->mm->mmap_sem);
- work->mm->locked_vm -= work->diff;
- up_write(&work->mm->mmap_sem);
- mmput(work->mm);
- kfree(work);
+ __ib_umem_release(dev, umem, 1);
}
void ib_umem_release(struct ib_device *dev, struct ib_umem *umem)
@@ -185,35 +172,11 @@ void ib_umem_release(struct ib_device *d
__ib_umem_release(dev, umem, 1);
mm = get_task_mm(current);
- if (mm) {
- /*
- * We may be called with the mm's mmap_sem already
- * held. This can happen when a userspace munmap() is
- * the call that drops the last reference to our file
- * and calls our release method. If there are memory
- * regions to destroy, we'll end up here and not be
- * able to take the mmap_sem.
- *
- * To handle this, we try to grab the mmap_sem, and if
- * we can't get it immediately, we defer the
- * accounting to the system workqueue.
- */
- if (down_write_trylock(&mm->mmap_sem)) {
- mm->locked_vm -= PAGE_ALIGN(umem->length +
umem->offset) >> PAGE_SHIFT;
- up_write(&mm->mmap_sem);
- mmput(mm);
- } else {
- struct ib_umem_account_work *work;
-
- work = kmalloc(sizeof *work, GFP_KERNEL);
- if (!work)
- return;
-
- INIT_WORK(&work->work, ib_umem_account, work);
- work->mm = mm;
- work->diff = PAGE_ALIGN(umem->length + umem->offset) >>
PAGE_SHIFT;
+ if (!mm)
+ return;
- schedule_work(&work->work);
- }
- }
+ down_write(&mm->mmap_sem);
+ mm->locked_vm -= PAGE_ALIGN(umem->length + umem->offset) >> PAGE_SHIFT;
+ up_write(&mm->mmap_sem);
+ mmput(mm);
}
Index: core/uverbs_main.c
===================================================================
--- core/uverbs_main.c (revision 2726)
+++ core/uverbs_main.c (working copy)
@@ -133,7 +133,7 @@ static int ib_dealloc_ucontext(struct ib
struct ib_umem_object *memobj;
memobj = container_of(uobj, struct ib_umem_object, uobject);
- ib_umem_release(mr->device, &memobj->umem);
+ ib_umem_put(mr->device, &memobj->umem);
idr_remove(&ib_uverbs_mr_idr, uobj->id);
ib_dereg_mr(mr);
Index: core/uverbs.h
===================================================================
--- core/uverbs.h (revision 2726)
+++ core/uverbs.h (working copy)
@@ -104,6 +104,7 @@ void ib_uverbs_qp_event_handler(struct i
int ib_umem_get(struct ib_device *dev, struct ib_umem *mem,
void *addr, size_t size, int write);
void ib_umem_release(struct ib_device *dev, struct ib_umem *umem);
+void ib_umem_put(struct ib_device *dev, struct ib_umem *umem);
#define IB_UVERBS_DECLARE_CMD(name) \
ssize_t ib_uverbs_##name(struct ib_uverbs_file *file, \
--
MST
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
