On Fri, 8 Dec 2000, Linus Torvalds wrote:
> It's really only __block_prepare_write() that can mark the buffers for
> sync writes, and even that case is fairly bogus: it really only wants to
> mark the non-upto-date buffers that it wants to _read_ for sync IO, it
> just "knows" that it is ok to change every buffer it sees. It isn't.
>
> Moving the
>
> bh->b_end_io = end_buffer_io_sync;
>
> into the read-path should be sufficient (hell, we should move the
> "ll_rw_block(READ, 1, &bh)" away, and make it one single read with
>
> ll_rw_block(READ, wait_bh-wait, wait);
>
> at the end of the loop.
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...
> If we do it that way, there is no way the two can clash, because a
> non-up-to-date buffer head won't ever be touched by the write path, so we
> can't get this kind of confusion.
>
> Al? I'd really prefer this alternative instead. Do you see any problems
> with it?
If I understood you right - no problems. I can do such patch, just give
me a couple of hours to recheck the locking rules. BTW, what do you
think about the following:
* add_to_page_cache() is not exported and never used. Kill?
* __lock_page() is called only in lock_page(). Make the latter
inlined wrapper or kill the former?
* rw_swap_page_base() would be simpler if it didn't leave unlocking the
page to caller - right now all callers do
if (!rw_swap_page_base(...))
UnlockPage(page);
* ramfs_writepage() needs UnlockPage()
* udf_adinicb_writepage() doesn't
* brw_page() needs liposuction. _Badly_. Just read it.
* Documentations/filesystems/Locking needs an update.
I can toss any subset of the above into patch - just tell...
> (New rule that makes 100% sense: NOBODY sets "bh->b_end_io" on any buffer
> that he isn't actually going to do IO on. And if there is pending IO on
> the buffer, it had better only be of the same type as the one you're going
> to do).
Hrm. Why not move setting ->b_end_io to ll_rw_block() while we are at it?
Or into ll_rw_block() wrapper...
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/
This archive was generated by hypermail 2b29 : Fri Dec 15 2000 - 21:00:15 EST