NetBSD Problem Report #55674

From kardel@Kardel.name  Mon Sep 21 09:15:09 2020
Return-Path: <kardel@Kardel.name>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 22D6E1A9217
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 21 Sep 2020 09:15:09 +0000 (UTC)
Message-Id: <20200921091505.EF3D444B37@Andromeda.Kardel.name>
Date: Mon, 21 Sep 2020 11:15:05 +0200 (CEST)
From: kardel@netbsd.org
Reply-To: kardel@netbsd.org
To: gnats-bugs@NetBSD.org
Subject: ld(4) should not attempt to allocate ld(4) for 0 size disks
X-Send-Pr-Version: 3.95

>Number:         55674
>Category:       kern
>Synopsis:       ld(4) should not attempt to allocate ld(4) for 0 size disks
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Sep 21 09:20:00 +0000 2020
>Closed-Date:    Sun Sep 27 10:50:51 +0000 2020
>Last-Modified:  Fri Dec 04 21:15:02 +0000 2020
>Originator:     kardel@netbsd.org
>Release:        NetBSD 9.99.55+ and older
>Organization:

>Environment:


System: NetBSD Toblerone 9.99.55 NetBSD 9.99.55 (XEN3_DOM0) #1: Thu Apr 9 11:39:49 CEST 2020 kardel@Toblerone:/usr/src/sys/arch/amd64/compile/obj/XEN3_DOM0 amd64
Architecture: x86_64
Machine: amd64
>Description:
	After upgrading the firmware of a U.2 NVMe disk it grew 128 name spaces :-)
	NetBSD reacted with:

...
ld26 at nvme0 nsid 27
ld27 at nvme0 nsid 28
ld28 at nvme0 nsid 29
ld29 at nvme0 nsid 30
...
ld125 at nvme0 nsid 126
ld126 at nvme0 nsid 127
ld127 at nvme0 nsid 128
ppb32 at pci34 dev 1 function 2: vendor 1022 product 1483 (rev. 0x00)
ppb32: PCI Express capability version 2 <Root Port of PCI-E Root Complex> x4 @ 8.0GT/s
pci36 at ppb32 bus 194
pci36: i/o space, memory space enabled
nvme1 at pci36 dev 0 function 0: vendor 8086 product 0a54 (rev. 0x00)
nvme1: NVMe 1.2
nvme1: interrupting at ioapic1 pin 4
nvme1: INTEL SSDPE2KX040T8, firmware VDV10170, serial X----X
ld128 at nvme1 nsid 1
ld128: 3726 GB, 486401 cyl, 255 head, 63 sec, 512 bytes/sect x 7814037168 sectors
ld129 at nvme1 nsid 2
ld130 at nvme1 nsid 3
ld131 at nvme1 nsid 4
...
ld254 at nvme1 nsid 127
ld255 at nvme1 nsid 128
ppb33 at pci34 dev 1 function 3: vendor 1022 product 1483 (rev. 0x00)
ppb33: PCI Express capability version 2 <Root Port of PCI-E Root Complex> x4 @ 8.0GT/s
pci37 at ppb33 bus 195
pci37: i/o space, memory space enabled
nvme2 at pci37 dev 0 function 0: vendor 8086 product 0a54 (rev. 0x00)
nvme2: NVMe 1.2
nvme2: interrupting at ioapic1 pin 8
nvme2: INTEL SSDPE2KX040T8, firmware VDV10170, serial X----X
ld256 at nvme2 nsid 1
ld256: 3726 GB, 486401 cyl, 255 head, 63 sec, 512 bytes/sect x 7814037168 sectors
ld257 at nvme2 nsid 2
ld258 at nvme2 nsid 3
...
i...
ld382 at nvme2 nsid 127
ld383 at nvme2 nsid 128

	Consequently devpubd created 13056(minus already existing devices) entries
	in /dev.
	sysctl shows a little better picture:
sysctl hw.disknames gives:
hw.disknames = ld0 ld128 ld256 dk0 dk1 dk2 dk3 vnd0
	All additional names spaces are of 0 size - we should not allocate
	a non functional ld entry for that.
nvmectl identify nvme0
Controller Capabilities/Features
================================
Vendor ID:                  8086
Subsystem Vendor ID:        8086
Serial Number:              X----X
Model Number:               INTEL SSDPE2KX020T8
Firmware Version:           VDV10170
Recommended Arb Burst:      0
IEEE OUI Identifier:        e4 d2 5c
Multi-Interface Cap:        00
Max Data Transfer Size:     131072
Controller ID:              0x00

Admin Command Set Attributes
============================
Security Send/Receive:       Not Supported
Format NVM:                  Supported
Firmware Activate/Download:  Supported
Namespace Managment:         Supported
Abort Command Limit:         4
Async Event Request Limit:   4
Number of Firmware Slots:    4
Firmware Slot 1 Read-Only:   No
Per-Namespace SMART Log:     No
Error Log Page Entries:      64
Number of Power States:      1

