OpenVZ Forum


Home » Mailing lists » Devel » [PATCH] cfq: async queue allocation per priority
[PATCH] cfq: async queue allocation per priority [message #15089] Wed, 18 July 2007 14:35 Go to next message
Vasily Tarasov is currently offline  Vasily Tarasov
Messages: 1345
Registered: January 2006
Senior Member
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.


Thanks,
Vasily
Re: [PATCH] cfq: async queue allocation per priority [message #15094 is a reply to message #15089] Wed, 18 July 2007 18:51 Go to previous messageGo to next message
Jens Axboe is currently offline  Jens Axboe
Messages: 30
Registered: October 2006
Member
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.

--
Jens Axboe
Re: [PATCH] cfq: async queue allocation per priority [message #15100 is a reply to message #15094] Thu, 19 July 2007 07:52 Go to previous messageGo to next message
Vasily Tarasov is currently offline  Vasily Tarasov
Messages: 1345
Registered: January 2006
Senior Member
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
Re: [PATCH] cfq: async queue allocation per priority [message #15133 is a reply to message #15100] Thu, 19 July 2007 17:30 Go to previous message
Jens Axboe is currently offline  Jens Axboe
Messages: 30
Registered: October 2006
Member
On Thu, Jul 19 2007, Vasily Tarasov wrote:
> 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! ;)

Yeah, it's probably still better off without the hash. I'll play with it
a bit and see what comes of it.

--
Jens Axboe
Previous Topic: [PATCH] Cleanup elevator_ops->trim function
Next Topic: [PATCH] Add kernel/notifier.c
Goto Forum:
  


Current Time: Wed Aug 14 14:27:38 GMT 2024

Total time taken to generate the page: 0.02799 seconds