Messages in this thread Patch in this message | | | From | Jeff Moyer <> | Subject | Re: Slow file transfer speeds with CFQ IO scheduler in some cases | Date | Tue, 11 Nov 2008 14:36:07 -0500 |
| |
Jens Axboe <jens.axboe@oracle.com> writes:
> OK, that looks better. Can I talk you into just trying this little > patch, just to see what kind of performance that yields? Remove the cfq > patch first. I would have patched nfsd only, but this is just a quick'n > dirty.
I went ahead and gave it a shot. The updated CFQ patch with no I/O context sharing does about 40MB/s reading a 1GB file. Backing that patch out, and then adding the patch to share io_context's between kthreads yields 45MB/s.
By the way, in looking at the copy_io function, I noticed what appears to be a (minor) bug:
if (clone_flags & CLONE_IO) { tsk->io_context = ioc_task_link(ioc); if (unlikely(!tsk->io_context)) return -ENOMEM;
According to comments in ioc_task_link, tsk->io_context == NULL means: /* * if ref count is zero, don't allow sharing (ioc is going away, it's * a race). */
It seems more appropriate to just create a new I/O context at this point, don't you think? (Sorry, I know it's off-topic!)
Cheers,
Jeff
diff --git a/kernel/fork.c b/kernel/fork.c index f608356..483d95c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -723,10 +723,17 @@ static int copy_io(unsigned long clone_flags, struct task_struct *tsk) * Share io context with parent, if CLONE_IO is set */ if (clone_flags & CLONE_IO) { + /* + * If ioc_task_link fails, it just means that we raced + * with io context cleanup. Continue on to allocate + * a new context in this case. + */ tsk->io_context = ioc_task_link(ioc); - if (unlikely(!tsk->io_context)) - return -ENOMEM; - } else if (ioprio_valid(ioc->ioprio)) { + if (likely(tsk->io_context)) + return 0; + } + + if (ioprio_valid(ioc->ioprio)) { tsk->io_context = alloc_io_context(GFP_KERNEL, -1); if (unlikely(!tsk->io_context)) return -ENOMEM;
| |