On Wed, 3 Jan 2007 20:44:36 -0800 (PST)
Linus Torvalds <torvalds@xxxxxxxx> wrote:
Actually, I think 2.6.18 may have a subtle variation on it.
In particular, I look back at the try_to_free_buffers() thing that I hated so much, and it makes me wonder.. It used to do:
spin_lock(&mapping->private_lock);
ret = drop_buffers(page, &buffers_to_free);
spin_unlock(&mapping->private_lock);
if (ret) {
.. crappy comment ..
if (test_clear_page_dirty(page))
task_io_account_cancelled_write(PAGE_CACHE_SIZE);
}
and I think that at least on SMP, we had a race with another CPU doing the "mark page dirty if it was dirty in the PTE" at the same time. Because the marking dirty would come in, find no buffers (they just got dropped), and then mark the page dirty (ignoring the lack of any buffers), but then the above would do the "test_clear_page_dirty()" thing on it.
That bug was introduced in 2.6.19, with the dirty page tracking patches.
2.6.18 and earlier used ->private_lock coverage in try_to_free_buffers() to
prevent it.
Ie the race, I think, existed where that crappy comment was.
The comment was complete, accurate and needed.