On Fri, Feb 13, 2015 at 04:26:00PM +1100, NeilBrown wrote:
> I choose ... Buzz Lightyear !!!
Great choice!
> From: NeilBrown <neilb@xxxxxxx>
> Date: Fri, 13 Feb 2015 15:49:17 +1100
> Subject: [PATCH] sched: prevent recursion in io_schedule()
>
> io_schedule() calls blk_flush_plug() which, depending on the
> contents of current->plug, can initiate arbitrary blk-io requests.
>
> Note that this contrasts with blk_schedule_flush_plug() which requires
> all non-trivial work to be handed off to a separate thread.
>
> This makes it possible for io_schedule() to recurse, and initiating
> block requests could possibly call mempool_alloc() which, in times of
> memory pressure, uses io_schedule().
>
> Apart from any stack usage issues, io_schedule() will not behave
> correctly when called recursively as delayacct_blkio_start() does
> not allow for repeated calls.
Which seems to still be an issue with this patch.
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1f37fe7f77a4..90f3de8bc7ca 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4420,30 +4420,27 @@ EXPORT_SYMBOL_GPL(yield_to);
> */
> void __sched io_schedule(void)
> {
> + io_schedule_timeout(MAX_SCHEDULE_TIMEOUT);
> }
> EXPORT_SYMBOL(io_schedule);
Might as well move it to sched.h as an inline or so..
> long __sched io_schedule_timeout(long timeout)
> {
> + struct rq *rq;
> long ret;
> + int old_iowait = current->in_iowait;
> +
> + current->in_iowait = 1;
> + if (old_iowait)
> + blk_schedule_flush_plug(current);
> + else
> + blk_flush_plug(current);
>
> delayacct_blkio_start();
> + rq = raw_rq();
> atomic_inc(&rq->nr_iowait);
> ret = schedule_timeout(timeout);
> + current->in_iowait = old_iowait;
> atomic_dec(&rq->nr_iowait);
> delayacct_blkio_end();
> return ret;
Like said, that will still recursive call delayacct_blkio_*() and would
increase nr_iowait for a second time; while arguably its still the same
one io-wait instance.
So would a little something like:
long __sched io_schedule_timeout(long timeout)
{
struct rq *rq;
long ret;
/*
* Recursive io_schedule() call; make sure to not recurse
* on the blk_flush_plug() stuff again.
*/
if (unlikely(current->in_iowait)) {
/*
* Our parent io_schedule() call will already have done
* all the required io-wait accounting.
*/
blk_schedule_flush_plug(current);
return schedule_timeout(timeout);
}
current->in_iowait = 1;
delayacct_blkio_start();
rq = raw_rq();
atomic_inc(&rq->nr_iowait);
blk_flush_plug(current);
ret = schedule_timeout(timeout);
atomic_dec(&rq->nr_iowait);
delayacct_blkio_end();
current->in_iowait = 0;
return ret;
}
not make more sense?
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
