<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>Hi Lynch,</p>
<p>thank you very much for your reply and for willingness of making
the global open source better. :)</p>
<p>From your replay i clearly understand that you have tried to
increase the number of segments<br>
and that has boosted the performance and that it did not work
properly before your fix.</p>
<p>But why this particular patch fixes the problem?</p>
<p>What was the exact root cause of the problem? (not high level
"front-end OS did not boot", but on the low level - what caused
the problem?)</p>
<p>As the patch effectively preserves the <span style="white-space: pre-wrap">original req->sector of the request - how did it matter?</span></p>
<p><span style="white-space: pre-wrap">Is there a possibility that a single request is handled in parallel somewhere where </span><span style="white-space: pre-wrap">req->sector is checked?</span></p>
<p><span style="white-space: pre-wrap">Or is the same request is handled sequentially and the second+ usages suffer from screwed </span><span style="white-space: pre-wrap"></span><span style="white-space: pre-wrap">req->sector value?</span></p>
<p><span style="white-space: pre-wrap">Can you please show the call traces?</span></p>
<p><span style="white-space: pre-wrap">Thank you!
</span></p>
<pre class="moz-signature" cols="102">--
Best regards,
Konstantin Khorenko,
Virtuozzo Linux Kernel Team</pre>
<div class="moz-cite-prefix">On 17.06.2024 07:51, Lynch wu wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAHKanihHAjMF+OdozhRoq0ZMODRFZhT=Ze6b8q4S02Yp_cOwNg@mail.gmail.com">
<div dir="ltr">Hi
Konstantin:
<div><br>
<div> Thanks for your reply.</div>
<div>virtio_blk.c is the front-end driver code of the virtio
block, which has the following code:<br>
/* We need to know how many segments before we allocate. */<br>
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,<br>
struct virtio_blk_config, seg_max,<br>
&sg_elems);<br>
<br>
/* We need at least one SG element, whatever they say. */<br>
if (err || !sg_elems)<br>
sg_elems = 1;<br>
<br>
Eventually, the max number of segments will be configured
through the blk_queue_max_segments interface.<br>
When I modify the value of the sg_elems due to performance
reasons, such as 128, I will find that the<br>
front-end operating system cannot be started normally, which
is manifested as the Android file system <br>
cannot be mounted normally.<br>
<br>
You can do a simple test to modify the default value of
sg_elems virtio_blk.c in your company's<br>
virtualization solution, as follows:<br>
vzkernel$ git diff drivers/block/virtio_blk.c<br>
diff --git a/drivers/block/virtio_blk.c
b/drivers/block/virtio_blk.c<br>
index fd086179f980..0a216c9cb563 100644<br>
--- a/drivers/block/virtio_blk.c<br>
+++ b/drivers/block/virtio_blk.c<br>
@@ -719,7 +719,7 @@ static int virtblk_probe(struct
virtio_device *vdev)<br>
<br>
/* We need at least one SG element, whatever they
say. */<br>
if (err || !sg_elems)<br>
- sg_elems = 1;<br>
+ sg_elems = 2;<br>
<br>
/* Prevent integer overflows and honor max vq size
*/<br>
sg_elems = min_t(u32, sg_elems,
VIRTIO_BLK_MAX_SG_ELEMS - 2);<br>
<br>
At this point, the expected test symptom is that the
front-end OS does not boot properly.<br>
<br>
In our virtualization solution, we used your company's blk.c
as the back-end driver of Virtio Block,<br>
found the above problems, and made problem fixes, and at the
same time, the performance of Virtio <br>
Block has also been improved, so we want to give back to the
open source community.<br>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">Konstantin Khorenko <<a href="mailto:khorenko@virtuozzo.com" moz-do-not-send="true" class="moz-txt-link-freetext">khorenko@virtuozzo.com</a>>
于2024年6月14日周五 19:23写道：<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi
Lynch, Andrey,<br>
<br>
thank you for the patch, but can you please describe the
problem it fixes in a bit more details?<br>
i see that the patch preserves original req->sector, but
why/how that becomes important in case <br>
VIRTIO_BLK_F_MQ feature is set?<br>
<br>
Thank you.<br>
<br>
--<br>
Best regards,<br>
<br>
Konstantin Khorenko,<br>
Virtuozzo Linux Kernel Team<br>
<br>
On 17.05.2024 11:09, Andrey Zhadchenko wrote:<br>
> Hi<br>
> <br>
> Thank you for the patch.<br>
> vhost-blk didn't spark enough interest to be reviewed and
merged into<br>
> the upstream and the code is not present here.<br>
> I have forwarded your patch to relevant openvz kernel
mailing list.<br>
> <br>
> On 5/17/24 07:34, Lynch wrote:<br>
>> ---<br>
>> drivers/vhost/blk.c | 6 ++++--<br>
>> 1 file changed, 4 insertions(+), 2 deletions(-)<br>
>><br>
>> diff --git a/drivers/vhost/blk.c
b/drivers/vhost/blk.c<br>
>> index 44fbf253e773..0e946d9dfc33 100644<br>
>> --- a/drivers/vhost/blk.c<br>
>> +++ b/drivers/vhost/blk.c<br>
>> @@ -251,6 +251,7 @@ static int
vhost_blk_bio_make(struct vhost_blk_req *req,<br>
>> struct page **pages, *page;<br>
>> struct bio *bio = NULL;<br>
>> int bio_nr = 0;<br>
>> + sector_t sector_tmp;<br>
>> <br>
>> if (unlikely(req->bi_opf == REQ_OP_FLUSH))<br>
>> return vhost_blk_bio_make_simple(req,
bdev);<br>
>> @@ -270,6 +271,7 @@ static int
vhost_blk_bio_make(struct vhost_blk_req *req,<br>
>> req->bio = req->inline_bio;<br>
>> }<br>
>> <br>
>> + sector_tmp = req->sector;<br>
>> req->iov_nr = 0;<br>
>> for (i = 0; i < iov_nr; i++) {<br>
>> int pages_nr =
iov_num_pages(&iov[i]);<br>
>> @@ -302,7 +304,7 @@ static int
vhost_blk_bio_make(struct vhost_blk_req *req,<br>
>> bio =
bio_alloc(GFP_KERNEL, pages_nr_total);<br>
>> if (!bio)<br>
>> goto fail;<br>
>> -
bio->bi_iter.bi_sector = req->sector;<br>
>> +
bio->bi_iter.bi_sector = sector_tmp;<br>
>> bio_set_dev(bio, bdev);<br>
>> bio->bi_private =
req;<br>
>> bio->bi_end_io =
vhost_blk_req_done;<br>
>> @@ -314,7 +316,7 @@ static int
vhost_blk_bio_make(struct vhost_blk_req *req,<br>
>> iov_len -= len;<br>
>> <br>
>> pos = (iov_base &
VHOST_BLK_SECTOR_MASK) + iov_len;<br>
>> - req->sector += pos >>
VHOST_BLK_SECTOR_BITS;<br>
>> + sector_tmp += pos >>
VHOST_BLK_SECTOR_BITS;<br>
>> }<br>
>> <br>
>> pages += pages_nr;<br>
</blockquote>
</div>
</blockquote>
</body>
</html>