lkml.org 
[lkml]   [2006]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [openib-general] Re: [PATCH 35 of 53] ipath - some interrelated stability and cleanliness fixes
    From
    Date
        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/

    \
     
     \ /
      Last update: 2006-05-18 06:57    [W:0.037 / U:0.008 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site