Support integration with new QAT products #6767

wli5 · 2017-10-18T09:04:11Z

Support integration with new QAT products: Intel(R) C62x Chipset,
or Atom(R) C3000 Processor Product Family SoC:
https://01.org/sites/default/files/downloads/intelr-quickassist-technology/336212qatswgsgrev002review.pdf

Detect new file name in auto-conf.
Change MAX_INSTANCES to 48.
Change "num_inst" to U16 to clean a build warning.

Signed-off-by: Weigang Li weigang.li@intel.com

Description

Motivation and Context

There are some slight changes in the new QAT driver, e.g., the driver file name is changed, and max instances is increased. To integrate with the new driver, the changes in ZFS code are required.
These changes do not impact the previous QAT product.

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.
Change has been approved by a ZFS on Linux member.

gmelikov

LGTM, just one style error: "commit subject over 50 characters"

behlendorf · 2017-10-18T17:20:04Z

module/zfs/qat_compress.c

@@ -104,7 +104,7 @@ static kstat_t *qat_ksp;
 static CpaInstanceHandle dc_inst_handles[MAX_INSTANCES];
 static CpaDcSessionHandle session_handles[MAX_INSTANCES];
 static CpaBufferList **buffer_array[MAX_INSTANCES];
-static Cpa32U num_inst = 0;
+static Cpa16U num_inst = 0;


The type here was previously changed from Cpa16U to Cpa32U in commit 7a25f08 due to an overflow issue.

the overflow is in "inst_num", the commit 7a25f08 changed "num_inst" also to keep consistent, but it causes a build warning when compile with QAT driver (head file).

Ahh yes, OK thanks for the clarification.

behlendorf · 2017-10-18T17:24:40Z

module/zfs/qat_compress.c

@@ -36,7 +36,7 @@
 * Max instances in QAT device, each instance is a channel to submit
 * jobs to QAT hardware
 */
-#define	MAX_INSTANCES		6
+#define	MAX_INSTANCES		48


This simply results in slightly larger arrays being allocated which are unused on the previous chipset, correct? It would be helpful to update the comment to document the maximum values per-chipset.

Yes, it is the maximum instances supported by the chipset. The real number of instances are defined in the QAT configure file. I will update comment, thanks!

behlendorf

Then this looks good to me pending an updated comment and a slightly shorted commit message subject.

codecov · 2017-10-20T02:11:33Z

Codecov Report

Merging #6767 into master will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6767      +/-   ##
==========================================
+ Coverage   74.36%   74.54%   +0.17%     
==========================================
  Files         297      297              
  Lines       94371    94356      -15     
==========================================
+ Hits        70181    70338     +157     
+ Misses      24190    24018     -172

Flag	Coverage Δ
#kernel	`74.54% <ø> (+0.25%)`	⬆️
#user	`66.75% <ø> (+0.34%)`	⬆️

Impacted Files	Coverage Δ
cmd/zed/agents/zfs_mod.c	`57.19% <0%> (-11.58%)`	⬇️
module/zfs/spa_errlog.c	`90% <0%> (-2.31%)`	⬇️
module/zfs/dsl_userhold.c	`88.49% <0%> (-1.2%)`	⬇️
module/zfs/zfs_dir.c	`76.32% <0%> (-1.01%)`	⬇️
module/zfs/zvol.c	`84.16% <0%> (-1%)`	⬇️
module/unicode/u8_textprep.c	`5.73% <0%> (-0.58%)`	⬇️
module/zfs/zfs_ioctl.c	`81.73% <0%> (-0.43%)`	⬇️
module/zfs/zfs_rlock.c	`90.16% <0%> (-0.41%)`	⬇️
module/zfs/vdev_raidz.c	`97.37% <0%> (-0.35%)`	⬇️
module/zfs/zap.c	`67.35% <0%> (-0.18%)`	⬇️
... and 42 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6044cf5...a956eed. Read the comment docs.

Support integration with new QAT products: Intel(R) C62x Chipset, or Atom(R) C3000 Processor Product Family SoC: 1. Detect new file name in auto-conf. 2. Change MAX_INSTANCES to 48. 3. Change "num_inst" to U16 to clean a build warning. Signed-off-by: Weigang Li <weigang.li@intel.com>

wli5 · 2017-10-20T03:06:26Z

@behlendorf I've updated the comments, please take a look, thanks!

behlendorf

Looks good! I'll get it merged once the bots are done with it.

wli5 · 2017-10-23T02:37:13Z

@behlendorf thanks Brian!

Support integration with new QAT products: Intel(R) C62x Chipset, or Atom(R) C3000 Processor Product Family SoC: 1. Detect new file name in auto-conf. 2. Change MAX_INSTANCES to 48. 3. Change "num_inst" to U16 to clean a build warning. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Weigang Li <weigang.li@intel.com> Closes openzfs#6767

bmerchant · 2017-12-15T20:57:52Z

The API for HW version 1.7 products looks considerably different than the 1.6 version in #5846. Autoconf definitely fails two of the ZFS_AC_QAT tests with driver qat1.7.upstream.l.1.0.3-42.tar.gz.

Are there plans to update ZFS QAT support for this driver?

wli5 · 2017-12-19T02:47:10Z

The driver you referred qat1.7.upstream.l.1.0.3-42.tar.gz doesn't support the kernel API, the kernel driver for the new device has not been released, so please wait for the new driver release. Once you get the kernel driver for HW 1.7, the latest ZFS release with this PR can support it.