NVM Command Set Attributes
==========================
Submission Queue Entry Size
  Max:                       64
  Min:                       64
Completion Queue Entry Size
  Max:                       16
  Min:                       16
Number of Namespaces:        128
Compare Command:             Not Supported
Write Uncorrectable Command: Supported
Dataset Management Command:  Supported
Write Zeroes Command:        Not Supported
Features Save/Select Field:  Not Supported
Reservation:                 Not Supported
Volatile Write Cache:        Not Present

Namespace Drive Attributes
==========================
NVM total cap:                  2000398934016 
NVM unallocated cap:            0 

nvmectl identify nvme0ns1
Size (in LBAs):              3907029168 (3726M)
Capacity (in LBAs):          3907029168 (3726M)
Utilization (in LBAs):       3907029168 (3726M)
Thin Provisioning:           Not Supported
Number of LBA Formats:       2
Current LBA Format:          LBA Format #00
LBA Format #00: Data Size:   512  Metadata Size:     0
LBA Format #01: Data Size:  4096  Metadata Size:     0

nvmectl identify nvme0ns2
Size (in LBAs):              0 (0M)
Capacity (in LBAs):          0 (0M)
Utilization (in LBAs):       0 (0M)
Thin Provisioning:           Not Supported
Number of LBA Formats:       1
Current LBA Format:          LBA Format #00
LBA Format #00: Data Size:     1  Metadata Size:     0
>How-To-Repeat:
	Have an NVMe device with multiple zero sized name spaces and see 
	ld devices allocated to zero sized name spaces.
>Fix:
	skip allocation of ld(4) devices for zero sized name spaces.

>Release-Note:

>Audit-Trail:

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55674: ld(4) should attempt to allocate ld(4) for 0 size disks
Date: Mon, 21 Sep 2020 10:51:59 -0000 (UTC)

 kardel@netbsd.org (Frank Kardel) writes:

 >> nvmectl identify nvme0ns2
 >> Size (in LBAs):              0 (0M)
 >> Capacity (in LBAs):          0 (0M)
 >> Utilization (in LBAs):       0 (0M)
 >> Thin Provisioning:           Not Supported
 >> Number of LBA Formats:       1
 >> Current LBA Format:          LBA Format #00
 >> LBA Format #00: Data Size:     1  Metadata Size:     0

 A size of zero is perfectly valid, but a "data size" of 1 (byte per block)
 is invalid.

 The ld(4) attachment already fails with that data size, but since this
 happens within the attach routine that cannot signal failure, the ld(4)
 units are still created.

 A workaround would be to issue the identify command and check the format
 already in the nvme driver. As long as ld(4) is the only child for nvme
 this shouldn't cause any problems.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: Frank Kardel <kardel@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55674: ld(4) should attempt to allocate ld(4) for 0 size
 disks
Date: Mon, 21 Sep 2020 14:02:07 +0200

 Yepp - the data size = 1 comes from the blind 1 << lbads(==0) expression 
 (nvmectl). lbads == 0 means not available.

 ld_attach() attempts to bail out on this condition, which is, as I 
 agree, too late.

 We should fix nvme_rescan() in ic/nvme.c to do the inquiriy part and 
 initial check from

 ld_nvme.c:ld_nvme_attach(). Unfortunately this introduces one additional 
 inquiry per

 valid namespace. But that is better than to allocate 13k mostly dead 
 ld(4) instances for just 3 devices.

 Frank

 On 09/21/20 12:55, Michael van Elst wrote:
 > The following reply was made to PR kern/55674; it has been noted by GNATS.
 >
 > From: mlelstv@serpens.de (Michael van Elst)
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: kern/55674: ld(4) should attempt to allocate ld(4) for 0 size disks
 > Date: Mon, 21 Sep 2020 10:51:59 -0000 (UTC)
 >
 >   kardel@netbsd.org (Frank Kardel) writes:
 >   
 >   >> nvmectl identify nvme0ns2
 >   >> Size (in LBAs):              0 (0M)
 >   >> Capacity (in LBAs):          0 (0M)
 >   >> Utilization (in LBAs):       0 (0M)
 >   >> Thin Provisioning:           Not Supported
 >   >> Number of LBA Formats:       1
 >   >> Current LBA Format:          LBA Format #00
 >   >> LBA Format #00: Data Size:     1  Metadata Size:     0
 >   
 >   A size of zero is perfectly valid, but a "data size" of 1 (byte per block)
 >   is invalid.
 >   
 >   The ld(4) attachment already fails with that data size, but since this
 >   happens within the attach routine that cannot signal failure, the ld(4)
 >   units are still created.
 >   
 >   A workaround would be to issue the identify command and check the format
 >   already in the nvme driver. As long as ld(4) is the only child for nvme
 >   this shouldn't cause any problems.
 >   
 >   --
 >   --
 >                                   Michael van Elst
 >   Internet: mlelstv@serpens.de
 >                                   "A potential Snark may lurk in every tree."
 >   

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55674: ld(4) should attempt to allocate ld(4) for 0 size disks
Date: Mon, 21 Sep 2020 14:53:57 -0000 (UTC)

 kardel@netbsd.org (Frank Kardel) writes:

 > We should fix nvme_rescan() in ic/nvme.c to do the inquiriy part and 
 > initial check from
 > 
 > ld_nvme.c:ld_nvme_attach(). Unfortunately this introduces one additional 
 > inquiry per

 There is no need to make ld_nvme repeat the inquiry. You could augment
 the attach args and pass down the data.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: "Frank Kardel" <kardel@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55674 CVS commit: src/sys/dev/ic
