Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

block: Leverage multiqueue for virtio-block #4503

Merged
merged 1 commit into from Jun 23, 2022

Conversation

amshinde
Copy link
Member

Similar to network, we can use multiple queues for virtio-block
devices. This can help improve storage performance.
This commit changes the number of queues for block devices to
the number of cpus for cloud-hypervisor and qemu.

Today the default number of cpus a VM starts with is 1.
Hence the queues used will be 1. This change will help
improve performance when the default cold-plugged cpus is greater
than one by changing this in the config file. This may also help
when we use the sandboxing feature with k8s that passes down
the sum of the resources required down to Kata.

Fixes #4502

Signed-off-by: Archana Shinde archana.m.shinde@intel.com

Similar to network, we can use multiple queues for virtio-block
devices. This can help improve storage performance.
This commit changes the number of queues for block devices to
the number of cpus for cloud-hypervisor and qemu.

Today the default number of cpus a VM starts with is 1.
Hence the queues used will be 1. This change will help
improve performance when the default cold-plugged cpus is greater
than one by changing this in the config file. This may also help
when we use the sandboxing feature with k8s that passes down
the sum of the resources required down to Kata.

Fixes kata-containers#4502

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@katacontainersbot katacontainersbot added the size/tiny Smallest and simplest task label Jun 21, 2022
if err = q.qmpMonitorCh.qmp.ExecutePCIDeviceAdd(q.qmpMonitorCh.ctx, drive.ID, devID, driver, addr, bridge.ID, romFile, 0, true, defaultDisableModern); err != nil {
queues := int(q.config.NumVCPUs)

if err = q.qmpMonitorCh.qmp.ExecutePCIDeviceAdd(q.qmpMonitorCh.ctx, drive.ID, devID, driver, addr, bridge.ID, romFile, queues, true, defaultDisableModern); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about virtio-scsi? Can we also enable multi-queue for virtio-scsi?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fengwang666 Yes, I am planning to add this to scsi as well, maybe in a follow-up PR. I do want to do some performance testing before that, like figuring out if we need to cap the number of queues. Beyond a point, increasing the queues may not give any benefits.

@amshinde amshinde requested a review from sboeuf June 21, 2022 20:39
@amshinde
Copy link
Member Author

/test

Copy link
Member

@fengwang666 fengwang666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -753,6 +753,11 @@ func (clh *cloudHypervisor) hotplugAddBlockDevice(drive *config.BlockDrive) erro
clhDisk.Readonly = &drive.ReadOnly
clhDisk.VhostUser = func(b bool) *bool { return &b }(false)

queues := int32(clh.config.NumVCPUs)
queueSize := int32(1024)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amshinde Can you add a comment to describe why here use 1024 as a queue size?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I agree with @liubin it'd be good to understand why you picked 1024. Having a deeper queue might bring some benefits or drawbacks depending on the use case.

@bergwolf
Copy link
Member

Nice work @amshinde ! Is it the only missing piece to have multi-queue support throughout the block IO stack or do we need to change more things inside the guest to more performance out of it (e.g., enable blk-mq in the guest virtio-block driver etc.)?

@amshinde
Copy link
Member Author

amshinde commented Jun 23, 2022

@bergwolf This is the kernel config required CONFIG_BLK_MQ_VIRTIO. We have to have that explicitly enabled before we moved to fragments structure for building kernel. We dont explicitly enable it, but its default value is "yes
" when VIRTIO is turned on : https://github.com/torvalds/linux/blob/master/block/Kconfig#L215
I verified this with a kernel I built using our kernel setup script and saw that CONFIG_BLK_MQ_VIRTIO=Y in the resulting kernel config file.

Copy link

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fidencio fidencio merged commit 133528d into kata-containers:main Jun 23, 2022
@amshinde amshinde deleted the multi-queue-block branch November 15, 2022 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/tiny Smallest and simplest task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add multiple queues for virtio-block devices
7 participants