Skip to content

Commit 42d3e2d

Browse files
rhvgoyalMiklos Szeredi
authored andcommitted
virtiofs: calculate number of scatter-gather elements accurately
virtiofs currently maps various buffers in scatter gather list and it looks at number of pages (ap->pages) and assumes that same number of pages will be used both for input and output (sg_count_fuse_req()), and calculates total number of scatterlist elements accordingly. But looks like this assumption is not valid in all the cases. For example, Cai Qian reported that trinity, triggers warning with virtiofs sometimes. A closer look revealed that if one calls ioctl(fd, 0x5a004000, buf), it will trigger following warning. WARN_ON(out_sgs + in_sgs != total_sgs) In this case, total_sgs = 8, out_sgs=4, in_sgs=3. Number of pages is 2 (ap->pages), but out_sgs are using both the pages but in_sgs are using only one page. In this case, fuse_do_ioctl() sets different size values for input and output. args->in_args[args->in_numargs - 1].size == 6656 args->out_args[args->out_numargs - 1].size == 4096 So current method of calculating how many scatter-gather list elements will be used is not accurate. Make calculations more precise by parsing size and ap->descs. Reported-by: Qian Cai <[email protected]> Signed-off-by: Vivek Goyal <[email protected]> Link: https://lore.kernel.org/linux-fsdevel/[email protected]/ Reviewed-by: Stefan Hajnoczi <[email protected]> Signed-off-by: Miklos Szeredi <[email protected]>
1 parent 413daa1 commit 42d3e2d

File tree

1 file changed

+27
-5
lines changed

1 file changed

+27
-5
lines changed

fs/fuse/virtio_fs.c

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,18 +1018,37 @@ __releases(fiq->lock)
10181018
spin_unlock(&fiq->lock);
10191019
}
10201020

1021+
/* Count number of scatter-gather elements required */
1022+
static unsigned int sg_count_fuse_pages(struct fuse_page_desc *page_descs,
1023+
unsigned int num_pages,
1024+
unsigned int total_len)
1025+
{
1026+
unsigned int i;
1027+
unsigned int this_len;
1028+
1029+
for (i = 0; i < num_pages && total_len; i++) {
1030+
this_len = min(page_descs[i].length, total_len);
1031+
total_len -= this_len;
1032+
}
1033+
1034+
return i;
1035+
}
1036+
10211037
/* Return the number of scatter-gather list elements required */
10221038
static unsigned int sg_count_fuse_req(struct fuse_req *req)
10231039
{
10241040
struct fuse_args *args = req->args;
10251041
struct fuse_args_pages *ap = container_of(args, typeof(*ap), args);
1026-
unsigned int total_sgs = 1 /* fuse_in_header */;
1042+
unsigned int size, total_sgs = 1 /* fuse_in_header */;
10271043

10281044
if (args->in_numargs - args->in_pages)
10291045
total_sgs += 1;
10301046

1031-
if (args->in_pages)
1032-
total_sgs += ap->num_pages;
1047+
if (args->in_pages) {
1048+
size = args->in_args[args->in_numargs - 1].size;
1049+
total_sgs += sg_count_fuse_pages(ap->descs, ap->num_pages,
1050+
size);
1051+
}
10331052

10341053
if (!test_bit(FR_ISREPLY, &req->flags))
10351054
return total_sgs;
@@ -1039,8 +1058,11 @@ static unsigned int sg_count_fuse_req(struct fuse_req *req)
10391058
if (args->out_numargs - args->out_pages)
10401059
total_sgs += 1;
10411060

1042-
if (args->out_pages)
1043-
total_sgs += ap->num_pages;
1061+
if (args->out_pages) {
1062+
size = args->out_args[args->out_numargs - 1].size;
1063+
total_sgs += sg_count_fuse_pages(ap->descs, ap->num_pages,
1064+
size);
1065+
}
10441066

10451067
return total_sgs;
10461068
}

0 commit comments

Comments
 (0)