Cc: Andrew Morton <akpm@...>, Christian Borntraeger <borntraeger@...>, <linux-mm@...>, <linux-kernel@...>, Martin Schwidefsky <schwidefsky@...>, Theodore Ts'o <tytso@...>
So I just took a little bit of time to look at and think about
the path you are referring to, and I don't see a problem.
The rule with the buffer dirty bit is that you first clear it
and then you submit the write. When the write request finally
makes it's way to rd.c we copy the data if necessary and call
set_page_dirty. Which will then mark the page and the buffer
dirty again.
In essence the ramdisk code just attempts to lock buffers in
ram by setting their dirty bit, just like we do for pages
in ramfs.
The only case where I see the dirty bit getting cleared without
submitting I/O is when dirty state doesn't matter, in which
case if we get a page full buffers all of whose data we don't care
about it is legitimate to drop the page.
As for ramdisk_writepage it probably makes sense to remove it,
as the generic code paths seem to work as well or better the
writepage method is NULL. However if we do keep it we should call
set_page_dirty there on general principles.
I thought through the logic in try_to_free_buffers and it actually
makes sense to me now. mark_buffer_dirty sets the page dirty bit
so dirty buffers reside on dirty pages. When we submit I/O we
aren't guaranteed to submit all of the dirty buffers for a page
at once, so we don't clear the page dirty bit. With the result
that we can end up with pages with the dirty bit set but all of
their buffers are clean.
Since we rather deliberately allow truly clean pages to be dropped
from the ramdisk overriding try_to_free_buffer_pages looks wrong
because then except for invalidate we can not remove buffers
from truly clean pages.
I do agree that with the amount of code duplication in the buffer
cache that locking pages into the buffer cache seems very error prone,
and difficult to maintain. So rewriting rd.c to store it's pages
elsewhere sounds very promising.
While I can see Christian's patch as fixing the symptom. I have
a very hard time see it as correct. If we did something more
elaborate to replace try_to_free_buffer_pages such that
we could drop buffers from clean buffer cache pages when they
became such and so were only suppressing the drop the dirty
bit logic for pages that contain data we want to keep I would be ok
with it. Just totally skipping buffer head freeing just feels
wrong to me.
Eric
-
