[Devel] Re: [PATCH] cfq: async queue allocation per priority

[Vasily Tarasov]

On Wed, 2007-07-18 at 20:51 +0200, Jens Axboe wrote:
> On Wed, Jul 18 2007, Vasily Tarasov wrote:
> > Jens, I think the last patch, that makes queues allocation per priority,
> > has a problem.
> > 
> > If we have two processes with different ioprio_class, but the same
> > ioprio_data, their async requests will fall into the same queue. I guess
> > such behavior is not expected, because it's not right to put real-time
> > requests and best-effort requests in the same queue.
> > 
> > The attached patch fixes the problem by introducing additional *cfqq
> > fields on cfqd, pointing to per-(class,priority) async queues.
> 
> Ugh yes. I'm pretty tempted just to reinstate the cfqq hash again, it
> used to be a clean up but now the it's not stacking up so well.
> 

Hello, Jens,

>From my humble point of view cfqq hash has two problems:

1. It is excess data structure. All needed information can be obtained
from other structures easily, so the presence of hash is a bit
strange... I mean that it's aim is not obvious :)

2. Hash hides from a developer a pretty important concept of CFQ: there
are shared between processes per-priority async queues. I think the code
is the best documentation, so the explicit async cfqq pointers at cfqd
structure reveal this concept greatly.

Summary:

IMHO the hash revival is not very good way. However, this is of course
fully in your competence to choose the right decision! ;)

Thank you,
Vasily