Re: [PATCH] bdi_congestion_funp (was: Re: [PATCH] per process request limits (was Re: IO scheduler, queue depth, nr_requests))

Jens Axboe
2004-02-20T15:00:49Z

On Fri, Feb 20 2004, Joe Thornber wrote:
> > + devices = dm_table_get_devices(t);
> > + for (d = devices->next; d != devices; d = d->next) {
> > + struct dm_dev *dd = list_entry(d, struct dm_dev, list);
> > + request_queue_t *q = bdev_get_queue(dd->bdev);
> > + r |= test_bit(bdi_state, &(q->backing_dev_info.state));
>
> Shouldn't this be calling your bdi_*_congested function rather than
> assuming it is a real device under dm ? (often not true).
>
> I'm also very slightly worried that or'ing together the congestion
> results for all the seperate devices isn't always the right thing.
> These devices include anything that the targets are using, exception
> stores for snapshots, logs for mirror, all paths for multipath (or'ing
> is most likely to be wrong for multipath).

Yeah the patch is pretty much crap in that area, I don't think Miquel
was aiming for inclusion :)

I'd suggest making queue functions for congestion state as well so it
stacks properly.

--
Jens Axboe