Commit c146024e authored by Miklos Szeredi's avatar Miklos Szeredi
Browse files

fuse: fix warning in tree_insert() and clean up writepage insertion 



fuse_writepages_fill() calls tree_insert() with ap->num_pages = 0 which
triggers the following warning:

 WARNING: CPU: 1 PID: 17211 at fs/fuse/file.c:1728 tree_insert+0xab/0xc0 [fuse]
 RIP: 0010:tree_insert+0xab/0xc0 [fuse]
 Call Trace:
  fuse_writepages_fill+0x5da/0x6a0 [fuse]
  write_cache_pages+0x171/0x470
  fuse_writepages+0x8a/0x100 [fuse]
  do_writepages+0x43/0xe0

Fix up the warning and clean up the code around rb-tree insertion:

 - Rename tree_insert() to fuse_insert_writeback() and make it return the
   conflicting entry in case of failure

 - Re-add tree_insert() as a wrapper around fuse_insert_writeback()

 - Rename fuse_writepage_in_flight() to fuse_writepage_add() and reverse
   the meaning of the return value to mean

    + "true" in case the writepage entry was successfully added

    + "false" in case it was in-fligt queued on an existing writepage
       entry's auxiliary list or the existing writepage entry's temporary
       page updated

   Switch from fuse_find_writeback() + tree_insert() to
   fuse_insert_writeback()

 - Move setting orig_pages to before inserting/updating the entry; this may
   result in the orig_pages value being discarded later in case of an
   in-flight request

 - In case of a new writepage entry use fuse_writepage_add()
   unconditionally, only set data->wpa if the entry was added.

Fixes: 6b2fb799 ("fuse: optimize writepages search")
Reported-by: default avatarkernel test robot <rong.a.chen@intel.com>
Original-path-by: default avatarVasily Averin <vvs@virtuozzo.com>
Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
parent 69a6487a

fs/fuse/file.c

+30 −32
Original line number Diff line number Diff line
@@ -1674,7 +1674,8 @@ __acquires(fi->lock) 
	}

}



static void tree_insert(struct rb_root *root, struct fuse_writepage_args *wpa)

static struct fuse_writepage_args *fuse_insert_writeback(struct rb_root *root,

						struct fuse_writepage_args *wpa)

{

	pgoff_t idx_from = wpa->ia.write.in.offset >> PAGE_SHIFT;

	pgoff_t idx_to = idx_from + wpa->ia.ap.num_pages - 1;
@@ -1697,11 +1698,17 @@ static void tree_insert(struct rb_root *root, struct fuse_writepage_args *wpa) 
		else if (idx_to < curr_index)

			p = &(*p)->rb_left;

		else

			return (void) WARN_ON(true);

			return curr;

	}



	rb_link_node(&wpa->writepages_entry, parent, p);

	rb_insert_color(&wpa->writepages_entry, root);

	return NULL;

}



static void tree_insert(struct rb_root *root, struct fuse_writepage_args *wpa)

{

	WARN_ON(fuse_insert_writeback(root, wpa));

}



static void fuse_writepage_end(struct fuse_conn *fc, struct fuse_args *args,
@@ -1953,13 +1960,13 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data) 
}



/*

 * First recheck under fi->lock if the offending offset is still under

 * writeback.  If yes, then iterate auxiliary write requests, to see if there's

 * Check under fi->lock if the page is under writeback, and insert it onto the

 * rb_tree if not. Otherwise iterate auxiliary write requests, to see if there's

 * one already added for a page at this offset.  If there's none, then insert

 * this new request onto the auxiliary list, otherwise reuse the existing one by

 * copying the new page contents over to the old temporary page.

 * swapping the new temp page with the old one.

 */

static bool fuse_writepage_in_flight(struct fuse_writepage_args *new_wpa,

static bool fuse_writepage_add(struct fuse_writepage_args *new_wpa,

			       struct page *page)

{

	struct fuse_inode *fi = get_fuse_inode(new_wpa->inode);
@@ -1968,17 +1975,15 @@ static bool fuse_writepage_in_flight(struct fuse_writepage_args *new_wpa, 
	struct fuse_args_pages *new_ap = &new_wpa->ia.ap;



	WARN_ON(new_ap->num_pages != 0);

	new_ap->num_pages = 1;



	spin_lock(&fi->lock);

	rb_erase(&new_wpa->writepages_entry, &fi->writepages);

	old_wpa = fuse_find_writeback(fi, page->index, page->index);

	old_wpa = fuse_insert_writeback(&fi->writepages, new_wpa);

	if (!old_wpa) {

		tree_insert(&fi->writepages, new_wpa);

		spin_unlock(&fi->lock);

		return false;

		return true;

	}



	new_ap->num_pages = 1;

	for (tmp = old_wpa->next; tmp; tmp = tmp->next) {

		pgoff_t curr_index;
@@ -2007,7 +2012,7 @@ static bool fuse_writepage_in_flight(struct fuse_writepage_args *new_wpa, 
		fuse_writepage_free(new_wpa);

	}



	return true;

	return false;

}



static int fuse_writepages_fill(struct page *page,
@@ -2086,12 +2091,6 @@ static int fuse_writepages_fill(struct page *page, 
		ap->args.end = fuse_writepage_end;

		ap->num_pages = 0;

		wpa->inode = inode;



		spin_lock(&fi->lock);

		tree_insert(&fi->writepages, wpa);

		spin_unlock(&fi->lock);



		data->wpa = wpa;

	}

	set_page_writeback(page);
@@ -2099,18 +2098,13 @@ static int fuse_writepages_fill(struct page *page, 
	ap->pages[ap->num_pages] = tmp_page;

	ap->descs[ap->num_pages].offset = 0;

	ap->descs[ap->num_pages].length = PAGE_SIZE;

	data->orig_pages[ap->num_pages] = page;



	inc_wb_stat(&inode_to_bdi(inode)->wb, WB_WRITEBACK);

	inc_node_page_state(tmp_page, NR_WRITEBACK_TEMP);



	err = 0;

	if (is_writeback && fuse_writepage_in_flight(wpa, page)) {

		end_page_writeback(page);

		data->wpa = NULL;

		goto out_unlock;

	}

	data->orig_pages[ap->num_pages] = page;



	if (data->wpa) {

		/*

		 * Protected by fi->lock against concurrent access by

		 * fuse_page_is_writeback().
@@ -2118,7 +2112,11 @@ static int fuse_writepages_fill(struct page *page, 
		spin_lock(&fi->lock);

		ap->num_pages++;

		spin_unlock(&fi->lock);



	} else if (fuse_writepage_add(wpa, page)) {

		data->wpa = wpa;

	} else {

		end_page_writeback(page);

	}

out_unlock:

	unlock_page(page);

CRA Git | Maintained and supported by SUSTech CRA and CCSE