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 [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
