nvme: return earlier when we have bdev-ns for smart-log command#2829
nvme: return earlier when we have bdev-ns for smart-log command#2829yizhanglinux wants to merge 1 commit intolinux-nvme:masterfrom
Conversation
After the fix, we will exit earlier and get more info when we have bdev-ns for smart-log command: [1] $ nvme smart-log /dev/nvme0n1 smart log: No such device or address [2] $ ./nvme smart-log /dev/nvme0n1 Only character device is allowed Signed-off-by: Yi Zhang <[email protected]>
yizhanglinux
force-pushed
the
smart-log-cmd-fix
branch
from
0b3eff3 to
bc346b7
Compare
| if (err) | ||
| return err; | ||
|
|
||
| if (is_blkdev(dev)) { |
There was a problem hiding this comment.
This looks wrong. We should be able to get the smart log page data from a NVMe block device itself, right?
There was a problem hiding this comment.
As far I understand the specs, it says SMART is only for the controller not a namespace.
5.1.12.1.3 SMART / Health Information (Log Page Identifier 02h)
This log page is used to provide SMART and general health information. The information provided is over
the life of the controller and is retained across power cycles unless otherwise specified. To request the
controller log page, the namespace identifier specified is FFFFFFFFh or 0h
There was a problem hiding this comment.
Ok. But I think it would be harsh if we restrict this command to char devices alone and not block devices as well, not to speak of the confusion this would generate in the field. And I believe there are multiple other areas in the code as well where the same issue exists. So we may have to be careful before enforcing such a check here.
There was a problem hiding this comment.
This PR is a result of a regression observed in blktests.
https://lore.kernel.org/linux-nvme/a25kumc6oyvzmr42jgowkf4yztmmiypxrvnygetwv6qvc2ol6b@tzhqqggcme3q/
If I read the spec right, then the block device is just wrong and it just worked because the devices are not checking the namespace identifier. If my understanding is correct I have no problem in breaking existing users. Or at least introduce a warning for nvme-cli 2.x and kill it for nvme-cli 3.x
There was a problem hiding this comment.
oh, I stopped reading too early. The spec says:
The controller may also
support requesting the log page on a per namespace basis, as indicated by the SMART Support bit of the
LPA field in the Identify Controller data structure in Figure 312.
|
So indeed this is not correct as @martin-gpy pointed out. Let's close this one and fix the blktests nvme/023 bug correctly. |
|
So the correct fix is only to use io_uring on char devices as the block device io_uring interface is the generic block subsystem interface. |
3 participants
No description provided.