Project

General

Profile

Actions

Bug #7312

closed

zfs checksum errors observed in a zpool full of NVMe SSDs

Added by Youzhong Yang almost 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Category:
driver - device drivers
Start date:
2016-08-16
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

We created a zpool using 24 Intel DC P3600/P3700 NVMe SSDs and observed zfs checksum errors when doing zfs scrub after some I/O ops.

Applying the attached patch seems to resolve the checksum errors. We are still doing stress testing but so far so good.


Files


Related issues

Related to illumos gate - Bug #6908: Samsung SSD SM951-NVMe shows checksum errorsClosedHans Rosenfeld2016-04-13

Actions
Actions #1

Updated by Hans Rosenfeld almost 5 years ago

Thanks Youzhong, that is very interesting. I will also test the patch.

Can you tell me how you determined the different values for the DMA attributes?

Actions #2

Updated by Youzhong Yang almost 5 years ago

page 52 of http://www.nvmexpress.org/wp-content/uploads/NVM-Express-1_1b.pdf says:

"Dependent on the command definition, the first PRP entry contained within the command may have a
non-zero offset within the memory page. The first PRP List entry (i.e. the first pointer to a memory page
containing additional PRP entries) that if present is typically contained in the PRP Entry 2 location within
the command, shall be Qword aligned and may also have a non-zero offset within the memory page.

PRP entries contained within a PRP List shall have a memory page offset of 0h. If a second PRP entry is
present within a command, it shall have a memory page offset of 0h. In both cases, the entries are
memory page aligned based on the value in CC.MPS."

The patch is to ensure we set up PRPs correctly, with the first one and the PRP list page possibly Qword aligned, and all cookies being pagesize.

The patch is modified and attached.

Actions #3

Updated by Hans Rosenfeld almost 5 years ago

I have experimented with your first patch and noticed that the only change needed to make the checksum errors disappear is setting seg to pagesize - 1. Can you please give the attached patch a test?

The other changes seem unrelated. I have a bit of trouble understanding how you concluded those numbers from the spec. When I wrote the driver I derived the numbers it's currently using from an earlier spec, but nothing of that changed in the later revision. Obviously I made a mistake in seg and didn't see it even when I was looking for a problem in the attributes. Hence I'd really appreciate it if you could explain to me where the new count_max, align, minxfer, maxxfer, sgllen, and granular values come from. I'd really like to understand your reasoning.

Actions #4

Updated by Hans Rosenfeld almost 5 years ago

  • Related to Bug #6908: Samsung SSD SM951-NVMe shows checksum errors added
Actions #5

Updated by Youzhong Yang almost 5 years ago

I've tested the change to dma_attr_seg only, yes it worked and I'm not seeing any checksum errors.

Here is my reasoning for those attrs:

dma_attr_align = 4: the spec says the 1st PRP entry or the first PRP list pointer can be Qword aligned.
dma_attr_granular = 0x1000: this is to ensure the sum of the sizes of the cookies in every DMA window (except the last one) is a whole multiple of granularity. In rootnex.c, there's something like this:

if(dma_attr_ganular > 1) 
    maxxfer = dma_attr_maxxfer - (dma_attr_maxxfer & (dma_attr_ganular - 1))
else
    maxxfer = dma_attr_maxxfer

dma_attr_seg = 0xfff: to ensure DMA cookie does not cross 4k page boundary.
dma_attr_maxxfer = 0x20000: this is the value which will be programmatically set in the driver.
dma_attr_minxfer = 1: less restrictive on alignment.

However, these atttrs changes have the same effect as with dma_attr_seg set to 0xfff in the original nvme_prp_dma_attr.

Actions #6

Updated by Electric Monk almost 5 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 2f95345b6f2a0bd2d48718fe10e82e351cb920c6

commit  2f95345b6f2a0bd2d48718fe10e82e351cb920c6
Author: Youzhong Yang <youzhong@gmail.com>
Date:   2016-08-25T15:04:45.000Z

    7312 zfs checksum errors observed in a zpool full of NVMe SSDs
    6908 Samsung SSD SM951-NVMe shows checksum errors
    Reviewed by: Hans Rosenfeld <hans.rosenfeld@nexenta.com>
    Reviewed by: Rick McNeal <rick.mcneal@nexenta.com>
    Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
    Approved by: Robert Mustacchi <rm@joyent.com>

Actions

Also available in: Atom PDF