lkml.org 
[lkml]   [2008]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Queue upcall locking (was: [dm-devel] [RFC][PATCH] fix dm_any_congested() to properly sync up with suspend code path)


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@redhat.com
>


\
 
 \ /
  Last update: 2008-11-10 15:43    [W:1.120 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site