lkml.org 
[lkml]   [2008]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/6] ioprio: move io priority from task_struct to io_context
> On Tue, 22 Jan 2008 10:49:16 +0100 Jens Axboe <jens.axboe@oracle.com> 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?




\
 
 \ /
  Last update: 2008-01-23 23:13    [W:0.171 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site