On Mon, 5 Mar 2007 13:43:03 -0800 (PST) Linus Torvalds <[EMAIL PROTECTED]> wrote:
> > > On Mon, 5 Mar 2007, Dave Kleikamp wrote: > > > > This fixes a regression caused by 22c8ca78f20724676b6006232bf06cc3e9299539. > > > > nobh_prepare_write() no longer marks the page uptodate, so > > nobh_truncate_page() needs to do it. > > I'm not convinced... > > If the page wasn't up-to-date from before, it's *not* necessarily > up-to-date after the truncate either! So why do we have that at all? The thing about nobh mode is that because we have no buffer_heads, we can't track the uptodateness of sections of the page. Hence nobh pages are basically always uptodate. The only place where we can tolerate partial uptodateness is in between prepare_write and commit_write, where we omit the initialisation of the section of the page which the caller is writing to. Of course, this won't perform very well with 64k pages.. > The same comment is true of "nobh_commit_write()" (which _does_ have the > SetPageUptodate() there). nobh_prepare_write brings uptodate the sections of the page (0->from) and (to->PAGE_CACHE_SIZE), and the nobh_prepare_write() caller brings the (from->to) section uptodate. So the page is uptodate at nobh_commit_write(). It has to be, because we don't know how to bring a non-uptodate nobh page uptodate apart from writing something to every byte in it. > So I have three questions: > > - why is that valid in the first place (the page is *not* guaranteed to > be up-to-date as far as I can see!) > > - why is it valid to do in "nobh_commit_write()" > > - why doesn't "nobh_truncate_page()" > (a) call nobh_prepare_write() through an indirect pointer? > (b) call nobh_commit_write() at all? (Yeah, I realize it's because > of brokenness with i_size, so this is more of a "those > functions should be factored out properly" statement rather > than a question. It's not really appropriate that nobh_truncate_page() call ->prepare_write() at all. But it just happened that nobh_prepare_write() does exactly what nobh_truncate_page() wants to do, so I just called nobh_prepare_write() for code-sharing reasons. Perhaps I should have called nobh_prepare_write() directly, or created some common private function which both nobh_prepare_write() and nobh_truncate_page() internally call. > IOW, I'm sure your patch _fixes_ something, but no, it's certainly not > obvious to me. A few added comments would be good.. Why is it ok to do > this on a page that wasn't up-to-date before (since obviously, if it *was* > up-to-date, it's pointless). Is OK, I think. nobh_prepare_write() brings the outside-from-and-to sections of the page uptodate and memset in nobh_truncate_page() brings the rest of the page uptodate. We bring the to->PAGE_CACHE_SIZE section uptodate twice, which could be optimised. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
