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
