Re: Queue upcall locking (was: [dm-devel] [RFC][PATCH] fixdm_any_congested() to properly sync up with suspend code path)

[Mikulas Patocka]

On Mon, 10 Nov 2008, Alasdair G Kergon wrote:

> On Mon, Nov 10, 2008 at 08:54:01AM -0500, Christoph Hellwig wrote:
> > On Mon, Nov 10, 2008 at 08:11:51AM -0500, Mikulas Patocka wrote:
> > > For upstream Linux developers: you are holding a spinlock and calling 
> > > bdi*_congested functions that can take indefinite amount of time (there 
> > > are even users reporting having 50 disks in one logical volume or so). I 
> > > think it would be good to move these calls out of spinlocks.
> > Umm, they shouldn't block that long, as that completely defeats their
> > purpose.  
> 
> Indeed - the blocking was a bug for which there's a patch, but that doesn't
> deal with how the function should be operating in the first place.

To me it looks like the bug is more severe --- holding a reference to 
table can happen in other upcalls too and in many other dm places.

I'm considering if we could call the table destructor just at one place 
(that would wait until all references are gone), instead of calling the 
destructor at each dm_table_put point (there are many of these points and 
it's hard to check all of them for correctness).

> - If one device is found to be congested, why bother checking the remaining
> devices?

Yes, that could be improved. But it doesn't solve the O(n) problem.

> - If the device is suspended, the response should be that it is 
> congested, I'd have thought.

Yes, but these congestion upcalls are used only for optimization and the 
device is suspended for so small time, that it doesn't matter if we 
optimize io acces in small moment or not.

Mikulas

> Alasdair
> -- 
> agk@xxxxxxxxxx
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/