Re: [PATCH 03/16] blkio: Keep queue on service tree until we expire it

Jens Axboe
2009-11-13T18:44:08Z

On Fri, Nov 13 2009, Vivek Goyal wrote:
> On Fri, Nov 13, 2009 at 11:48:08AM +0100, Jens Axboe wrote:
> > On Fri, Nov 13 2009, Corrado Zoccolo wrote:
> > > Hi Vivek,
> > > the following is probably not on a critical path, but maybe it can be
> > > written more efficiently.
> > > Now, it will cicle through all service trees multiple times, to
> > > retrieve the queues for the last one.
> > > What about having a cfq_for_each_queue that takes a function pointer
> > > and will apply it to all of them?
> >
> > I thought the same when reading this. Or perhaps have a small bitmap
> > instead/with the counter to avoid looping around known empty groups.
>
> Hi Jens,
>
> Empty groups will not be on the group service tree. Only exception is
> the group belonging to current active queue where queue might be empty
> but still on service tree because we might be idling on it.
>
> So we don't have to worry about looping through empty groups.

Looks ok, the forced dispatch is less of an issue.

> > I want to start merging the initial prep patches soon, so we can both
> > cut back on the size of your patchset while getting the simpler bits in.
> > Unfortunately I had to stop here, but with a few corrections it can be
> > merged too.
>
> Now we are left with the issue of looping through various empty service
> trees with-in group. We already keep a count of busy queues in each
> service tree. One simple way is to just check for that count and if tree
> is empty, simply return back. I have modified cfq_rb_first() to check for
> count first before it tries to do rb_next().
>
> Please let me know if you think that this is not good enough and we need
> to maintain a bit map of busy service trees in a group.

I think so, thanks!

--
Jens Axboe