Messages in this thread |  | | | Subject | Re: [openib-general] Re: [PATCH 35 of 53] ipath - some interrelated stability and cleanliness fixes | | From | Roland Dreier <> | | Date | Wed, 17 May 2006 21:55:21 -0700 |
| |
Dave> We did discover one possible problem today, which is shared Dave> between our device code and the core openib code, and that's Dave> doing some memory freeing and accounting from a work thread Dave> (updating mm->locked_vm and cleaning up from earlier Dave> get_user_pages); the code in our driver was copied from the Dave> openib core code, it's not literally shared.
Dave> I have a strong suspicion that at least sometimes, it's Dave> executing after the current->mm has gone away. I'm looking Dave> at that more right now.
It doesn't seem likely to me. In uverbs_mem.c, ib_umem_release_on_close() does get_task_mm() and gives up if it can't take a reference to the task's mm. The mmput() doesn't happen until ib_umem_account() runs in the work thread.
I do see obvious bugs in ipath_user_pages.c, though. In ipath_release_user_pages_on_close(), you have:
mm = get_task_mm(current); if (!mm) goto bail; work = kmalloc(sizeof(*work), GFP_KERNEL); if (!work) goto bail_mm; goto bail;
INIT_WORK(&work->work, user_pages_account, work); work->mm = mm; work->num_pages = num_pages;
bail_mm: mmput(mm); bail: return;
So with the "goto bail" you skip the code which does something with the work you allocate, which means that you leak not only the work structure but also the reference to the task's mm that you took.
Even without the "goto bail" the code still wouldn't actually schedule the work, so the work structure would be leaked, although you would do mmput().
I'm not sure what you were trying to do here.c
- R. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |