Re: [PATCH linux-2.6-block:master 02/05] blk: update ioscheds to use generic dispatch queue

[Jens Axboe]

On Wed, Oct 19 2005, Tejun Heo wrote:
> 02_blk_generic-dispatch-queue-update-for-ioscheds.patch
> 
> 	This patch updates all four ioscheds to use generic dispatch
> 	queue.  There's one behavior change in as-iosched.
> 
> 	* In as-iosched, when force dispatching
> 	  (ELEVATOR_INSERT_BACK), batch_data_dir is reset to REQ_SYNC
> 	  and changed_batch and new_batch are cleared to zero.  This
> 	  prevernts AS from doing incorrect update_write_batch after
> 	  the forced dispatched requests are finished.
> 
> 	* In cfq-iosched, cfqd->rq_in_driver currently counts the
> 	  number of activated (removed) requests to determine
> 	  whether queue-kicking is needed and cfq_max_depth has been
> 	  reached.  With generic dispatch queue, I think counting
> 	  the number of dispatched requests would be more appropriate.
> 
> 	* cfq_max_depth can be lowered to 1 again.

I applied this one as well, with some minor changes. The biggest one is
a cleanup of the 'force' logic, it seems to be a little mixed up in this
patch. You use it for forcing dispatch, which is fine. But then it also
doubles as whether you want to sort insert on the generic queue or just
add to the tail?
  
> -		if (cfq_class_idle(cfqq))
> -			max_dispatch = 1;
> +		if (force)
> +			max_dispatch = INT_MAX;
> +		else
> +			max_dispatch =
> +				cfq_class_idle(cfqq) ? 1 : cfqd->cfq_quantum;

Also, please don't use these ?: constructs, I absolutely hate them as
they are weird to read.

-- 
Jens Axboe

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