| Message ID | 20210824131158.39970-1-slp@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtiofsd: Reverse req_list before processing it | expand |
On Tue, Aug 24, 2021 at 03:11:58PM +0200, Sergio Lopez wrote: > With the thread pool disabled, we add the requests in the queue to a > GList, processing by iterating over there afterwards. > > For adding them, we're using "g_list_prepend()", which is more > efficient but causes the requests to be processed in reverse order, > breaking the read-ahead and request-merging optimizations in the host > for sequential operations. > > According to the documentation, if you need to process the request > in-order, using "g_list_prepend()" and then reversing the list with > "g_list_reverse()" is more efficient than using "g_list_append()", so > let's do it that way. > > Testing on a spinning disk (to boost the increase of read-ahead and > request-merging) shows a 4x improvement on sequential write fio test: > > Test: > fio --directory=/mnt/virtio-fs --filename=fio-file1 --runtime=20 > --iodepth=16 --size=4G --direct=1 --blocksize=4K --ioengine libaio > --rw write --name seqwrite-libaio > > Without "g_list_reverse()": > ... > Jobs: 1 (f=1): [W(1)][100.0%][w=22.4MiB/s][w=5735 IOPS][eta 00m:00s] > seqwrite-libaio: (groupid=0, jobs=1): err= 0: pid=710: Tue Aug 24 12:58:16 2021 > write: IOPS=5709, BW=22.3MiB/s (23.4MB/s)(446MiB/20002msec); 0 zone resets > ... > > With "g_list_reverse()": > ... > Jobs: 1 (f=1): [W(1)][100.0%][w=84.0MiB/s][w=21.5k IOPS][eta 00m:00s] > seqwrite-libaio: (groupid=0, jobs=1): err= 0: pid=716: Tue Aug 24 13:00:15 2021 > write: IOPS=21.3k, BW=83.1MiB/s (87.2MB/s)(1663MiB/20001msec); 0 zone resets > ... > That's a very impressive improvememnt. Thanks Sergio for fixing this. Reviewed-by: Vivek Goyal <vgoyal@redhat.com> Vivek > Signed-off-by: Sergio Lopez <slp@redhat.com> > --- > tools/virtiofsd/fuse_virtio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c > index fc2564a603..8f4fd165b9 100644 > --- a/tools/virtiofsd/fuse_virtio.c > +++ b/tools/virtiofsd/fuse_virtio.c > @@ -716,6 +716,7 @@ static void *fv_queue_thread(void *opaque) > > /* Process all the requests. */ > if (!se->thread_pool_size && req_list != NULL) { > + req_list = g_list_reverse(req_list); > g_list_foreach(req_list, fv_queue_worker, qi); > g_list_free(req_list); > req_list = NULL; > -- > 2.26.2 >
* Sergio Lopez (slp@redhat.com) wrote: > With the thread pool disabled, we add the requests in the queue to a > GList, processing by iterating over there afterwards. > > For adding them, we're using "g_list_prepend()", which is more > efficient but causes the requests to be processed in reverse order, > breaking the read-ahead and request-merging optimizations in the host > for sequential operations. > > According to the documentation, if you need to process the request > in-order, using "g_list_prepend()" and then reversing the list with > "g_list_reverse()" is more efficient than using "g_list_append()", so > let's do it that way. > > Testing on a spinning disk (to boost the increase of read-ahead and > request-merging) shows a 4x improvement on sequential write fio test: > > Test: > fio --directory=/mnt/virtio-fs --filename=fio-file1 --runtime=20 > --iodepth=16 --size=4G --direct=1 --blocksize=4K --ioengine libaio > --rw write --name seqwrite-libaio > > Without "g_list_reverse()": > ... > Jobs: 1 (f=1): [W(1)][100.0%][w=22.4MiB/s][w=5735 IOPS][eta 00m:00s] > seqwrite-libaio: (groupid=0, jobs=1): err= 0: pid=710: Tue Aug 24 12:58:16 2021 > write: IOPS=5709, BW=22.3MiB/s (23.4MB/s)(446MiB/20002msec); 0 zone resets > ... > > With "g_list_reverse()": > ... > Jobs: 1 (f=1): [W(1)][100.0%][w=84.0MiB/s][w=21.5k IOPS][eta 00m:00s] > seqwrite-libaio: (groupid=0, jobs=1): err= 0: pid=716: Tue Aug 24 13:00:15 2021 > write: IOPS=21.3k, BW=83.1MiB/s (87.2MB/s)(1663MiB/20001msec); 0 zone resets > ... > > Signed-off-by: Sergio Lopez <slp@redhat.com> Queued > --- > tools/virtiofsd/fuse_virtio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c > index fc2564a603..8f4fd165b9 100644 > --- a/tools/virtiofsd/fuse_virtio.c > +++ b/tools/virtiofsd/fuse_virtio.c > @@ -716,6 +716,7 @@ static void *fv_queue_thread(void *opaque) > > /* Process all the requests. */ > if (!se->thread_pool_size && req_list != NULL) { > + req_list = g_list_reverse(req_list); > g_list_foreach(req_list, fv_queue_worker, qi); > g_list_free(req_list); > req_list = NULL; > -- > 2.26.2 >
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index fc2564a603..8f4fd165b9 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -716,6 +716,7 @@ static void *fv_queue_thread(void *opaque) /* Process all the requests. */ if (!se->thread_pool_size && req_list != NULL) { + req_list = g_list_reverse(req_list); g_list_foreach(req_list, fv_queue_worker, qi); g_list_free(req_list); req_list = NULL;
With the thread pool disabled, we add the requests in the queue to a GList, processing by iterating over there afterwards. For adding them, we're using "g_list_prepend()", which is more efficient but causes the requests to be processed in reverse order, breaking the read-ahead and request-merging optimizations in the host for sequential operations. According to the documentation, if you need to process the request in-order, using "g_list_prepend()" and then reversing the list with "g_list_reverse()" is more efficient than using "g_list_append()", so let's do it that way. Testing on a spinning disk (to boost the increase of read-ahead and request-merging) shows a 4x improvement on sequential write fio test: Test: fio --directory=/mnt/virtio-fs --filename=fio-file1 --runtime=20 --iodepth=16 --size=4G --direct=1 --blocksize=4K --ioengine libaio --rw write --name seqwrite-libaio Without "g_list_reverse()": ... Jobs: 1 (f=1): [W(1)][100.0%][w=22.4MiB/s][w=5735 IOPS][eta 00m:00s] seqwrite-libaio: (groupid=0, jobs=1): err= 0: pid=710: Tue Aug 24 12:58:16 2021 write: IOPS=5709, BW=22.3MiB/s (23.4MB/s)(446MiB/20002msec); 0 zone resets ... With "g_list_reverse()": ... Jobs: 1 (f=1): [W(1)][100.0%][w=84.0MiB/s][w=21.5k IOPS][eta 00m:00s] seqwrite-libaio: (groupid=0, jobs=1): err= 0: pid=716: Tue Aug 24 13:00:15 2021 write: IOPS=21.3k, BW=83.1MiB/s (87.2MB/s)(1663MiB/20001msec); 0 zone resets ... Signed-off-by: Sergio Lopez <slp@redhat.com> --- tools/virtiofsd/fuse_virtio.c | 1 + 1 file changed, 1 insertion(+)