Date: Tue, 22 Sep 2020 11:53:10 +0000

 Module Name:	src
 Committed By:	kardel
 Date:		Tue Sep 22 11:53:10 UTC 2020

 Modified Files:
 	src/sys/dev/ic: ld_nvme.c nvme.c

 Log Message:
 PR kern/55674:
 	move name space availability check from ld_nvme.c:ld_nvme_attach()
 	to nvme.c:nvme_rescan().
 	this avoids allocation of ld(4) instances for every possible
 	name space, even if it is not usable. it also reduces the device
 	node flood generated from that strategy.


 To generate a diff of this commit:
 cvs rdiff -u -r1.23 -r1.24 src/sys/dev/ic/ld_nvme.c
 cvs rdiff -u -r1.49 -r1.50 src/sys/dev/ic/nvme.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: kardel@NetBSD.org
State-Changed-When: Tue, 22 Sep 2020 19:32:53 +0000
State-Changed-Why:
fix committed - waiting for issues before requesting pullups


State-Changed-From-To: feedback->pending-pullups
State-Changed-By: kardel@NetBSD.org
State-Changed-When: Fri, 25 Sep 2020 12:59:50 +0000
State-Changed-Why:
Pullups requested:
[pullup-8 #1610] pullup PR kern/55674 fix nvme creating many unusable ld(4) devices for nvmes with multiple name spaces
[pullup-9 #1094] pullup PR kern/55674 fix nvme creating many unusable ld(4) devices for nvmes with multiple name spaces


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55674 CVS commit: [netbsd-9] src/sys/dev/ic
Date: Sun, 27 Sep 2020 10:30:16 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sun Sep 27 10:30:16 UTC 2020

 Modified Files:
 	src/sys/dev/ic [netbsd-9]: ld_nvme.c nvme.c

 Log Message:
 Pull up following revision(s) (requested by kardel in ticket #1094):

 	sys/dev/ic/ld_nvme.c: revision 1.24
 	sys/dev/ic/nvme.c: revision 1.50

 PR kern/55674:
 	move name space availability check from ld_nvme.c:ld_nvme_attach()
 	to nvme.c:nvme_rescan().
 	this avoids allocation of ld(4) instances for every possible
 	name space, even if it is not usable. it also reduces the device
 	node flood generated from that strategy.


 To generate a diff of this commit:
 cvs rdiff -u -r1.22.2.1 -r1.22.2.2 src/sys/dev/ic/ld_nvme.c
 cvs rdiff -u -r1.44.2.3 -r1.44.2.4 src/sys/dev/ic/nvme.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55674 CVS commit: [netbsd-8] src/sys/dev/ic
Date: Sun, 27 Sep 2020 10:33:46 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sun Sep 27 10:33:45 UTC 2020

 Modified Files:
 	src/sys/dev/ic [netbsd-8]: ld_nvme.c nvme.c

 Log Message:
 Pull up following revision(s) (requested by kardel in ticket #1610):

 	sys/dev/ic/ld_nvme.c: revision 1.24 (patch)
 	sys/dev/ic/nvme.c: revision 1.50 (patch)

 PR kern/55674:
 	move name space availability check from ld_nvme.c:ld_nvme_attach()
 	to nvme.c:nvme_rescan().
 	this avoids allocation of ld(4) instances for every possible
 	name space, even if it is not usable. it also reduces the device
 	node flood generated from that strategy.


 To generate a diff of this commit:
 cvs rdiff -u -r1.16.2.4 -r1.16.2.5 src/sys/dev/ic/ld_nvme.c
 cvs rdiff -u -r1.30.2.7 -r1.30.2.8 src/sys/dev/ic/nvme.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: kardel@NetBSD.org
State-Changed-When: Sun, 27 Sep 2020 10:50:51 +0000
State-Changed-Why:
pullups done.
Thanks!


From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55674
Date: Fri, 4 Dec 2020 13:11:42 -0800 (PST)

 This fix seems to have introduced a new problem - see kern/55839 for
 details.


 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

>Unformatted:

Home	PR Database Search