Project

General

Profile

Bug #8268

NVMe driver needs to ignore namespaces that specify an unused LBA Format

Added by Moinak Ghosh over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
-
Start date:
2017-05-24
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
needs-triage

Description

While running illumos with a Samsung Enterprise SSD I kept seeing flood of messages like these on the console:

May 20 06:40:46 some-machine blkdev: [ID 470892 kern.warning] WARNING: blkdev8: Invalid media block size (1)
May 20 06:40:46 some-machine last message repeated 1 time
May 20 06:40:46 some-machine blkdev: [ID 470892 kern.warning] WARNING: blkdev9: Invalid media block size (1)
May 20 06:40:46 some-machine last message repeated 1 time
May 20 06:40:46 some-machine blkdev: [ID 470892 kern.warning] WARNING: blkdev10: Invalid media block size (1)
May 20 06:40:46 some-machine last message repeated 1 time

I was testing with the following model: http://www.samsung.com/semiconductor/products/flash-storage/enterprise-ssd/MZPLK3T2HCJL?ia=832
It turns out that disk is listing namespaces with the LBADS = 0 of the LBA Format that is indicated for the namespace. There is only one EUI64 namespace that is the actual storage and has non-zero LBADS.
Changing nvme.c to ignore such namespaces avoided these flood of warnings. Patch attached.


Files

History

#1

Updated by Moinak Ghosh over 2 years ago

Sorry, mentioned the wrong link in the bug description. This is the correct model:
http://www.samsung.com/semiconductor/products/flash-storage/enterprise-ssd/MZPLL1T6HEHP?ia=832

#2

Updated by Hans Rosenfeld over 2 years ago

The patch you attached won't work as ns_ignore is overwritten in the if-clause a few lines down. Can you please try the attached patch, which should work correctly?

#3

Updated by Electric Monk over 2 years ago

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

git commit 621738e24ed094c80643e1cd8d545241ae1978b1

commit  621738e24ed094c80643e1cd8d545241ae1978b1
Author: Hans Rosenfeld <hans.rosenfeld@joyent.com>
Date:   2017-06-27T14:12:51.000Z

    8268 NVMe driver needs to ignore namespaces that specify an unused LBA Format
    Reviewed by: Yuri Pankov <yuripv@gmx.com>
    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

#4

Updated by Moinak Ghosh over 2 years ago

Sorry I got caught up with other things. Yes, my original patch didn't work and I moved the code block down. Your updated patch worked till the commit for Bug #8305.

However, with the integration of nvmeadm (6235) the problem re-appeared. I digged it down to nvme_attach():

/*
         * Attach the blkdev driver for each namespace.
*/
for (i = 0; i != nvme->n_namespace_count; i++) {
if (ddi_create_minor_node(nvme->n_dip, nvme->n_ns[i].ns_name,
S_IFCHR, NVME_MINOR(ddi_get_instance(nvme->n_dip), i + 1),
DDI_NT_NVME_ATTACHMENT_POINT, 0) != DDI_SUCCESS) {
dev_err(dip, CE_WARN,
"!failed to create minor node for namespace %d", i);
goto fail;
}
if (nvme->n_ns[i].ns_ignore)
continue;
...

This code block attaches the namespace to the blkdev instance and nv_ignore is checked afterwards. With illegal ns_block_size blkdev starts shouting again. So I added an illegal block size check before ddi_create_minor_node() and it started working again:

/*
         * Attach the blkdev driver for each namespace.
/
for (i = 0; i != nvme->n_namespace_count; i++) {
/
* Skip entirely for illegal block sizes.
*/
if (nvme->n_ns[i].ns_block_size < 512)
continue;
if (ddi_create_minor_node(nvme->n_dip, nvme->n_ns[i].ns_name,
S_IFCHR, NVME_MINOR(ddi_get_instance(nvme->n_dip), i + 1),
DDI_NT_NVME_ATTACHMENT_POINT, 0) != DDI_SUCCESS) {
dev_err(dip, CE_WARN,
"!failed to create minor node for namespace %d", i);
goto fail;
}

dmesg snippet:

Jul 5 23:24:40 xxxxxx nvme: [ID 259564 kern.info] nvme0: NVMe spec version 1.2
Jul 5 23:24:40 xxxxxx nvme: [ID 768197 kern.warning] WARNING: nvme0: ignoring namespace 2, unsupported block size 1
Jul 5 23:24:40 xxxxxx nvme: [ID 768197 kern.warning] WARNING: nvme0: ignoring namespace 3, unsupported block size 1
Jul 5 23:24:40 xxxxxx nvme: [ID 768197 kern.warning] WARNING: nvme0: ignoring namespace 4, unsupported block size 1
Jul 5 23:24:40 xxxxxx nvme: [ID 768197 kern.warning] WARNING: nvme0: ignoring namespace 5, unsupported block size 1
...
Jul 5 23:24:40 xxxxxx nvme: [ID 768197 kern.warning] WARNING: nvme0: ignoring namespace 32, unsupported block size 1

#5

Updated by Hans Rosenfeld over 2 years ago

I'm not sure I follow your logic. In nvme_attach(), the check for ns_ignore comes before doing anything with blkdev. Creating the minor node will allow nvmeadm to work with the ignored namespace, which may be useful in cases where a namespace is formatted with parameters we don't support as it would still allow us to reformat that.

Looking at the code in nvme_init_ns() again, it rather looks like we've just forgot to set ns_ignore=B_TRUE if we detected an illegal block size. Thats easy to fix, and given how long 8268 has been closed already a new bug should be filed for this.

#6

Updated by Moinak Ghosh over 2 years ago

Okay I managed to confuse myself and then figure it out:

1) While testing your changeset I just hand edited the file because it is a small change and added the ns_ignore = B_TRUE without noticing that it was missed in the changeset. So it worked - my confusion. Filed Bug #8466.

2) While testing I mixed up two different versions of binaries and came to the wrong conclusion about nvme_attach() - my confusion again!

I can confirm now that it work with your changeset with the ns_ignore = B_TRUE without problems. The nvme_attach() changes are not required. Also these ignored namespaces are visible via nvmeadm:

root@xxxxxx:~# nvmeadm list
nvme0: model: SAMSUNG MZPLL1T6HEHP-00003, serial: S3HBNA0J500049, FW rev: GPNA4B3Q, NVMe v1.2
nvme0/002538e571000063 (c1t002538E571000063d0): Size = 1526185 MB, Capacity = 1526185 MB, Used = 1526185 MB
nvme0/2 (unattached): Size = 0 MB, Capacity = 0 MB, Used = 0 MB
nvme0/3 (unattached): Size = 0 MB, Capacity = 0 MB, Used = 0 MB
nvme0/4 (unattached): Size = 0 MB, Capacity = 0 MB, Used = 0 MB
....
nvme0/31 (unattached): Size = 0 MB, Capacity = 0 MB, Used = 0 MB
nvme0/32 (unattached): Size = 0 MB, Capacity = 0 MB, Used = 0 MB

#7

Updated by Hans Rosenfeld over 2 years ago

Thanks for confirming, I posted a webrev for review.

That nvmeadm output looks like we should hide ignored namespaces by default in the list output, and only print them when -v is given.

And I really wonder what those extra namespaces are good for.

Also available in: Atom PDF