> On Tue, 22 Jan 2008 10:49:16 +0100 Jens Axboe <[EMAIL PROTECTED]> wrote:
> This is where it belongs and then it doesn't take up space for a
> process that doesn't do IO.
>
> ...
>
> struct io_context *get_io_context(gfp_t gfp_flags, int node)
> {
> - struct io_context *ret;
> - ret = current_io_context(gfp_flags, node);
> - if (likely(ret))
> - atomic_inc(&ret->refcount);
> + struct io_context *ret = NULL;
> +
> + do {
> + ret = current_io_context(gfp_flags, node);
> + if (unlikely(!ret))
> + break;
> + } while (!atomic_inc_not_zero(&ret->refcount));
Looks weird. Could do with a comment. Or unweirding ;)
What's going on here?
> return ret;
> }
> EXPORT_SYMBOL(get_io_context);
> diff --git a/fs/ioprio.c b/fs/ioprio.c
> index e4e01bc..a760040 100644
> --- a/fs/ioprio.c
> +++ b/fs/ioprio.c
> @@ -41,18 +41,29 @@ static int set_task_ioprio(struct task_struct *task, int
> ioprio)
> return err;
>
> task_lock(task);
> + do {
> + ioc = task->io_context;
> + /* see wmb() in current_io_context() */
> + smp_read_barrier_depends();
> + if (ioc)
> + break;
>
> - task->ioprio = ioprio;
> -
> - ioc = task->io_context;
> - /* see wmb() in current_io_context() */
> - smp_read_barrier_depends();
> + ioc = alloc_io_context(GFP_ATOMIC, -1);
> + if (!ioc) {
> + err = -ENOMEM;
> + break;
> + }
> + task->io_context = ioc;
> + ioc->task = task;
> + } while (1);
argh. Can't sit there in a loop retrying GFP_ATOMIC!
> - if (ioc)
> + if (!err) {
> + ioc->ioprio = ioprio;
> ioc->ioprio_changed = 1;
> + }
>
> task_unlock(task);
> - return 0;
> + return err;
> }
>
> asmlinkage long sys_ioprio_set(int which, int who, int ioprio)
>
> ...
>
> void put_io_context(struct io_context *ioc);
> void exit_io_context(void);
> struct io_context *get_io_context(gfp_t gfp_flags, int node);
> +struct io_context *alloc_io_context(gfp_t, int);
> void copy_io_context(struct io_context **pdst, struct io_context **psrc);
> void swap_io_context(struct io_context **ioc1, struct io_context **ioc2);
The rest of the declarations around here nicely name their args.
> +static int copy_io(struct task_struct *tsk)
> +{
> + struct io_context *ioc = current->io_context;
> +
> + if (!ioc)
> + return 0;
> +
> + if (ioprio_valid(ioc->ioprio)) {
> + tsk->io_context = alloc_io_context(GFP_KERNEL, -1);
> + if (unlikely(!tsk->io_context))
> + return -ENOMEM;
> +
> + tsk->io_context->task = tsk;
> + tsk->io_context->ioprio = ioc->ioprio;
> + }
> +
> + return 0;
> +}
Should this depend on CONFIG_BLOCK?
--
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/
