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 <cai@redhat.com> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> Link: https://lore.kernel.org/linux-fsdevel/5ea77e9f6cb8c2db43b09fbd4158ab2d8c066a0a.camel@redhat.com/ Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
This commit is contained in:
parent
413daa1a3f
commit
42d3e2d041
|
@ -1018,18 +1018,37 @@ __releases(fiq->lock)
|
|||
spin_unlock(&fiq->lock);
|
||||
}
|
||||
|
||||
/* Count number of scatter-gather elements required */
|
||||
static unsigned int sg_count_fuse_pages(struct fuse_page_desc *page_descs,
|
||||
unsigned int num_pages,
|
||||
unsigned int total_len)
|
||||
{
|
||||
unsigned int i;
|
||||
unsigned int this_len;
|
||||
|
||||
for (i = 0; i < num_pages && total_len; i++) {
|
||||
this_len = min(page_descs[i].length, total_len);
|
||||
total_len -= this_len;
|
||||
}
|
||||
|
||||
return i;
|
||||
}
|
||||
|
||||
/* Return the number of scatter-gather list elements required */
|
||||
static unsigned int sg_count_fuse_req(struct fuse_req *req)
|
||||
{
|
||||
struct fuse_args *args = req->args;
|
||||
struct fuse_args_pages *ap = container_of(args, typeof(*ap), args);
|
||||
unsigned int total_sgs = 1 /* fuse_in_header */;
|
||||
unsigned int size, total_sgs = 1 /* fuse_in_header */;
|
||||
|
||||
if (args->in_numargs - args->in_pages)
|
||||
total_sgs += 1;
|
||||
|
||||
if (args->in_pages)
|
||||
total_sgs += ap->num_pages;
|
||||
if (args->in_pages) {
|
||||
size = args->in_args[args->in_numargs - 1].size;
|
||||
total_sgs += sg_count_fuse_pages(ap->descs, ap->num_pages,
|
||||
size);
|
||||
}
|
||||
|
||||
if (!test_bit(FR_ISREPLY, &req->flags))
|
||||
return total_sgs;
|
||||
|
@ -1039,8 +1058,11 @@ static unsigned int sg_count_fuse_req(struct fuse_req *req)
|
|||
if (args->out_numargs - args->out_pages)
|
||||
total_sgs += 1;
|
||||
|
||||
if (args->out_pages)
|
||||
total_sgs += ap->num_pages;
|
||||
if (args->out_pages) {
|
||||
size = args->out_args[args->out_numargs - 1].size;
|
||||
total_sgs += sg_count_fuse_pages(ap->descs, ap->num_pages,
|
||||
size);
|
||||
}
|
||||
|
||||
return total_sgs;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue