[lkml]   [2012]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 6/6] kmod: optional: Convert the use of xchg to a kref
    On 03/26, Boaz Harrosh wrote:
    > if we use a kref instead of xchg to govern the lifetime
    > of struct subprocess_info, the code becomes simpler and
    > more generic. We can avoid some of the special cases.

    Looks correct, and perhaps this makes the code more understandable.
    If we use kref, we have to move the completion into subprocess_info,
    but this allows to remove the "fallback to wait_for_completion()"
    subtleness. Up to maintainer.

    However, I agree with Peter, wait_for_completion_timeout_state()
    helper from 4/6 needs more discussion.

    And the previous looks patch is wrong, see my reply. The bug goes
    away after this patch, but this is not good anyway. IOW, I think
    you should redo 5 and 6 even if the resulting code looks correct.

    Just one nit below.

    > Actually I like the xchg use here. But if it will break
    > some ARCHs that do not like xchg, then here is the work
    > needed.

    Well, xchg() has other arch-neutral users. But again, I am not
    arguing with this change.

    > @@ -302,7 +297,7 @@ static void __call_usermodehelper(struct work_struct *work)
    > switch (wait) {
    > case UMH_NO_WAIT:
    > - call_usermodehelper_freeinfo(sub_info);
    > + kref_put(&sub_info->kref, call_usermodehelper_freeinfo);
    > break;

    This is minor and subjective, but UMH_NO_WAIT could use umh_complete()
    too, the unnecessary complete() doesn't hurt. In this case we could do

    switch (wait) {
    case UMH_WAIT_PROC:
    if (pid > 0)
    case UMH_WAIT_EXEC:
    if (pid < 0)
    sub_info->retval = pid;
    case UMH_NO_WAIT:


     \ /
      Last update: 2012-03-28 18:47    [W:0.021 / U:14.336 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site