linux-kernel - Re: [PATCH] cfq-iosched: don't allow aliased requests to starve others

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 Jul 2010 12:17:03 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Jeff Moyer <jmoyer@...hat.com>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cfq-iosched: don't allow aliased requests to starve
 others

On Fri, Jul 23, 2010 at 11:07:11AM -0400, Jeff Moyer wrote:

[..]
> >> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> >> index 7982b83..0d8d2cd 100644
> >> --- a/block/cfq-iosched.c
> >> +++ b/block/cfq-iosched.c
> >> @@ -417,6 +417,7 @@ static inline int cfqg_busy_async_queues(struct cfq_data *cfqd,
> >>  }
> >>  
> >>  static void cfq_dispatch_insert(struct request_queue *, struct request *);
> >> +static struct request *cfq_check_fifo(struct cfq_queue *cfqq);
> >>  static struct cfq_queue *cfq_get_queue(struct cfq_data *, bool,
> >>  				       struct io_context *, gfp_t);
> >>  static struct cfq_io_context *cfq_cic_lookup(struct cfq_data *,
> >> @@ -1394,10 +1395,22 @@ static void cfq_add_rq_rb(struct request *rq)
> >>  
> >>  	/*
> >>  	 * looks a little odd, but the first insert might return an alias.
> >> -	 * if that happens, put the alias on the dispatch list
> >> +	 * If that happens, put the alias on the dispatch list, but don't
> >> +	 * allow issuing of aliased requests to starve out the queue.
> >>  	 */
> >> -	while ((__alias = elv_rb_add(&cfqq->sort_list, rq)) != NULL)
> >> +	while ((__alias = elv_rb_add(&cfqq->sort_list, rq)) != NULL) {
> >> +		int fifo_checked = cfq_cfqq_fifo_expire(cfqq);
> >> +		struct request *__rq;
> >> +
> >>  		cfq_dispatch_insert(cfqd->queue, __alias);
> >> +		cfq_clear_cfqq_fifo_expire(cfqq);
> >> +		while ((__rq = cfq_check_fifo(cfqq))) {
> >> +			cfq_dispatch_insert(cfqd->queue, __rq);
> >> +			cfq_clear_cfqq_fifo_expire(cfqq);
> >> +		}
> >> +		if (fifo_checked)
> >> +			cfq_mark_cfqq_fifo_expire(cfqq);
> >> +	}
> >
> > Jeff,
> >
> > IIUC correctly upon receiving an alias, you are checking fifo of same
> > cfqq and not the fifo of other cfqqs which are blocked. So dispatching
> > more expired requests from the cfqq which is generating lots of aliases
> > can at best only worsen the problem. I am wondering how does this patch
> > help in dispatching requests from other cfqqs. 
> 
> I think you're missing a key element, here.  Let's assume some harmless
> process has the I/O scheduler.  Then, your pathological alias generator
> process comes in and inserts a request into the queue.  If that request
> is an alias, then it gets moved to the dispatch list immediately, even
> though the cfqq for that process is not currently being served.
> 
> Now, I'm not sure why we ever get back into select_queue if the request
> queue is always full of these aliases.  I didn't dig down that far, but
> I did verify that progress was made in my simple reproducer.

Jeff, I do understand the problem here. Just that I don't understand
how this patch is going to fix it for CFQ. How this patch will make
sure that requests from other starved queue are put onto dispatch queue.

Also I have been trying to reproduce the issue with 2.6.36-rc6 kernel
and have not been successful so far.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/