Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

kernel: Enable virtio-scsi #202

Merged
merged 1 commit into from
Dec 18, 2017

Conversation

amshinde
Copy link

@amshinde amshinde commented Dec 8, 2017

Need this config to move to virtio-scsi.

Fixes #201

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

@@ -1,6 +1,6 @@
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.9.54 Kernel Configuration
# Linux/x86 4.9.60 Kernel Configuration

Choose a reason for hiding this comment

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

This change should be on a different commit I think?

# CONFIG_SCSI is not set
# CONFIG_SCSI_DMA is not set
CONFIG_SCSI=y
CONFIG_SCSI_DMA=y
Copy link
Contributor

Choose a reason for hiding this comment

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

Can I ask - do we know if we need the SCSI_DMA set?
I was going to ask the same of the SG, but the virtio scsi driver does have SG code, and I can see how that could be used. Having DMA for a purely virtual device feels slightly odd though (but maybe it too is tied into how SG is passed/used?).

Copy link
Author

Choose a reason for hiding this comment

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

@grahamwhaley This is enabled by default with virtio-scsi, maybe pulled in with SG as well.

@@ -603,7 +603,7 @@ CONFIG_PCI_MSI_IRQ_DOMAIN=y
# CONFIG_HT_IRQ is not set
CONFIG_PCI_ATS=y
# CONFIG_PCI_IOV is not set
# CONFIG_PCI_PRI is not set
CONFIG_PCI_PRI=y
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know we need this? I looked it up - it allows PCI devices behind an iommu to recover from page faults - sounds a little 'fringe' for our virtio case?

Copy link
Author

Choose a reason for hiding this comment

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

@grahamwhaley Yes you are right, we dont typically need this for the virtio case, taking this out.

@@ -1,6 +1,6 @@
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.9.54 Kernel Configuration
# Linux/x86 4.9.60 Kernel Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @jodh-intel - if you did this upgrade against a new kernel (and took a defconfig or allyes or similar), and/or you need to have the latest kernel for a feature, can you do the version upgrade as a separate commit (and possibly a separate PR?).

Copy link
Author

Choose a reason for hiding this comment

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

We are currently using kernel 4.9.60, but our config file still contains 4.9.54!
I can put this change in a separate PR itself.

Choose a reason for hiding this comment

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

This sounds like a bit of a "process gap".

@grahamwhaley, @jcvenegas, @erick0z - the packaging scripts run make oldconfig, but shouldn't we be committing the changes to avoid this sort of issue? Ideally, we'd build both .deb and .rpm packages, diff the two make oldconfig-updated .config files (they should be the same we'd hope!) then commit one of them as https://github.com/clearcontainers/packaging/blob/master/kernel/kernel-config-4.9.x so the comment line above is always in sync with the kernel version we're building?

# CONFIG_SCSI_NETLINK is not set
# CONFIG_SCSI_MQ_DEFAULT is not set

Choose a reason for hiding this comment

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

@amshinde can we enable mq and see if we can leverage it in the virtio case? mq typically gives us clear performance and scaling benefits.

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/virtualization_tuning_and_optimization_guide/sect-virtualization_tuning_optimization_guide-blockio-techniques

Copy link
Author

Choose a reason for hiding this comment

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

@mcastelino I have enabled this option

@egernst
Copy link

egernst commented Dec 12, 2017

I agree. At the very least this should be done when the kernel is updated to a new dot release.

@amshinde
Copy link
Author

@grahamwhaley Can you take another look?

@amshinde
Copy link
Author

ping @grahamwhaley

Need this config to move to virtio-scsi.

Fixes clearcontainers#201

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

lgtm
I know @egernst noted we could land this with a kernel minor upgrade - but I'm not sure how we'd co-ordinate that.
Merging...

@grahamwhaley grahamwhaley merged commit e944581 into clearcontainers:master Dec 18, 2017
egernst pushed a commit to egernst/cc-packaging that referenced this pull request Nov 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants