Maxim Patlasov <mpatla...@virtuozzo.com> writes:
> The patch fixes another race dealing with fuse_invalidate_files,
> this time when it races with truncate(2):
>
> Thread A: the flusher performs writeback as usual:
>
> fuse_writepages -->
> fuse_send_writepages -->
> end_page_writeback
>
> but before fuse_send_writepages acquires fc->lock and calls
> fuse_flush_writepages,
> some innocent user process re-dirty-es the page.
>
> Thread B: truncate(2) attempts to truncate (shrink) file as usual:
>
> fuse_do_setattr -->
> invalidate_inode_pages2
>
> (This is possible because Thread A has not incremented fi->writectr yet.) But
> invalidate_inode_pages2 finds that re-dirty-ed page and sticks in:
>
> invalidate_inode_pages2 -->
> fuse_launder_page -->
> fuse_writepage_locked -->
> fuse_wait_on_page_writeback
>
> Thread A: the flusher proceeds with fuse_flush_writepages, sends write request
> to userspace fuse daemon, but the daemon is not obliged to fulfill it
> immediately.
> So, thread B waits now for thread A, while thread A waits for userspace.
>
> Now fuse_invalidate_files steps in sticking in filemap_write_and_wait on the
> page locked by Thread B (launder_page always work on a locked page). Deadlock.
>
> The patch fixes deadlock by waking up fuse_writepage_locked after marking
> files with FAIL_IMMEDIATELY flag.
>
> Changed in v2:
> - instead of flagging "fail_immediately", let fuse_writepage_locked return
> fuse_file pointer, then the caller (fuse_launder_page) can use it for
> conditional wait on __fuse_wait_on_page_writeback_or_invalidate. This is
> important because otherwise fuse_invalidate_files may deadlock when
> launder waits for fuse writeback.
ACK-by: dmonak...@openvz.org
>
> Signed-off-by: Maxim Patlasov <mpatla...@virtuozzo.com>
> ---
> fs/fuse/file.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 0ffc806..34e75c2 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1963,7 +1963,8 @@ static struct fuse_file *fuse_write_file(struct
> fuse_conn *fc,
> }
>
> static int fuse_writepage_locked(struct page *page,
> - struct writeback_control *wbc)
> + struct writeback_control *wbc,
> + struct fuse_file **ff_pp)
> {
> struct address_space *mapping = page->mapping;
> struct inode *inode = mapping->host;
> @@ -1971,13 +1972,30 @@ static int fuse_writepage_locked(struct page *page,
> struct fuse_inode *fi = get_fuse_inode(inode);
> struct fuse_req *req;
> struct page *tmp_page;
> + struct fuse_file *ff;
> + int err = 0;
>
> if (fuse_page_is_writeback(inode, page->index)) {
> if (wbc->sync_mode != WB_SYNC_ALL) {
> redirty_page_for_writepage(wbc, page);
> return 0;
> }
> - fuse_wait_on_page_writeback(inode, page->index);
> +
> + /* we can acquire ff here because we do have locked pages here!
> */
> + ff = fuse_write_file(fc, get_fuse_inode(inode));
> + if (!ff)
> + goto dummy_end_page_wb_err;
> +
> + /* FUSE_NOTIFY_INVAL_FILES must be able to wake us up */
> + __fuse_wait_on_page_writeback_or_invalidate(inode, ff,
> page->index);
> +
> + if (test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)) {
> + if (ff_pp)
> + *ff_pp = ff;
> + goto dummy_end_page_wb;
> + }
> +
> + fuse_release_ff(inode, ff);
> }
>
> if (test_set_page_writeback(page))
> @@ -1995,6 +2013,8 @@ static int fuse_writepage_locked(struct page *page,
> req->ff = fuse_write_file(fc, fi);
> if (!req->ff)
> goto err_nofile;
> + if (ff_pp)
> + *ff_pp = fuse_file_get(req->ff);
> fuse_write_fill(req, req->ff, page_offset(page), 0);
> fuse_account_request(fc, PAGE_CACHE_SIZE);
>
> @@ -2029,13 +2049,23 @@ err_free:
> err:
> end_page_writeback(page);
> return -ENOMEM;
> +
> +dummy_end_page_wb_err:
> + printk("FUSE: page under fwb dirtied on dead file\n");
> + err = -EIO;
> + /* fall through ... */
> +dummy_end_page_wb:
> + if (test_set_page_writeback(page))
> + BUG();
> + end_page_writeback(page);
> + return err;
> }
>
> static int fuse_writepage(struct page *page, struct writeback_control *wbc)
> {
> int err;
>
> - err = fuse_writepage_locked(page, wbc);
> + err = fuse_writepage_locked(page, wbc, NULL);
> unlock_page(page);
>
> return err;
> @@ -2423,9 +2453,18 @@ static int fuse_launder_page(struct page *page)
> struct writeback_control wbc = {
> .sync_mode = WB_SYNC_ALL,
> };
> - err = fuse_writepage_locked(page, &wbc);
> - if (!err)
> - fuse_wait_on_page_writeback(inode, page->index);
> + struct fuse_file *ff = NULL;
> + err = fuse_writepage_locked(page, &wbc, &ff);
> + if (!err) {
> + /*
> + * We need to check FAIL_IMMEDIATELY because otherwise
> + * fuse_do_setattr may stick in invalidate_inode_pages2
> + * forever (if fuse_invalidate_files is in progress).
> + */
> + __fuse_wait_on_page_writeback_or_invalidate(inode,
> + ff,
> page->index);
> + fuse_release_ff(inode, ff);
> + }
> }
> return err;
> }
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
