Messages in this thread | | | Date | Fri, 8 Dec 2000 17:30:06 -0500 (EST) | From | Alexander Viro <> | Subject | Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7 |
| |
On Fri, 8 Dec 2000, Linus Torvalds wrote:
> > > On Fri, 8 Dec 2000, Alexander Viro wrote: > > > > Erm... So you want to make ->commit_write() page-unlocking? Fine with me, > > but that will make for somewhat bigger patch. Hey, _you_ are in position > > to change the locking rules, freeze or not, so if it's OK with you... > > No. > > Read the code a bit more. > > commit_write() doesn't start any IO at all. It only marks the buffer > dirty.
I'm quite aware of that fact ;-) However, you said
On the other hand, I have this suspicion that there is an even simpler solution: stop using the end_buffer_io_sync version for writes altogether.
If that happens (i.e. if write requests resulting from prepare_write()/ commit_write()/bdflush sequence become async) we must stop unlocking pages after commit_write(). Essentially it would become unlocker of the same kind as readpage() and writepage() - callers must assume that page submitted to commit_write() will eventually be unlocked.
> The dirty flush-out works the way it has always worked. > > (You're right in that the dirty flusher should make sure to change > b_end_io when they do their write-outs - that code used to just depend on > the buffer always having b_end_io magically set)
Hmm... IDGI. Do you want the request resulting from commit_write() ("resulting" != "issued") to stay sync (in that case we still need to change block_write_full_page() since the analysis stands - it dirties blocks unconditionally) or you want them to go async (in that case we need to change commit_write() callers)? Could you clarify?
> Btw, I also think that the dirty buffer flushing should get the page lock. > Right now it touches the buffer list without holding the lock on the page > that the buffer is on, which means that there is really nothign that > prevents it from racing with the page-based writeout. I don't like that: > it does hold the LRU list lock, so the list state itself is ok, but it > does actually touch part of the buffer state that is not really protected > by the lock. > > I think it ends up being ok because ll_rw_block will ignore buffers that > get output twice, and the rest of the state is handled with atomic > operations (b_count and b_flags), but it's not really proper. If > flush_dirty_buffers() got the page lock, everything would be protected > properly. Thoughts?
Umm... I don't think that we need to do it on per-page basis. We need some exclusion, all right, but I'ld rather provide it from block_write_full_page() and its ilk. Hell knows... Cheers, Al
- 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/
| |