Re: [PATCH 4 of 4] Introduce aio system call submission and completion system calls

[Davide Libenzi <davidel@xmailserver.org>]

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