BACKPORT: 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: 6b2fb79963fb ("fuse: optimize writepages search") Reported-by: kernel test robot <rong.a.chen@intel.com> Original-path-by: Vasily Averin <vvs@virtuozzo.com> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> Signed-off-by: UtsavBalar1231 <utsavbalar1231@gmail.com> Change-Id: I2ae5f55cc95ce93a5a6892baca52c9cfd4a85503
This commit is contained in:
parent
ad8bf0915b
commit
9440600419
@ -1697,7 +1697,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_from = wpa->ia.write.in.offset >> PAGE_SHIFT;
|
||||||
pgoff_t idx_to = idx_from + wpa->ia.ap.num_pages - 1;
|
pgoff_t idx_to = idx_from + wpa->ia.ap.num_pages - 1;
|
||||||
@ -1720,11 +1721,17 @@ static void tree_insert(struct rb_root *root, struct fuse_writepage_args *wpa)
|
|||||||
else if (idx_to < curr_index)
|
else if (idx_to < curr_index)
|
||||||
p = &(*p)->rb_left;
|
p = &(*p)->rb_left;
|
||||||
else
|
else
|
||||||
return (void) WARN_ON(true);
|
return curr;
|
||||||
}
|
}
|
||||||
|
|
||||||
rb_link_node(&wpa->writepages_entry, parent, p);
|
rb_link_node(&wpa->writepages_entry, parent, p);
|
||||||
rb_insert_color(&wpa->writepages_entry, root);
|
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,
|
static void fuse_writepage_end(struct fuse_conn *fc, struct fuse_args *args,
|
||||||
@ -1987,14 +1994,14 @@ static void fuse_writepages_send(struct fuse_fill_wb_data *data)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* First recheck under fi->lock if the offending offset is still under
|
* Check under fi->lock if the page is under writeback, and insert it onto the
|
||||||
* writeback. If yes, then iterate auxiliary write requests, to see if there's
|
* 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
|
* 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
|
* 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 page *page)
|
||||||
{
|
{
|
||||||
struct fuse_inode *fi = get_fuse_inode(new_wpa->inode);
|
struct fuse_inode *fi = get_fuse_inode(new_wpa->inode);
|
||||||
struct fuse_writepage_args *tmp;
|
struct fuse_writepage_args *tmp;
|
||||||
@ -2002,17 +2009,15 @@ static bool fuse_writepage_in_flight(struct fuse_writepage_args *new_wpa,
|
|||||||
struct fuse_args_pages *new_ap = &new_wpa->ia.ap;
|
struct fuse_args_pages *new_ap = &new_wpa->ia.ap;
|
||||||
|
|
||||||
WARN_ON(new_ap->num_pages != 0);
|
WARN_ON(new_ap->num_pages != 0);
|
||||||
|
new_ap->num_pages = 1;
|
||||||
|
|
||||||
spin_lock(&fi->lock);
|
spin_lock(&fi->lock);
|
||||||
rb_erase(&new_wpa->writepages_entry, &fi->writepages);
|
old_wpa = fuse_insert_writeback(&fi->writepages, new_wpa);
|
||||||
old_wpa = fuse_find_writeback(fi, page->index, page->index);
|
|
||||||
if (!old_wpa) {
|
if (!old_wpa) {
|
||||||
tree_insert(&fi->writepages, new_wpa);
|
|
||||||
spin_unlock(&fi->lock);
|
spin_unlock(&fi->lock);
|
||||||
return false;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
new_ap->num_pages = 1;
|
|
||||||
for (tmp = old_wpa->next; tmp; tmp = tmp->next) {
|
for (tmp = old_wpa->next; tmp; tmp = tmp->next) {
|
||||||
pgoff_t curr_index;
|
pgoff_t curr_index;
|
||||||
|
|
||||||
@ -2041,7 +2046,7 @@ static bool fuse_writepage_in_flight(struct fuse_writepage_args *new_wpa,
|
|||||||
fuse_writepage_free(new_wpa);
|
fuse_writepage_free(new_wpa);
|
||||||
}
|
}
|
||||||
|
|
||||||
return true;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
static int fuse_writepages_fill(struct page *page,
|
static int fuse_writepages_fill(struct page *page,
|
||||||
@ -2120,12 +2125,6 @@ static int fuse_writepages_fill(struct page *page,
|
|||||||
ap->args.end = fuse_writepage_end;
|
ap->args.end = fuse_writepage_end;
|
||||||
ap->num_pages = 0;
|
ap->num_pages = 0;
|
||||||
wpa->inode = inode;
|
wpa->inode = inode;
|
||||||
|
|
||||||
spin_lock(&fi->lock);
|
|
||||||
tree_insert(&fi->writepages, wpa);
|
|
||||||
spin_unlock(&fi->lock);
|
|
||||||
|
|
||||||
data->wpa = wpa;
|
|
||||||
}
|
}
|
||||||
set_page_writeback(page);
|
set_page_writeback(page);
|
||||||
|
|
||||||
@ -2133,26 +2132,25 @@ static int fuse_writepages_fill(struct page *page,
|
|||||||
ap->pages[ap->num_pages] = tmp_page;
|
ap->pages[ap->num_pages] = tmp_page;
|
||||||
ap->descs[ap->num_pages].offset = 0;
|
ap->descs[ap->num_pages].offset = 0;
|
||||||
ap->descs[ap->num_pages].length = PAGE_SIZE;
|
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_wb_stat(&inode_to_bdi(inode)->wb, WB_WRITEBACK);
|
||||||
inc_node_page_state(tmp_page, NR_WRITEBACK_TEMP);
|
inc_node_page_state(tmp_page, NR_WRITEBACK_TEMP);
|
||||||
|
|
||||||
err = 0;
|
err = 0;
|
||||||
if (is_writeback && fuse_writepage_in_flight(wpa, page)) {
|
if (data->wpa) {
|
||||||
|
/*
|
||||||
|
* Protected by fi->lock against concurrent access by
|
||||||
|
* fuse_page_is_writeback().
|
||||||
|
*/
|
||||||
|
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);
|
end_page_writeback(page);
|
||||||
data->wpa = NULL;
|
|
||||||
goto out_unlock;
|
|
||||||
}
|
}
|
||||||
data->orig_pages[ap->num_pages] = page;
|
|
||||||
|
|
||||||
/*
|
|
||||||
* Protected by fi->lock against concurrent access by
|
|
||||||
* fuse_page_is_writeback().
|
|
||||||
*/
|
|
||||||
spin_lock(&fi->lock);
|
|
||||||
ap->num_pages++;
|
|
||||||
spin_unlock(&fi->lock);
|
|
||||||
|
|
||||||
out_unlock:
|
out_unlock:
|
||||||
unlock_page(page);
|
unlock_page(page);
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user