Messages in this thread | | | Date | Thu, 28 Dec 2000 14:25:56 -0200 (BRST) | From | Marcelo Tosatti <> | Subject | Re: [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/
| |