Re: [openib-general] Re: [PATCH 35 of 53] ipath - some interrelated stability and cleanliness fixes

From: Roland Dreier
Date: Thu May 18 2006 - 00:54:48 EST

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;


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

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@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at