Messages in this thread | | | Date | Thu, 29 Aug 2013 16:38:22 +0400 | From | Maxim Patlasov <> | Subject | Re: [PATCH] fuse: fix race in fuse_writepages() |
| |
Hi,
08/29/2013 03:46 PM, Miklos Szeredi пишет: > On Fri, Aug 16, 2013 at 03:51:41PM +0400, Maxim Patlasov wrote: >> The patch is for >> >> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git writepages.v2 >> >> The patch fixes a race between ftruncate(2), mmap-ed write and write(2): >> >> 1) An user makes a page dirty via mmap-ed write. >> 2) The user performs shrinking truncate(2) intended to purge the page. >> 3) Before fuse_do_setattr calls truncate_pagecache, the page goes to >> writeback. fuse_writepages_fill attaches a new page to FUSE_WRITE request, >> then releases the original page by end_page_writeback and unlock it. >> 4) fuse_do_setattr completes and successfully returns. Since now, i_mutex >> is free. >> 5) Ordinary write(2) extends i_size back to cover the page. Note that >> fuse_send_write_pages do wait for fuse writeback, but for another >> page->index. >> 6) fuse_writepages_fill attaches more pages to the request (if any), then >> fuse_writepages_send is eventually called. It is supposed to crop >> inarg->size of the request, but it doesn't because i_size has already been >> extended back. >> >> Moving end_page_writeback behind fuse_writepages_send guarantees that >> __fuse_release_nowrite (called from fuse_do_setattr) will crop inarg->size >> of the request before write(2) gets the chance to extend i_size. > Thanks for the report. Your analysis looks correct. > > Just one nit, why orig_pages? req->pages is already there, so why duplicate it?
req->pages is there, but it is already occupied by new pages (allocated by fuse_writepages_fill). We can't re-use req->pages for original pages because as soon as we put the request to bg_queue (in fuse_writepages_send) and released fc->lock, req->pages may be accessed w/o any delay. So we have two bunches of pointers to "struct page" to be stashed somewhere : original and new one. req->pages is for new pages, orig_pages[] is for original ones.
> Note: you can do __fuse_get_request()/fuse_put_request() to prevent the req from > going away after it's been sent.
Yes, I experimented with this technique before adding orig_pages[]. I was very reluctant about duplicating that page array and was looking for any opportunity to avoid it. Pinning original pages to new ones using page->private looked promising, but unfortunately it didn't work because __fuse_get_request() protects only request itself from disappearing, not from releasing pages that req->pages[] points to. And obviously, as soon as a page released, it's not correct to rely on the content of its 'private' field.
Thanks, Maxim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |