[linux-lvm] Re: PATCH: bdi_congested-dm.patch (was: Re: IO scheduler, queue depth, nr_requests)

[Andrew Morton]

Miquel van Smoorenburg <miquels at cistron.nl> wrote:
>
> +	down_read(&md->lock);
> +	r = dm_table_any_congested(md->map, bdi_bits);
> +	up_read(&md->lock);

This can deadlock if anyone ever performs a __GFP_IO memory allocation
while holding md->lock.

We could enter page reclaim with the lock held, come back in here and take
it again.  That's an instant deadlock if we were holding it for write and a
super-rare deadlock if we were holding it for read (requires some other
process to come in and run down_write() in between the two down_read()s).

A quick audit shows that we're OK, I think.  What does
dm_table_suspend_targets() do?  But still, this restriction needs to be
understood, documented and adhered to in future DM development.

If it gets really sticky in here it would be acceptable to do
down_read_trylock() and return 0 if it fails - the congestion code is only
advisory and 99% is good enough.


I like these patches btw - I was always worried about what the
stacked-device impementation of queue congestion would look like, and this
is nice and simple.