On Tue, 30 Jan 2007, Zach Brown wrote:

> +void asys_task_exiting(struct task_struct *tsk)
> +{
> +     struct asys_result *res, *next;
> +
> +     list_for_each_entry_safe(res, next, &tsk->asys_completed, item)
> +             kfree(res);
> +
> +     /* 
> +      * XXX this only works if tsk->fibril was allocated by
> +      * sys_asys_submit(), not if its embedded in an asys_call.  This
> +      * implies that we must forbid sys_exit in asys_submit.
> +      */
> +     if (tsk->fibril) {
> +             BUG_ON(!list_empty(&tsk->fibril->run_list));
> +             kfree(tsk->fibril);
> +             tsk->fibril = NULL;
> +     }
> +}

What happens to lingering fibrils? Better keep track of both runnable and 
sleepers, and do proper cleanup.



> +asmlinkage long sys_asys_submit(struct asys_input __user *user_inp,
> +                             unsigned long nr_inp)
> +{
> +     struct asys_input inp;
> +     struct asys_result *res;
> +     struct asys_call *call;
> +     struct thread_info *ti;
> +     unsigned long i;
> +     long err = 0;
> +
> +     /* Allocate a fibril for the submitter's thread_info */
> +     if (current->fibril == NULL) {
> +             current->fibril = kzalloc(sizeof(struct fibril), GFP_KERNEL);
> +             if (current->fibril == NULL)
> +                     return -ENOMEM;
> +
> +             INIT_LIST_HEAD(&current->fibril->run_list);
> +             current->fibril->state = TASK_RUNNING;
> +             current->fibril->ti = current_thread_info();
> +     }

Why do we need the "special" submission fibril?




> +     for (i = 0; i < nr_inp; i++) {
> +
> +             if (copy_from_user(&inp, &user_inp[i], sizeof(inp))) {
> +                     err = -EFAULT;
> +                     break;
> +             }
> +
> +             res = kmalloc(sizeof(struct asys_result), GFP_KERNEL);
> +             if (res == NULL) {
> +                     err = -ENOMEM;
> +                     break;
> +             }
> +
> +             /* XXX kzalloc to init call.fibril.per_cpu, add helper */
> +             call = kzalloc(sizeof(struct asys_call), GFP_KERNEL);
> +             if (call == NULL) {
> +                     kfree(res);
> +                     err = -ENOMEM;
> +                     break;
> +             }
> +
> +             ti = alloc_thread_info(tsk);
> +             if (ti == NULL) {
> +                     kfree(res);
> +                     kfree(call);
> +                     err = -ENOMEM;
> +                     break;
> +             }
> +
> +             err = asys_init_fibril(&call->fibril, ti, &inp);
> +             if (err) {
> +                     kfree(res);
> +                     kfree(call);
> +                     free_thread_info(ti);
> +                     break;
> +             }
> +
> +             res->comp.cookie = inp.cookie;
> +             call->result = res;
> +             ti->task = current;
> +
> +             sched_new_runnable_fibril(&call->fibril);
> +             schedule();
> +     }
> +
> +     return i ? i : err;
> +}

Streamline error path (kfree(NULL) is OK):

        err =  -ENOMEM;
        a = alloc();
        b = alloc();
        c = alloc();
        if (!a || !b || !c)
                goto error;
        ...
error:
        kfree(c);
        kfree(b);
        kfree(a);
        return err;


- Davide


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to