lkml.org 
[lkml]   [2000]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] not sleep while holding a locked page in block_truncate_page


On Thu, 28 Dec 2000, Linus Torvalds wrote:

>
>
> On Thu, 28 Dec 2000, Marcelo Tosatti wrote:
> >
> > Hi Linus,
> >
> > block_truncate_page() function unecessarily calls mark_buffer_dirty(),
> > which may wait on bdflush, while holding a locked page.
>
> Good catch. It should be ok to sleep for bdflush while holding the page,
> but at the same time it's certainly preferable _not_ to do that.
>
> bdflush should not need any locks that we hold, so it shouldn't have
> deadlocked. How did you find this? Just reading the source, or have you
> seen any real problems?

Just reading the code.

> If the latter, maybe there's something that tries to get a FS lock
> when it shouldn't?

No, its not a deadlock. Its just a potential performance
problem.

Actually, ext2 is full of calls to mark_buffer_dirty() while holding the
superblock lock. Juan Quintela (which is being CC'ed) has a patch to
minimize the contention of the superblock lock by calling balance_dirty()
only after the sb lock gets unlocked all over ext2. Would you accept that
patch for 2.4?

Moreover, it seems mark_buffer_dirty does not makes a lot of sense wrt
balance_dirty:

---

/* atomic version, the user must call balance_dirty() by hand
as soon as it become possible to block */
void __mark_buffer_dirty(struct buffer_head *bh)
{
if (!atomic_set_buffer_dirty(bh))
__mark_dirty(bh);
}

void mark_buffer_dirty(struct buffer_head *bh)
{
__mark_buffer_dirty(bh);
balance_dirty(bh->b_dev);
}

---

If we call mark_buffer_dirty() on an already dirty buffer, we may sleep
waiting for bdflush even if we haven't caused _any_ real disk IO (because
the buffer was already dirty anyway).

I think it makes more sense if we only call balance_dirty if we actually
caused real disk IO.

Would you accept a patch to change that situation by making
__mark_buffer_dirty return the old dirty bit value and make
mark_buffer_dirty only sleep on bdflush if we dirtied a clean buffer?


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

\
 
 \ /
  Last update: 2005-03-22 12:52    [W:0.041 / U:3.292 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site