On Wed, 2007-04-04 at 10:10 -0700, Linus Torvalds wrote:
> Ok. I'm a bit worried about something like this, this late in the release
> cycle, but since I guess page_test_and_clear_dirty() is always 0 for any
> architecture but S390, I guess there are no possible downsides except for
> that architecture.
Yes, the change can only affect s390 since for all other architectures
page_test_and_clear_dirty is a nop.
> So I'll apply it, but:
>
> > The effect of the two changes is that for every call to
> > clear_page_dirty_for_io a page_test_and_clear_dirty is done. If
> > the per page dirty bit is set set_page_dirty is called. Strangly
> > clear_page_dirty_for_io is called for not-uptodate pages, e.g.
> > over this call-chain:
> >
> > [<000000000007c0f2>] clear_page_dirty_for_io+0x12a/0x130
> > [<000000000007c494>] generic_writepages+0x258/0x3e0
> > [<000000000007c692>] do_writepages+0x76/0x7c
> > [<00000000000c7a26>] __writeback_single_inode+0xba/0x3e4
> > [<00000000000c831a>] sync_sb_inodes+0x23e/0x398
> > [<00000000000c8802>] writeback_inodes+0x12e/0x140
> > [<000000000007b9ee>] wb_kupdate+0xd2/0x178
> > [<000000000007cca2>] pdflush+0x162/0x23c
> >
> > The bad news now is that page_test_and_clear_dirty might claim
> > that a not-uptodate page is dirty since SetPageUptodate which
> > resets the per page dirty bit has not yet been called. The page
> > writeback that follows clobbers the data on disk.
>
> Wouldn't it be best if S390 tried to avoid this by clearing the dirty bit
> whenever a new page is allocated?
We would love to but we cannot. The point is that I/O makes a page
dirty. We could clear the dirty bit on allocation time but the page-in
operation would make it dirty again and we'd have to make it clean AGAIN
in SetPageUptodate. The iske + sske instructions are in the range of
several 100 cycles, so they are quite expensive.
> Anyway, I'll apply the patch, since for 2.6.21 this is clearly the
> simplest solution, but
> (a) I think it might be ugly
> and
> (b) are you sure that it doesn't introduce a new bug on S390, where some
> page has been *removed* from the mappings, and should still trigger
> the "page_test_and_clear_dirty()" test, but now, because it's done
> inside the "if (page_mapped())" case, we miss it?
No, I'm very sure that this won't be the case. The per page dirty bit on
s390 is used as a replacement for the per pte dirty bits. We check a
single time after all the pte operations instead of doing it for every
pte. As long as there is a page_test_and_clear_dirty after the last pte
related to a page has been modified in page_mkclean or removed in
page_remove_rmap we are fine.
--
blue skies, IBM Deutschland Entwicklung GmbH
Martin Vorsitzender des Aufsichtsrats: Johann Weihen
Geschäftsführung: Herbert Kircher
Martin Schwidefsky Sitz der Gesellschaft: Böblingen
Linux on zSeries Registergericht: Amtsgericht Stuttgart,
Development HRB 243294
"Reality continues to ruin my life." - Calvin.
-
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
