On Fri, 20 Feb 2004 16:00:13, Jens Axboe wrote:
> 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.
I've been looking at this. If you want to keep the struct backing_dev_info
reasonably stand-alone (no function pointer and aux data like I did) you
probably need to add a list of parent_queues to every request_list. If
the queue get congested, you mark the parent queue(s) congested as well.
Congested state would need to be changed into a counter instead of a
bitfield, I think.
The pdflush-is-running-on-this-queue bit can probably remain as-is. It's
mostly meant to prevent 2 pdflush daemons from running the same queue.
I don't see much harm in pdflush #1 running on /dev/md0 which consists
of /dev/sda1 and /dev/sdb1 and pdflush #2 running on /dev/sdb2, right ?
Would that be the way to go? If so, I'll take a stab at it.
Mike.
_______________________________________________
linux-lvm mailing list
linux-lvm@redhat.com
https://www.redhat.com/mailman/listinfo/linux-lvm
read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