bmerchant · 2017-12-19T14:39:48Z

Thanks for the info. Makes sense since I couldn't find either of the modules checked for by autoconf anywhere in the driver source tree. I figured the API had changed, not that support wasn't included yet.

Support integration with new QAT products: Intel(R) C62x Chipset, or Atom(R) C3000 Processor Product Family SoC: 1. Detect new file name in auto-conf. 2. Change MAX_INSTANCES to 48. 3. Change "num_inst" to U16 to clean a build warning. Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Weigang Li <weigang.li@intel.com> Closes openzfs#6767

bmerchant · 2018-04-18T15:23:11Z

The first driver update qat1.7.l.4.1.0-00022.tar.gz since I made the previous post was just released on 13 April. It doesn't appear to support the kernel API either. Is there an ETA when we can expect support for C62x chipsets?

wli5 · 2018-04-19T08:51:52Z

@bmerchant, sorry I can't give an actual date, I'm not in charge of the QAT driver release. Maybe you can feedback your request via the official support?

bmerchant · 2018-04-19T12:44:56Z

Thanks, no worries. Our reps from that group are slow on responses so I thought I'd ask here too.

wli5 · 2018-04-19T15:09:22Z

@bmerchant I see.. anyway I can help you connect with the team to speed up the response if you like, we can talk offline. Thanks!

wli5 · 2018-09-28T04:08:34Z

The new QAT driver has been released which can be used to work with the latest QAT hardware C62xx chipset in ZFS, please download the driver from: https://01.org/intel-quickassist-technology
https://01.org/sites/default/files/downloads/intelr-quickassist-technology/qat1.7.l.4.3.0-00033.tar.gz

wli5 · 2018-09-28T04:09:42Z

@bmerchant please get the new driver from https://01.org/sites/default/files/downloads/intelr-quickassist-technology/qat1.7.l.4.3.0-00033.tar.gz

bmerchant · 2018-09-28T18:30:27Z

@wli5 Great, thanks for the heads up!

gmelikov approved these changes Oct 18, 2017

View reviewed changes

behlendorf requested changes Oct 18, 2017

View reviewed changes

behlendorf added the Status: Work in Progress Not yet ready for general review label Oct 18, 2017

behlendorf approved these changes Oct 19, 2017

View reviewed changes

behlendorf added Ready for Testing and removed Status: Work in Progress Not yet ready for general review labels Oct 19, 2017

wli5 force-pushed the qat branch from 1975b0c to c8f1c46 Compare October 20, 2017 02:10

wli5 force-pushed the qat branch from c8f1c46 to a956eed Compare October 20, 2017 02:31

wli5 changed the title ~~Support integration with new QAT products: Intel(R) C62x Chipset,~~ Support integration with new QAT products Oct 20, 2017

behlendorf approved these changes Oct 20, 2017

View reviewed changes

behlendorf merged commit 1cfdb0e into openzfs:master Oct 20, 2017

behlendorf added this to PR Needed for 0.7.4 in 0.7.4 Oct 20, 2017

gmelikov removed the Ready for Testing label Oct 23, 2017

behlendorf moved this from PR Needed for 0.7.4 to Merged to 0.7.4 in 0.7.4 Dec 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support integration with new QAT products #6767

Support integration with new QAT products #6767

wli5 commented Oct 18, 2017 •

edited

gmelikov left a comment

behlendorf Oct 18, 2017

wli5 Oct 19, 2017

behlendorf Oct 19, 2017

behlendorf Oct 18, 2017

wli5 Oct 19, 2017

behlendorf left a comment •

edited

codecov bot commented Oct 20, 2017 •

edited

wli5 commented Oct 20, 2017

behlendorf left a comment

wli5 commented Oct 23, 2017

bmerchant commented Dec 15, 2017

wli5 commented Dec 19, 2017

bmerchant commented Dec 19, 2017

bmerchant commented Apr 18, 2018

wli5 commented Apr 19, 2018

bmerchant commented Apr 19, 2018

wli5 commented Apr 19, 2018

wli5 commented Sep 28, 2018

wli5 commented Sep 28, 2018

bmerchant commented Sep 28, 2018

Support integration with new QAT products #6767

Support integration with new QAT products #6767

Conversation

wli5 commented Oct 18, 2017 • edited

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

gmelikov left a comment

Choose a reason for hiding this comment

behlendorf Oct 18, 2017

Choose a reason for hiding this comment

wli5 Oct 19, 2017

Choose a reason for hiding this comment

behlendorf Oct 19, 2017

Choose a reason for hiding this comment

behlendorf Oct 18, 2017

Choose a reason for hiding this comment

wli5 Oct 19, 2017

Choose a reason for hiding this comment

behlendorf left a comment • edited

Choose a reason for hiding this comment

codecov bot commented Oct 20, 2017 • edited

Codecov Report

wli5 commented Oct 20, 2017

behlendorf left a comment

Choose a reason for hiding this comment

wli5 commented Oct 23, 2017

bmerchant commented Dec 15, 2017

wli5 commented Dec 19, 2017

bmerchant commented Dec 19, 2017

bmerchant commented Apr 18, 2018

wli5 commented Apr 19, 2018

bmerchant commented Apr 19, 2018

wli5 commented Apr 19, 2018

wli5 commented Sep 28, 2018

wli5 commented Sep 28, 2018

bmerchant commented Sep 28, 2018

wli5 commented Oct 18, 2017 •

edited

behlendorf left a comment •

edited

codecov bot commented Oct 20, 2017 •

edited