On Thu, 16 Dec 2010, Henry C Chang wrote:
> > That aside, I have no idea if this patch is the right thing to do. I
> > thought the page bits were mainly important for the page cache... what do
> > we accomplish by marking user pages dirty this way?
>
> Hmm.... It's just because I read this:
> http://www.makelinux.net/ldd3/chp-15-sect-3.shtml
>
> It says:
> "Once your direct I/O operation is complete, you must release the user
> pages. Before doing so, however, you must inform the kernel if you
> changed the contents of those pages. Otherwise, the kernel may think
> that the pages are "clean," meaning that they match a copy found on
> the swap device, and free them without writing them out to backing
> store. So, if you have changed the pages (in response to a user-space
> read request), you must mark each affected page dirty with a call to:
> void SetPageDirty(struct page *page);"
Oh, I see now; I was looking at the wrong implementation of
get_user_pages. I fixed it up to use the put_page_vector helper in the
error path (to put the pages we _did_ get).
For the dirtying, that makes sense now too. The get_user_pages() comment
indicates that set_page_dirty_lock() and put_page() should be used, though
(see mm/memory.c line ~1500).
Pushed a patch that does this. Care to test it out?
git://ceph.newdream.net/git/ceph-client.git master
Thanks!
sage
